feat(react): support Element Template refs#2645
Conversation
🦋 Changeset detectedLatest commit: e717cde The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Element Template ref support by introducing core ordinary-ref primitives ( ChangesElement Template ref integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 21.77%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | 002-hello-reactLynx-destroyBackground |
931.4 µs | 690.8 µs | +34.82% |
| ❌ | 006-static-raw-text-destroyBackground |
1.9 ms | 2 ms | -8.12% |
| ⚡ | transform 1000 view elements |
46.8 ms | 41.7 ms | +12.35% |
| ⚡ | basic-performance-large-css |
25.5 ms | 16.2 ms | +57.95% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Yradex:parallel/element-template/01-et-ref-support (e717cde) with main (3161422)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React External#1591 Bundle Size — 697.9KiB (+0.32%).e717cde(current) vs 3161422 main#1588(baseline) Bundle metrics
|
| Current #1591 |
Baseline #1588 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
41.03% |
41.03% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle size by type
1 change
1 regression
| Current #1591 |
Baseline #1588 |
|
|---|---|---|
697.9KiB (+0.32%) |
695.71KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#745 Bundle Size — 204.53KiB (+2.19%).e717cde(current) vs 3161422 main#742(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#8476 Bundle Size — 237.74KiB (+0.18%).e717cde(current) vs 3161422 main#8473(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard Generated by RelativeCI Documentation Report issue |
React MTF Example#1609 Bundle Size — 208.69KiB (+0.21%).e717cde(current) vs 3161422 main#1606(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard Generated by RelativeCI Documentation Report issue |
Web Explorer#10050 Bundle Size — 903.53KiB (0%).e717cde(current) vs 3161422 main#10047(baseline) Bundle metrics
Bundle size by type
|
| Current #10050 |
Baseline #10047 |
|
|---|---|---|
499.15KiB |
499.15KiB |
|
402.16KiB |
402.16KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:parallel/element-template... Project dashboard
Generated by RelativeCI Documentation Report issue
577b40b to
210a1e9
Compare
`Vitest (Windows)` failed on
`packages/rspeedy/plugin-react/test/background-only.test.ts:153`:
AssertionError: expected error.message === 'Rspack build failed.'
Expected: "Rspack build failed."
Received: "ENOENT: no such file or directory,
open 'C:\\…\\test\\dist\\.rspeedy\\rspeedy.config.js'"
The test expects rsbuild to report `Rspack build failed.` when the
configured entry is missing, but on Windows the failure surfaces
earlier as an ENOENT trying to open a generated config file under the
test's dist tree. This PR doesn't touch
`packages/rspeedy/plugin-react`; Vitest (Ubuntu) passed; the same
Vitest (Windows) check passes on other open PRs against the same
upstream (lynx-family#2645, lynx-family#2646, lynx-family#2648). Windows-only rspeedy infrastructure
flake — same family as the `validate is not a function` race we hit
earlier. Empty commit to retry.
210a1e9 to
10b5da6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react/runtime/src/element-template/background/instance.ts (1)
248-279: 💤 Low valueEdge case: silent removal after hydration could leak refs.
When
removeChildis called withsilent=true(Line 276-278), no ref cleanup is queued. This is correct for move operations (where the child is being re-inserted), but if an external caller uses silent removal for other purposes after hydration, refs would not be detached.Consider whether this behavior is intentional and properly documented, or if the silent flag should only bypass native ops while still queuing ref cleanup.
🤖 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/react/runtime/src/element-template/background/instance.ts` around lines 248 - 279, The silent-branch currently returns without queuing ref cleanup (silent path at the end), which can leak refs post-hydration; update the silent removal logic so that when silent is true you still call child.queueRefCleanupForSubtree() for hydrated trees (use isElementTemplateHydrated() to detect hydration) unless the caller explicitly intends a move (add a new flag like silentSkipRefCleanup or check an existing move indicator), or alternatively document that silent only skips native ops and must not be used post-hydration—adjust removeChild's silent handling to call child.queueRefCleanupForSubtree() (and markRemovedSubtreeForPostDispatchTeardown if appropriate) when isElementTemplateHydrated() is true, while preserving current behavior for needsMainThreadCreate(), canEmitUpdatePatch(), and ElementTemplateUpdateOps.removeNode paths.
🤖 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/react/runtime/__test__/snapshot/ref.test.jsx`:
- Line 2142: The test currently only asserts
expect(reportError).toHaveBeenCalledWith(error) which allows duplicate calls to
pass; add an exact call-count assertion for the reportError mock (e.g.,
expect(reportError).toHaveBeenCalledTimes(1)) alongside the existing
toHaveBeenCalledWith to ensure the handler was invoked exactly once; target the
reportError mock used in this test and place the toHaveBeenCalledTimes assertion
adjacent to the existing expect(reportError).toHaveBeenCalledWith(error).
In
`@packages/react/runtime/src/element-template/background/hydration-listener.ts`:
- Around line 82-91: The hydrate failure branches currently clear refs and
delayed UI ops but leave the pre-hydration event queue intact; add a call to
discard the pending event buffer (e.g. clearPendingEvents() or the project's
equivalent) alongside clearPendingRefs(), clearDelayedRefUiOps(), and
resetGlobalCommitContext() in the failure branch shown and the analogous branch
around lines 121-128 so queued events are dropped on hydrate failure.
---
Nitpick comments:
In `@packages/react/runtime/src/element-template/background/instance.ts`:
- Around line 248-279: The silent-branch currently returns without queuing ref
cleanup (silent path at the end), which can leak refs post-hydration; update the
silent removal logic so that when silent is true you still call
child.queueRefCleanupForSubtree() for hydrated trees (use
isElementTemplateHydrated() to detect hydration) unless the caller explicitly
intends a move (add a new flag like silentSkipRefCleanup or check an existing
move indicator), or alternatively document that silent only skips native ops and
must not be used post-hydration—adjust removeChild's silent handling to call
child.queueRefCleanupForSubtree() (and markRemovedSubtreeForPostDispatchTeardown
if appropriate) when isElementTemplateHydrated() is true, while preserving
current behavior for needsMainThreadCreate(), canEmitUpdatePatch(), and
ElementTemplateUpdateOps.removeNode paths.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5eb196a4-67e7-45ef-ae72-dbe5884a28be
⛔ Files ignored due to path filters (3)
packages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (50)
.changeset/empty-et-ref-adapter.mdpackages/react/runtime/__test__/core/ref.test.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/ref/direct-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/spread-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/unsupported-ref/index.tsxpackages/react/runtime/__test__/element-template/internal/legacy-internal-guardrail.test.tspackages/react/runtime/__test__/element-template/native/callDestroyLifetimeFun.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-context.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/background/hydrate.test.tspackages/react/runtime/__test__/element-template/runtime/background/instance.test.tspackages/react/runtime/__test__/element-template/runtime/background/ref/compiled-fixtures.test.tsxpackages/react/runtime/__test__/element-template/runtime/background/root-render.test.tsxpackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/ref.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsxpackages/react/runtime/__test__/element-template/runtime/template/element-template-attr-slot-plan.test.tspackages/react/runtime/__test__/snapshot/ref.test.jsxpackages/react/runtime/src/core/ref.tspackages/react/runtime/src/element-template/background/attr-slots.tspackages/react/runtime/src/element-template/background/commit-context.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/background/destroy.tspackages/react/runtime/src/element-template/background/hydrate.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/background/instance.tspackages/react/runtime/src/element-template/background/manager.tspackages/react/runtime/src/element-template/hydration-map.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/ref.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/runtime/src/element-template/runtime/template/registry.tspackages/react/runtime/src/snapshot/debug/vnodeSource.tspackages/react/runtime/src/snapshot/lifecycle/ref/delay.tspackages/react/runtime/src/snapshot/list/list.tspackages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.tspackages/react/runtime/src/snapshot/snapshot/list.tspackages/react/runtime/src/snapshot/snapshot/ref.tspackages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.tspackages/react/runtime/src/worklet-runtime/workletRuntime.tspackages/react/transform/crates/swc_plugin_element_template/attr_name.rspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/lowering.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
10b5da6 to
6d25f8b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/react/runtime/src/element-template/background/hydration-listener.ts`:
- Around line 82-84: The code calls markElementTemplateHydrated() before
triggering the hydrate update dispatch path, so if dispatchEvent(...) throws the
runtime will be left in a hydrated state incorrectly; change the flow to defer
calling markElementTemplateHydrated() until after a successful dispatchEvent
(wrap the dispatchEvent call(s) in a try/catch or promise resolution and only
call markElementTemplateHydrated() on success), and ensure the same change is
applied to the other similar blocks (the ranges around lines 96-133 and 135-140)
that currently flip hydrated state before dispatch.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20c80f85-73c6-40ce-916e-f3260e9ef4ef
⛔ Files ignored due to path filters (3)
packages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (51)
.changeset/empty-et-ref-adapter.mdpackages/react/runtime/__test__/core/ref.test.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/ref/direct-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/spread-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/unsupported-ref/index.tsxpackages/react/runtime/__test__/element-template/internal/legacy-internal-guardrail.test.tspackages/react/runtime/__test__/element-template/native/callDestroyLifetimeFun.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-context.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/background/hydrate.test.tspackages/react/runtime/__test__/element-template/runtime/background/instance.test.tspackages/react/runtime/__test__/element-template/runtime/background/ref/compiled-fixtures.test.tsxpackages/react/runtime/__test__/element-template/runtime/background/root-render.test.tsxpackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/ref.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsxpackages/react/runtime/__test__/element-template/runtime/template/element-template-attr-slot-plan.test.tspackages/react/runtime/__test__/snapshot/ref.test.jsxpackages/react/runtime/src/core/ref.tspackages/react/runtime/src/element-template/background/attr-slots.tspackages/react/runtime/src/element-template/background/commit-context.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/background/destroy.tspackages/react/runtime/src/element-template/background/hydrate.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/background/instance.tspackages/react/runtime/src/element-template/background/manager.tspackages/react/runtime/src/element-template/hydration-map.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/event.tspackages/react/runtime/src/element-template/prop-adapters/ref.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/runtime/src/element-template/runtime/template/registry.tspackages/react/runtime/src/snapshot/debug/vnodeSource.tspackages/react/runtime/src/snapshot/lifecycle/ref/delay.tspackages/react/runtime/src/snapshot/list/list.tspackages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.tspackages/react/runtime/src/snapshot/snapshot/list.tspackages/react/runtime/src/snapshot/snapshot/ref.tspackages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.tspackages/react/runtime/src/worklet-runtime/workletRuntime.tspackages/react/transform/crates/swc_plugin_element_template/attr_name.rspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/lowering.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
✅ Files skipped from review due to trivial changes (11)
- packages/react/runtime/src/snapshot/debug/vnodeSource.ts
- packages/react/runtime/src/worklet-runtime/workletRuntime.ts
- packages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.ts
- packages/react/runtime/src/snapshot/snapshot/list.ts
- packages/react/runtime/test/element-template/fixtures/background/ref/spread-ref/index.tsx
- packages/react/runtime/src/snapshot/list/list.ts
- packages/react/runtime/src/element-template/runtime/template/registry.ts
- packages/react/runtime/src/element-template/background/manager.ts
- packages/react/runtime/test/element-template/fixtures/background/ref/direct-ref/index.tsx
- packages/react/runtime/test/element-template/runtime/prop-adapters/ref.test.ts
- .changeset/empty-et-ref-adapter.md
🚧 Files skipped from review as they are similar to previous changes (33)
- packages/react/runtime/test/element-template/native/callDestroyLifetimeFun.test.ts
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.ts
- packages/react/transform/crates/swc_plugin_element_template/lib.rs
- packages/react/runtime/test/element-template/fixtures/background/ref/unsupported-ref/index.tsx
- packages/react/runtime/src/element-template/runtime/template/attr-slot-plan.ts
- packages/react/runtime/test/element-template/runtime/background/root-render.test.tsx
- packages/react/runtime/test/element-template/internal/legacy-internal-guardrail.test.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template.rs
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.ts
- packages/react/transform/crates/swc_plugin_element_template/attr_name.rs
- packages/react/runtime/test/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsx
- packages/react/runtime/src/element-template/prop-adapters/spread.ts
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.ts
- packages/react/runtime/src/element-template/hydration-map.ts
- packages/react/runtime/src/element-template/background/destroy.ts
- packages/react/runtime/test/element-template/runtime/background/commit-context.test.ts
- packages/react/transform/crates/swc_plugin_element_template/lowering.rs
- packages/react/runtime/test/element-template/runtime/background/commit-hook.test.ts
- packages/react/runtime/src/snapshot/lifecycle/ref/delay.ts
- packages/react/runtime/src/element-template/background/commit-context.ts
- packages/react/runtime/src/element-template/background/commit-hook.ts
- packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts
- packages/react/runtime/test/element-template/runtime/background/hydrate.test.ts
- packages/react/runtime/test/element-template/runtime/hydration/hydration-listener.test.ts
- packages/react/runtime/test/element-template/runtime/template/element-template-attr-slot-plan.test.ts
- packages/react/runtime/test/element-template/runtime/background/ref/compiled-fixtures.test.tsx
- packages/react/runtime/src/core/ref.ts
- packages/react/runtime/src/element-template/background/attr-slots.ts
- packages/react/runtime/test/core/ref.test.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
- packages/react/runtime/src/snapshot/snapshot/ref.ts
- packages/react/runtime/src/element-template/background/hydrate.ts
- packages/react/runtime/src/element-template/background/instance.ts
6d25f8b to
9a2d057
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/react/runtime/src/snapshot/lifecycle/ref/delay.ts`:
- Around line 53-65: RefProxy.createProxyTarget() currently returns new
RefProxy(this.refAttr) which causes the constructor to wrap it again via
createProxy, leading the proxy handler to intercept method calls like setTask
and cause recursion; change createProxyTarget to return a raw, unwrapped
instance (e.g., instantiate the underlying concrete object without calling
createProxy) so that calls to setTask invoke the real method instead of going
through the proxy handler, or alternatively update the proxy handler to
special-case 'setTask' (and possibly 'createProxyTarget') so it returns the
actual bound method instead of wrapping it; locate RefProxy, its constructor,
createProxyTarget, SelectorRefProxy.createProxy and setTask to implement the
unwrapped-instance or handler-exception fix.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92033142-f872-42ec-bfbd-b4769d46ba8b
⛔ Files ignored due to path filters (3)
packages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (51)
.changeset/empty-et-ref-adapter.mdpackages/react/runtime/__test__/core/ref.test.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/ref/direct-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/spread-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/unsupported-ref/index.tsxpackages/react/runtime/__test__/element-template/internal/legacy-internal-guardrail.test.tspackages/react/runtime/__test__/element-template/native/callDestroyLifetimeFun.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-context.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/background/hydrate.test.tspackages/react/runtime/__test__/element-template/runtime/background/instance.test.tspackages/react/runtime/__test__/element-template/runtime/background/ref/compiled-fixtures.test.tsxpackages/react/runtime/__test__/element-template/runtime/background/root-render.test.tsxpackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/ref.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsxpackages/react/runtime/__test__/element-template/runtime/template/element-template-attr-slot-plan.test.tspackages/react/runtime/__test__/snapshot/ref.test.jsxpackages/react/runtime/src/core/ref.tspackages/react/runtime/src/element-template/background/attr-slots.tspackages/react/runtime/src/element-template/background/commit-context.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/background/destroy.tspackages/react/runtime/src/element-template/background/hydrate.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/background/instance.tspackages/react/runtime/src/element-template/background/manager.tspackages/react/runtime/src/element-template/hydration-map.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/event.tspackages/react/runtime/src/element-template/prop-adapters/ref.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/runtime/src/element-template/runtime/template/registry.tspackages/react/runtime/src/snapshot/debug/vnodeSource.tspackages/react/runtime/src/snapshot/lifecycle/ref/delay.tspackages/react/runtime/src/snapshot/list/list.tspackages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.tspackages/react/runtime/src/snapshot/snapshot/list.tspackages/react/runtime/src/snapshot/snapshot/ref.tspackages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.tspackages/react/runtime/src/worklet-runtime/workletRuntime.tspackages/react/transform/crates/swc_plugin_element_template/attr_name.rspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/lowering.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
✅ Files skipped from review due to trivial changes (8)
- packages/react/runtime/test/element-template/fixtures/background/ref/unsupported-ref/index.tsx
- .changeset/empty-et-ref-adapter.md
- packages/react/runtime/src/snapshot/list/list.ts
- packages/react/runtime/src/snapshot/debug/vnodeSource.ts
- packages/react/runtime/src/element-template/background/manager.ts
- packages/react/runtime/src/snapshot/snapshot/list.ts
- packages/react/runtime/src/element-template/runtime/template/registry.ts
- packages/react/runtime/src/element-template/hydration-map.ts
🚧 Files skipped from review as they are similar to previous changes (37)
- packages/react/runtime/test/element-template/native/callDestroyLifetimeFun.test.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template.rs
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.ts
- packages/react/transform/crates/swc_plugin_element_template/lib.rs
- packages/react/runtime/test/element-template/fixtures/background/ref/direct-ref/index.tsx
- packages/react/transform/crates/swc_plugin_element_template/lowering.rs
- packages/react/runtime/test/snapshot/ref.test.jsx
- packages/react/runtime/test/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsx
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.ts
- packages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.ts
- packages/react/runtime/src/element-template/prop-adapters/event.ts
- packages/react/runtime/src/element-template/runtime/template/attr-slot-plan.ts
- packages/react/runtime/src/element-template/prop-adapters/spread.ts
- packages/react/transform/crates/swc_plugin_element_template/attr_name.rs
- packages/react/runtime/src/element-template/background/hydration-listener.ts
- packages/react/runtime/test/element-template/runtime/prop-adapters/ref.test.ts
- packages/react/runtime/src/element-template/background/commit-context.ts
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.ts
- packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
- packages/react/runtime/src/element-template/background/destroy.ts
- packages/react/runtime/test/element-template/runtime/background/commit-context.test.ts
- packages/react/runtime/test/element-template/runtime/background/root-render.test.tsx
- packages/react/runtime/src/snapshot/snapshot/ref.ts
- packages/react/runtime/src/element-template/background/commit-hook.ts
- packages/react/runtime/src/element-template/background/attr-slots.ts
- packages/react/runtime/test/element-template/runtime/background/commit-hook.test.ts
- packages/react/runtime/test/element-template/runtime/prop-adapters/spread.test.ts
- packages/react/runtime/src/element-template/background/hydrate.ts
- packages/react/runtime/test/element-template/runtime/background/ref/compiled-fixtures.test.tsx
- packages/react/runtime/test/core/ref.test.ts
- packages/react/runtime/test/element-template/runtime/template/element-template-attr-slot-plan.test.ts
- packages/react/runtime/test/element-template/runtime/hydration/hydration-listener.test.ts
- packages/react/runtime/src/core/ref.ts
- packages/react/runtime/src/element-template/prop-adapters/ref.ts
- packages/react/runtime/test/element-template/runtime/background/hydrate.test.ts
- packages/react/runtime/test/element-template/runtime/background/instance.test.ts
9a2d057 to
df9bcdf
Compare
df9bcdf to
e717cde
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/runtime/src/snapshot/snapshot/ref.ts (1)
16-18: 💤 Low valueType definition for
__refdoesn't match runtime value.
Object.defineProperty(validRef, '__ref', { value: 1 })setsvalidRef.__ref = 1(a number), but theReftype declares__ref?: { value: number }(an object). Since__refis only used for existence checks, this doesn't cause runtime issues, but the type is misleading.♻️ Suggested type fix
type Ref = OrdinaryRef<RefProxy> & { - __ref?: { value: number }; + __ref?: number; };Also applies to: 82-85
🤖 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/react/runtime/src/snapshot/snapshot/ref.ts` around lines 16 - 18, The Ref type's __ref property is declared as __ref?: { value: number } but at runtime Object.defineProperty(validRef, '__ref', { value: 1 }) sets __ref to a number; update the type to match the runtime shape (e.g., __ref?: number) and apply the same change to the other identical type declaration(s) (look for Ref, OrdinaryRef<RefProxy>, and places where validRef is defined) so the TypeScript types reflect the actual runtime value.
🤖 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.
Nitpick comments:
In `@packages/react/runtime/src/snapshot/snapshot/ref.ts`:
- Around line 16-18: The Ref type's __ref property is declared as __ref?: {
value: number } but at runtime Object.defineProperty(validRef, '__ref', { value:
1 }) sets __ref to a number; update the type to match the runtime shape (e.g.,
__ref?: number) and apply the same change to the other identical type
declaration(s) (look for Ref, OrdinaryRef<RefProxy>, and places where validRef
is defined) so the TypeScript types reflect the actual runtime value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8f3b42f-9005-44d9-a618-f7f54175f744
⛔ Files ignored due to path filters (3)
packages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_refs_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (51)
.changeset/empty-et-ref-adapter.mdpackages/react/runtime/__test__/core/ref.test.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/does-not-emit-patch-when-attrs-reference-reused/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.tspackages/react/runtime/__test__/element-template/fixtures/background/ref/direct-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/multi-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/spread-ref/index.tsxpackages/react/runtime/__test__/element-template/fixtures/background/ref/unsupported-ref/index.tsxpackages/react/runtime/__test__/element-template/internal/legacy-internal-guardrail.test.tspackages/react/runtime/__test__/element-template/native/callDestroyLifetimeFun.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-context.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/background/hydrate.test.tspackages/react/runtime/__test__/element-template/runtime/background/instance.test.tspackages/react/runtime/__test__/element-template/runtime/background/ref/compiled-fixtures.test.tsxpackages/react/runtime/__test__/element-template/runtime/background/root-render.test.tsxpackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/ref.test.tspackages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsxpackages/react/runtime/__test__/element-template/runtime/template/element-template-attr-slot-plan.test.tspackages/react/runtime/__test__/snapshot/ref.test.jsxpackages/react/runtime/src/core/ref.tspackages/react/runtime/src/element-template/background/attr-slots.tspackages/react/runtime/src/element-template/background/commit-context.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/background/destroy.tspackages/react/runtime/src/element-template/background/hydrate.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/background/instance.tspackages/react/runtime/src/element-template/hydration-map.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/event.tspackages/react/runtime/src/element-template/prop-adapters/ref.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/runtime/src/element-template/runtime/template/registry.tspackages/react/runtime/src/snapshot/debug/vnodeSource.tspackages/react/runtime/src/snapshot/lifecycle/ref/delay.tspackages/react/runtime/src/snapshot/list/list.tspackages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.tspackages/react/runtime/src/snapshot/snapshot/list.tspackages/react/runtime/src/snapshot/snapshot/ref.tspackages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.tspackages/react/runtime/src/worklet-runtime/workletRuntime.tspackages/react/transform/crates/swc_plugin_element_template/attr_name.rspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/lowering.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
✅ Files skipped from review due to trivial changes (6)
- packages/react/runtime/src/snapshot/snapshot/list.ts
- .changeset/empty-et-ref-adapter.md
- packages/react/runtime/src/worklet-runtime/workletRuntime.ts
- packages/react/runtime/src/snapshot/list/list.ts
- packages/react/runtime/src/snapshot/snapshot/snapshotInstanceHydrationMap.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
🚧 Files skipped from review as they are similar to previous changes (36)
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template.rs
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-nullish-attrs-as-empty-object/case.ts
- packages/react/runtime/test/element-template/fixtures/background/ref/unsupported-ref/index.tsx
- packages/react/runtime/test/element-template/native/callDestroyLifetimeFun.test.ts
- packages/react/runtime/test/element-template/runtime/background/root-render.test.tsx
- packages/react/runtime/test/element-template/internal/legacy-internal-guardrail.test.ts
- packages/react/runtime/test/element-template/runtime/prop-adapters/spread.test.ts
- packages/react/runtime/src/element-template/hydration-map.ts
- packages/react/runtime/src/element-template/runtime/template/attr-slot-plan.ts
- packages/react/runtime/test/element-template/fixtures/background/ref/multi-ref/index.tsx
- packages/react/runtime/test/element-template/fixtures/background/instance/ops/treats-non-object-attrs-entry-as-empty-object/case.ts
- packages/react/runtime/src/element-template/background/attr-slots.ts
- packages/react/transform/crates/swc_plugin_element_template/lowering.rs
- packages/react/transform/crates/swc_plugin_element_template/attr_name.rs
- packages/react/runtime/test/snapshot/ref.test.jsx
- packages/react/runtime/test/core/ref.test.ts
- packages/react/runtime/src/snapshot/snapshot/backgroundSnapshot.ts
- packages/react/runtime/src/element-template/background/commit-context.ts
- packages/react/transform/crates/swc_plugin_element_template/lib.rs
- packages/react/runtime/src/element-template/internal.ts
- packages/react/runtime/test/element-template/fixtures/background/ref/spread-ref/index.tsx
- packages/react/runtime/src/element-template/background/hydration-listener.ts
- packages/react/runtime/test/element-template/runtime/background/commit-hook.test.ts
- packages/react/runtime/test/element-template/runtime/background/instance.test.ts
- packages/react/runtime/test/element-template/runtime/background/commit-context.test.ts
- packages/react/runtime/test/element-template/runtime/prop-adapters/ref.test.ts
- packages/react/runtime/src/element-template/background/commit-hook.ts
- packages/react/runtime/test/element-template/runtime/background/hydrate.test.ts
- packages/react/runtime/src/core/ref.ts
- packages/react/runtime/test/element-template/runtime/hydration/hydration-listener.test.ts
- packages/react/runtime/test/element-template/runtime/template/element-template-attr-slot-plan.test.ts
- packages/react/runtime/test/element-template/runtime/background/ref/compiled-fixtures.test.tsx
- packages/react/runtime/src/snapshot/lifecycle/ref/delay.ts
- packages/react/runtime/src/element-template/background/hydrate.ts
- packages/react/runtime/src/element-template/prop-adapters/ref.ts
- packages/react/runtime/src/element-template/background/instance.ts
Summary by CodeRabbit
New Features
Tests
Overview
Key Points
refand spreadrefwithout putting user ref values into Template Definition or native payloads.Checklist