fix: counter double-0 + always-latest scaffold; RTL-style @creact-labs/testing; decouple key from identity; drop h - #17
Merged
Conversation
… creact/testing The counter logged "Count: 0" twice at startup: the effect ran once on the undefined initial output (shown as 0 via ?? 0) and again when the async handler set count to 0. Wait for the first real value before logging, so it prints 0 once, then 1, 2, ... Also pin the scaffolded deps to "latest" so a fresh app always gets the newest @creact-labs/creact and @creact-labs/testing instead of an aging caret range. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the author fields and the LICENSE copyright line with the project org (CReact Labs) so no personal name or email address remains in the codebase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…testId Previously `useAsyncOutput` forced a `key` on every component (and the root's key was overwritten with the stack name), conflating React's reconciliation key with the durable-state address. Now: - A lone component addresses by its name (name-name); a `key` is required only to disambiguate colliding siblings (the existing collision check enforces that). - `key` is once again just the React key; the root is no longer stamped with the stack name (injectRootKey removed). - InstanceNode carries `key`, `type`, and `testId` (read off the element props), so tools can query nodes by any of them. `testId` is a universal JSX attribute. - Widen JSX.Element / CReactNode to what the runtime actually accepts (arrays, text, booleans, accessors) — Solid-style — so components may return fragments. BREAKING: node ids for previously-keyed roots change; components no longer need a key when unambiguous. Bumped to 0.5.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rework @creact-labs/testing around RTL's philosophy — you observe deployed nodes, their outputs, and testId/key/type instead of DOM: - `render(ui, options?)` returns a view with getBy/queryBy/findBy queries by testId, key, and type (get throws on 0 or >1, query returns null, find waits), plus `nodes()`, `waitFor`, and ergonomic node handles with `output()`/`outputs()`. - `waitFor(callback)` retries a callback until it returns truthy (RTL-style). - Remove the nonsensical public `h` element factory and the low-level findNode/queryNodes/readOutput/renderTest surface; app authors write real JSX. - peerDependency and tests track creact ^0.5.0. BREAKING API change. Bumped to 0.2.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With `h` gone from @creact-labs/testing, the runtime's own tests use real JSX (component elements) and plain object literals (raw string/Symbol element types the pipeline tests build by hand). Components that read `props.key` as a useAsyncOutput dependency — an artifact of the old h (which wrongly left key in props) — now use real deps; identity comes from the element key. Coverage stays at 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The generated index.test.tsx now teaches @creact-labs/testing's real API: render() + getByTestId (with getByType/getByKey noted) + output(), no h. The app renders a lone <Counter /> (no key needed) with a testId for querying. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR introduces a query-based testing render API, replaces legacy hyperscript helpers, updates JSX and runtime identity contracts, migrates tests to JSX syntax, and refreshes generated app templates, package metadata, and versions. ChangesTesting and runtime transition
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
🤖 Grok fallow review — proceedClean gate: zero dead code, duplication, health threshold, and audit findings on the change. Proceed.
Reviews the full fallow report — dead code, duplication, health, and audit. Re-runs edit this comment. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/create-creact-app/src/__tests__/templates.test.ts (1)
45-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert executable template code, not comments.
toContain("render(")andtoContain("getByTestId")also match the template’s explanatory comments. Assert the concrete statements or compile/run the generated test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/create-creact-app/src/__tests__/templates.test.ts` around lines 45 - 49, Update the template assertions for files["index.test.tsx"] in the templates test to match executable testing statements rather than generic substrings that explanatory comments can satisfy. Assert concrete render and getByTestId usage, while preserving the existing renderTest exclusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/create-creact-app/src/templates.ts`:
- Line 277: Update the comment generated by indexTsx() to reference
index.test.tsx instead of index.test.ts, matching the filename emitted by
projectFiles().
- Around line 257-259: Update the template documentation around index.test.tsx
to remove the claim that it uses “no real timers.” Describe instead that the
test renders and queries the counter in isolation without requiring a running
app or timer-based waiting, while preserving the existing query-method details.
In `@packages/testing/src/__tests__/render.test.tsx`:
- Around line 122-129: The test “defaults to a no-op backend that saves nothing”
currently asserts against an unused NoopMemory instance instead of render’s
actual backend. Update the test to verify the view’s persistence behavior
through the render path, such as asserting that no saveState operation occurs,
or remove the unused memory and assert only behavior observable from the
returned view.
In `@packages/testing/src/wait-for.ts`:
- Around line 20-38: Update the exported waitFor function to await callback()
before evaluating its truthiness, while preserving the existing retry, timeout,
and error-capture behavior for falsy results and rejected callbacks. Add
coverage in wait-for.test.ts for an async predicate that resolves falsy before
eventually resolving truthy.
---
Nitpick comments:
In `@packages/create-creact-app/src/__tests__/templates.test.ts`:
- Around line 45-49: Update the template assertions for files["index.test.tsx"]
in the templates test to match executable testing statements rather than generic
substrings that explanatory comments can satisfy. Assert concrete render and
getByTestId usage, while preserving the existing renderTest exclusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7cd9e3e8-fced-4cf0-b879-c286a956343c
📒 Files selected for processing (31)
LICENSEpackage.jsonpackages/creact/package.jsonpackages/creact/src/flow/__tests__/error-boundary.test.tsxpackages/creact/src/flow/__tests__/for.test.tsxpackages/creact/src/flow/__tests__/show.test.tsxpackages/creact/src/flow/__tests__/switch.test.tsxpackages/creact/src/jsx/jsx-runtime.tspackages/creact/src/runtime/__tests__/create-runtime.test.tsxpackages/creact/src/runtime/__tests__/instance.test.tsxpackages/creact/src/runtime/__tests__/render.test.tsxpackages/creact/src/runtime/__tests__/run.test.tsxpackages/creact/src/runtime/instance.tspackages/creact/src/runtime/run.tspackages/creact/src/store/__tests__/store.test.tsxpackages/creact/tsconfig.test.jsonpackages/create-creact-app/package.jsonpackages/create-creact-app/src/__tests__/templates.test.tspackages/create-creact-app/src/templates.tspackages/testing/package.jsonpackages/testing/src/__tests__/memory.test.tspackages/testing/src/__tests__/query.test.tspackages/testing/src/__tests__/render-test.test.tspackages/testing/src/__tests__/render.test.tsxpackages/testing/src/__tests__/wait-for.test.tspackages/testing/src/index.tspackages/testing/src/jsx.tspackages/testing/src/query.tspackages/testing/src/render-test.tspackages/testing/src/render.tspackages/testing/src/wait-for.ts
💤 Files with no reviewable changes (6)
- packages/testing/src/jsx.ts
- packages/testing/src/tests/render-test.test.ts
- packages/testing/src/tests/query.test.ts
- packages/testing/src/render-test.ts
- packages/testing/src/query.ts
- packages/creact/src/runtime/run.ts
- waitFor now awaits its callback, so an async predicate retries on its resolved value instead of returning immediately on the always-truthy pending promise; covered with an async test. TestView.waitFor accepts async callbacks too. - the default-backend test spies NoopMemory.saveState to actually verify render's default (was tautological — the local memory was never passed to render). - scaffold: fix the index.test.tsx comment and reword the README (the counter does start a real timer; the test just doesn't wait on it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One PR covering the counter bug, the always-latest scaffold, a real testing library, and the
key/hcleanup you asked for.1. The double-
0bugThe counter logged
Count: 0twice at startup — the effect ran once on theundefinedinitial output (shown as0via?? 0) and again when the handler set0. Now it waits for the first real value → a single0, then1,2, …2. Scaffold always uses the latest
Generated
package.jsonpins@creact-labs/creactand@creact-labs/testingtolatest, so a fresh app never gets an aging runtime.3.
@creact-labs/testing— a real library (RTL for creact)Redesigned around React-Testing-Library's philosophy: you observe deployed nodes, outputs, and testId/key/type, not DOM.
render(ui)→ a view withgetBy/queryBy/findByby testId, key, and type (get throws on 0/>1, query returns null, find waits),nodes(),waitFor, and node handles withoutput()/outputs().hexport (and the low-level findNode/readOutput/renderTest surface) — app authors write real JSX.testIdis now a universal JSX attribute (<Comp testId="…" />), captured by the runtime and queryable viagetByTestId.4.
keyno longer does double duty (creact)useAsyncOutputused to force akeyon every component and stamp the root with the stack name — conflating React's key with the durable-state address. Now:keyis required only to disambiguate colliding siblings.keyis just the React key again;node.key/node.type/node.testIdare exposed for tooling.JSX.Element/CReactNodewidened (Solid-style) so components can return fragments/arrays/text.5.
hgone everywhere, no maskingAll 462
h()calls across 9 creact test files converted to real JSX (object literals only for raw string/Symbol element types). The oldhwrongly keptkeyinprops; tests relying on that were corrected to real deps — documented, not silenced. creact stays at 100% coverage, 656 tests.6. Scaffold showcase
index.test.tsxnow teaches the API:render()+getByTestId+output().7. Removed personal name/email from the codebase (author fields + LICENSE).
Bumps
creact 0.5.0, @creact-labs/testing 0.2.0 (peer
^0.5.0), create-creact-app 0.1.1.Verified
npm testpasses (getByTestId showcase) → app runs with a singleCount: 0.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
render()test utility with RTL-style queries (by type, key, andtestId), output inspection, and lifecycle controls.string/number/boolean) and adds a universaltestIdattribute.key.waitForwithtimeout/interval(plusdelay).Breaking Changes
h,renderTest, and prior query helpers;waitFornow uses a callback-based signature.Chores