chore(react): align element template runtime naming#2588
Conversation
🦋 Changeset detectedLatest commit: 2aac96c 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughRenames key runtime exports and the builtin raw-text ID, updates core commit/hydration/patch/render logic, refactors background instance behavior, aligns SWC plugin, and refreshes all tests and fixtures accordingly. ChangesElement Template Runtime Export Rename & Builtin ID Update
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! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94f9a893e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/runtime/__test__/element-template/runtime/background/hydrate.test.ts (1)
480-480:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix stale commit-context symbol at Line 480
GlobalCommitContextis still referenced after the import was switched toglobalCommitContext, so this test now references an undefined identifier.Suggested fix
- expect(GlobalCommitContext.nonPayload.removedSubtrees).toEqual([]); + expect(globalCommitContext.nonPayload.removedSubtrees).toEqual([]);🤖 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/__test__/element-template/runtime/background/hydrate.test.ts` at line 480, The test references the outdated symbol GlobalCommitContext which is now defined as globalCommitContext; update the assertion to use globalCommitContext (e.g., replace GlobalCommitContext.nonPayload.removedSubtrees with globalCommitContext.nonPayload.removedSubtrees) so the test uses the imported identifier, and run the hydrate.test to ensure no other stale references remain.
🧹 Nitpick comments (1)
packages/react/runtime/__test__/element-template/runtime/patch/update-timing.test.ts (1)
18-23: ⚡ Quick winPrefer shared builtin template-id constant over inline literal.
Use the exported builtin raw-text template ID constant in
createRawTextOpsto avoid future rename drift across ET tests.Proposed refactor
-import { registerBuiltinRawTextTemplate } from '../../test-utils/debug/registry.js'; +import { + BUILTIN_RAW_TEXT_TEMPLATE_ID, + registerBuiltinRawTextTemplate, +} from '../../test-utils/debug/registry.js'; ... function createRawTextOps(id: number, text: string) { return [ ElementTemplateUpdateOps.createTemplate, id, - '_et_builtin_raw_text', + BUILTIN_RAW_TEXT_TEMPLATE_ID, null, [text], [], ]; }🤖 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/__test__/element-template/runtime/patch/update-timing.test.ts` around lines 18 - 23, Replace the inline literal '_et_builtin_raw_text' in createRawTextOps with the exported builtin raw-text template ID constant: import the constant (the module exports the builtin template id, e.g. BUILTIN_RAW_TEXT_TEMPLATE_ID or similarly named) and use that identifier in createRawTextOps instead of the string literal to prevent rename drift; update the import at the top of the test file and substitute the literal in createRawTextOps.
🤖 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.
Outside diff comments:
In
`@packages/react/runtime/__test__/element-template/runtime/background/hydrate.test.ts`:
- Line 480: The test references the outdated symbol GlobalCommitContext which is
now defined as globalCommitContext; update the assertion to use
globalCommitContext (e.g., replace
GlobalCommitContext.nonPayload.removedSubtrees with
globalCommitContext.nonPayload.removedSubtrees) so the test uses the imported
identifier, and run the hydrate.test to ensure no other stale references remain.
---
Nitpick comments:
In
`@packages/react/runtime/__test__/element-template/runtime/patch/update-timing.test.ts`:
- Around line 18-23: Replace the inline literal '_et_builtin_raw_text' in
createRawTextOps with the exported builtin raw-text template ID constant: import
the constant (the module exports the builtin template id, e.g.
BUILTIN_RAW_TEXT_TEMPLATE_ID or similarly named) and use that identifier in
createRawTextOps instead of the string literal to prevent rename drift; update
the import at the top of the test file and substitute the literal in
createRawTextOps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6593ea4b-1186-4408-9dd5-3cfc5bd5145e
⛔ Files ignored due to path filters (26)
packages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_generate_attribute_slots_for_dynamic_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_background_conditional_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_boolean_and_number_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_complex_text_structure.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_dataset_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_deeply_nested_user_components.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_dynamic_class_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_events_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_events_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_id_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_inline_styles.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_interpolated_text_with_siblings_js.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_interpolated_text_with_siblings_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_mixed_content.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_nested_structure_and_dynamic_content.snapis excluded by!**/*.snappackages/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_handle_sibling_user_components.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_spread_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_handle_user_component.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_isolate_arrays_with_element_slot_placeholder.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!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_output_element_template_simple_lepus.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_output_template_with_static_attributes.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_verify_template_structure_complex.snapis excluded by!**/*.snappackages/react/transform/crates/swc_plugin_element_template/tests/__combined_snapshots__/should_verify_text_attribute_and_child_text_slots.snapis excluded by!**/*.snap
📒 Files selected for processing (85)
.changeset/empty-et-runtime-cleanup.mdpackages/react/runtime/__test__/element-template/debug/alog.test.tspackages/react/runtime/__test__/element-template/debug/profile-hook.test.tsxpackages/react/runtime/__test__/element-template/fixtures/background/instance/_shared.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/supports-silent-insert-before/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/hydrate/background-hydrate-compiled/children.non-string-raw-text-key-on-main/output.txtpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/_shared.tsxpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/children.creates-missing-nodes-recursively/output.txtpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/coverage.emit-create-raw-text-non-text/output.txtpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/coverage.emit-create-raw-text/output.txtpackages/react/runtime/__test__/element-template/fixtures/hydrate/hydration-data/_shared.tsxpackages/react/runtime/__test__/element-template/fixtures/hydrate/hydration-data/sub-components/output.txtpackages/react/runtime/__test__/element-template/fixtures/hydrate/hydration-data/text-children/output.txtpackages/react/runtime/__test__/element-template/fixtures/page/render-page/case.tsxpackages/react/runtime/__test__/element-template/fixtures/patch/_shared.tsxpackages/react/runtime/__test__/element-template/fixtures/patch/detaches-node-when-moving-between-slots/case.tsxpackages/react/runtime/__test__/element-template/fixtures/render/child-siblings/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/component-slot-content/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/component/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/component/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mapped-view-children/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/mapped-view-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mixed-children/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/mixed-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/multiple-text/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/multiple-text/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/nested-templates/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/nested-templates/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/_shared.tspackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/appends-root-text-via-append-element/native-log.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/appends-root-text-via-append-element/output.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/creates-template-from-attrs-and-slot-text/native-log.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/creates-template-from-attrs-and-slot-text/output.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/handles-multiple-text-nodes-in-the-same-slot/native-log.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/handles-multiple-text-nodes-in-the-same-slot/output.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/inserts-nested-templates-into-parent-slots/native-log.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/inserts-nested-templates-into-parent-slots/output.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/keeps-slot-children-separated-and-ordered/native-log.txtpackages/react/runtime/__test__/element-template/fixtures/render/opcodes-into-element-template/keeps-slot-children-separated-and-ordered/output.txtpackages/react/runtime/__test__/element-template/fixtures/render/react-example/papi.txtpackages/react/runtime/__test__/element-template/fixtures/render/react-example/templates.json.txtpackages/react/runtime/__test__/element-template/lynx/performance.test.tspackages/react/runtime/__test__/element-template/native/callDestroyLifetimeFun.test.tspackages/react/runtime/__test__/element-template/native/mts-destroy.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/render/preact.test.tsxpackages/react/runtime/__test__/element-template/runtime/background/update-timing.test.tspackages/react/runtime/__test__/element-template/runtime/hydration/hydration-listener.test.tspackages/react/runtime/__test__/element-template/runtime/patch/element-template-patch.test.tsxpackages/react/runtime/__test__/element-template/runtime/patch/patch-listener-alog.test.tspackages/react/runtime/__test__/element-template/runtime/patch/update-timing.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-main-thread.contract.test.tspackages/react/runtime/__test__/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsxpackages/react/runtime/__test__/element-template/runtime/render/render-to-opcodes.et.test.jsxpackages/react/runtime/__test__/element-template/runtime/render/render-transform.contract.test.tspackages/react/runtime/__test__/element-template/runtime/template/element-template-handle.test.tspackages/react/runtime/__test__/element-template/runtime/template/element-template-registry.test.tspackages/react/runtime/__test__/element-template/test-utils/debug/envManager.tspackages/react/runtime/__test__/element-template/test-utils/debug/registry.tspackages/react/runtime/__test__/element-template/test-utils/debug/renderFixtureRunner.tspackages/react/runtime/__test__/element-template/test-utils/debug/serializer.tspackages/react/runtime/__test__/element-template/test-utils/debug/updateRunner.test.tsxpackages/react/runtime/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.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/hydrate.tspackages/react/runtime/src/element-template/background/hydration-listener.tspackages/react/runtime/src/element-template/background/instance.tspackages/react/runtime/src/element-template/debug/profile.tspackages/react/runtime/src/element-template/lynx/performance.tspackages/react/runtime/src/element-template/native/mts-destroy.tspackages/react/runtime/src/element-template/runtime/patch.tspackages/react/runtime/src/element-template/runtime/render/render-opcodes.tspackages/react/runtime/src/element-template/runtime/render/render-to-opcodes.tspackages/react/runtime/src/element-template/runtime/template/registry.tspackages/react/transform/crates/swc_plugin_element_template/asset.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rspackages/webpack/react-webpack-plugin/test/cases/element-template/basic/rspack.config.js
94f9a89 to
2aac96c
Compare
React Example#7958 Bundle Size — 235.77KiB (0%).2aac96c(current) vs 19092e5 main#7957(baseline) Bundle metrics
|
| Current #7958 |
Baseline #7957 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
197 |
197 |
|
80 |
80 |
|
44.85% |
44.85% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7958 |
Baseline #7957 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
90.01KiB |
90.01KiB |
Bundle analysis report Branch Yradex:slice/element-template/07 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9530 Bundle Size — 900.02KiB (0%).2aac96c(current) vs 19092e5 main#9529(baseline) Bundle metrics
|
| Current #9530 |
Baseline #9529 |
|
|---|---|---|
44.46KiB |
44.46KiB |
|
2.22KiB |
2.22KiB |
|
0% |
0% |
|
9 |
9 |
|
11 |
11 |
|
229 |
229 |
|
11 |
11 |
|
27.28% |
27.28% |
|
10 |
10 |
|
0 |
0 |
Bundle size by type no changes
| Current #9530 |
Baseline #9529 |
|
|---|---|---|
495.88KiB |
495.88KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:slice/element-template/07 Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1088 Bundle Size — 206.69KiB (+0.04%).2aac96c(current) vs 19092e5 main#1087(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Yradex:slice/element-template/07 Project dashboard Generated by RelativeCI Documentation Report issue |
React Example with Element Template#223 Bundle Size — 197.79KiB (-0.12%).2aac96c(current) vs 19092e5 main#222(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Yradex:slice/element-template/07 Project dashboard Generated by RelativeCI Documentation Report issue |
Merging this PR will improve performance by 17.62%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | 002-hello-reactLynx-destroyBackground |
912.5 µs | 863.4 µs | +5.68% |
| ⚡ | transform 1000 view elements |
47.1 ms | 40 ms | +17.62% |
Comparing Yradex:slice/element-template/07 (2aac96c) with main (887d8a3)
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#1072 Bundle Size — 690.27KiB (0%).2aac96c(current) vs 19092e5 main#1071(baseline) Bundle metrics
|
| Current #1072 |
Baseline #1071 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch Yradex:slice/element-template/07 Project dashboard
Generated by RelativeCI Documentation Report issue
Summary by CodeRabbit
Overview
This PR narrows Element Template runtime terminology around template ids and background instances without changing the public API surface. It keeps the current ET behavior intact while making the internal model easier to follow before the next event-slot work lands.
Key Points
BackgroundElementTemplateInstanceownership so the background runtime carries the instance data it actually needs.Checklist
Verification
pnpm -C packages/react/runtime test:etcargo test --test element_template --test element_template_contract