refactor(react): simplify template attribute descriptors#2643
Merged
Conversation
🦋 Changeset detectedLatest commit: a492897 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
Contributor
|
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 ignored due to path filters (27)
📒 Files selected for processing (22)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughUnifies compiled element-template attribute descriptors to a discriminant ChangesTemplate attribute descriptor schema refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.ts`:
- Line 24: CompiledAttributeDescriptor no longer has descriptor.binding nor a
'attribute' kind, but decodeDynamicAttrsForNode still checks descriptor.binding
and descriptor.kind === 'attribute'; update the function to use the current
descriptor shape: remove references to descriptor.binding, replace kind ===
'attribute' checks with the proper kinds ('static' | 'slot' | 'spread'), and
branch on those kinds (or on existing properties like name/value/slot) to handle
attribute/static/slot/spread cases; ensure all branches compile under strict
TypeScript and reference the CompiledAttributeDescriptor fields that actually
exist.
🪄 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: b9feba6a-446b-42a4-839d-b1314e587823
⛔ Files ignored due to path filters (27)
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!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.changeset/upgrade-tasm-039.mdpackages/react/runtime/__test__/element-template/fixtures/patch/props-update/case.tsxpackages/react/runtime/__test__/element-template/fixtures/patch/state-update/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/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mapped-view-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mixed-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/multiple-text/templates.json.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/react-example/templates.json.txtpackages/react/runtime/__test__/element-template/runtime/patch/update-fixture-helper.test.tsxpackages/react/runtime/__test__/element-template/runtime/render/render-main-thread.contract.test.tspackages/react/runtime/__test__/element-template/test-utils/debug/registry.tspackages/react/runtime/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.tspackages/react/transform/crates/swc_plugin_element_template/asset.rspackages/react/transform/crates/swc_plugin_element_template/template_definition.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rspackages/rspeedy/lynx-bundle-rslib-config/package.jsonpackages/webpack/react-webpack-plugin/test/cases/element-template/basic/rspack.config.jspackages/webpack/template-webpack-plugin/package.json
Yradex
force-pushed
the
wt/et-template-shape-kind-pr
branch
2 times, most recently
from
a31b102 to
3cf5579
Compare
Yradex
requested review from
HuJean,
colinaaa,
gaoachao,
hzy,
luhc228 and
upupming
as code owners
Merging this PR will improve performance by 8.9%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
Footnotes
|
React External#1389 Bundle Size — 695.33KiB (0%).a492897(current) vs 00a0725 main#1386(baseline) Bundle metrics
|
| Current #1389 |
Baseline #1386 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch Yradex:wt/et-template-shape-kind... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9848 Bundle Size — 901.35KiB (0%).a492897(current) vs 00a0725 main#9845(baseline) Bundle metrics
|
| Current #9848 |
Baseline #9845 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
 Modules |
229(+0.44%) |
228 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.22% |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9848 |
Baseline #9845 |
|
|---|---|---|
| JS |
497.08KiB |
497.08KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:wt/et-template-shape-kind... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8275 Bundle Size — 237.15KiB (0%).a492897(current) vs 00a0725 main#8272(baseline) Bundle metrics
|
| Current #8275 |
Baseline #8272 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
197 |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.89% |
44.89% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8275 |
Baseline #8272 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.39KiB |
91.39KiB |
Bundle analysis report Branch Yradex:wt/et-template-shape-kind... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#543 Bundle Size — 199.83KiB (-0.06%).a492897(current) vs 00a0725 main#540(baseline) Bundle metrics
|
| Current #543 |
Baseline #540 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
27.1% |
27.06% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
88(-1.12%) |
89 |
 Duplicate Modules |
26(-3.7%) |
27 |
| Duplicate Code |
40.01%(-0.15%) |
40.07% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 improvement
| Current #543 |
Baseline #540 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
54.08KiB (-0.22%) |
54.19KiB |
Bundle analysis report Branch Yradex:wt/et-template-shape-kind... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1408 Bundle Size — 208.1KiB (0%).a492897(current) vs 00a0725 main#1405(baseline) Bundle metrics
|
| Current #1408 |
Baseline #1405 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
192 |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.4% |
44.4% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1408 |
Baseline #1405 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.86KiB |
96.86KiB |
Bundle analysis report Branch Yradex:wt/et-template-shape-kind... Project dashboard
Generated by RelativeCI Documentation Report issue
Yradex
force-pushed
the
wt/et-template-shape-kind-pr
branch
from
3cf5579 to
87bc988
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/runtime/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.ts (1)
23-28: ⚡ Quick winConsider using a discriminated union for stronger type safety.
The current interface allows any combination of optional fields, which is flexible but doesn't enforce correctness at compile time. A discriminated union would make the type system guarantee that each
kindhas the appropriate required fields:type CompiledAttributeDescriptor = | { kind: 'static'; key: string; value: unknown } | { kind: 'slot'; key: string; attrSlotIndex: number } | { kind: 'spread'; attrSlotIndex: number };This would catch errors at compile time if the producer emits descriptors with missing or unexpected fields for a given kind. The existing defensive checks (
if (descriptor.key)) would still work, but TypeScript would provide additional guarantees.🤖 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/test-utils/mock/mockNativePapi/templateTree.ts` around lines 23 - 28, The CompiledAttributeDescriptor interface is too permissive and should be converted to a discriminated union so TypeScript enforces required fields per kind; replace the current interface with a union type for CompiledAttributeDescriptor where 'static' requires key and value, 'slot' requires key and attrSlotIndex, and 'spread' requires attrSlotIndex, then update any places that construct or narrow CompiledAttributeDescriptor (search for the symbol CompiledAttributeDescriptor and usages in templateTree.ts and related mocks) to rely on the discriminant (descriptor.kind) for type-safe access instead of optional-field checks.
🤖 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/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.ts`:
- Around line 23-28: The CompiledAttributeDescriptor interface is too permissive
and should be converted to a discriminated union so TypeScript enforces required
fields per kind; replace the current interface with a union type for
CompiledAttributeDescriptor where 'static' requires key and value, 'slot'
requires key and attrSlotIndex, and 'spread' requires attrSlotIndex, then update
any places that construct or narrow CompiledAttributeDescriptor (search for the
symbol CompiledAttributeDescriptor and usages in templateTree.ts and related
mocks) to rely on the discriminant (descriptor.kind) for type-safe access
instead of optional-field checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a35d3fd0-8299-4ea1-b597-50debd26ceb2
⛔ Files ignored due to path filters (27)
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!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.changeset/upgrade-tasm-039.mdpackages/react/runtime/__test__/element-template/fixtures/patch/props-update/case.tsxpackages/react/runtime/__test__/element-template/fixtures/patch/state-update/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/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mapped-view-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/mixed-children/templates.json.txtpackages/react/runtime/__test__/element-template/fixtures/render/multiple-text/templates.json.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/react-example/templates.json.txtpackages/react/runtime/__test__/element-template/runtime/patch/update-fixture-helper.test.tsxpackages/react/runtime/__test__/element-template/runtime/render/render-main-thread.contract.test.tspackages/react/runtime/__test__/element-template/test-utils/debug/registry.tspackages/react/runtime/__test__/element-template/test-utils/mock/mockNativePapi/templateTree.tspackages/react/transform/crates/swc_plugin_element_template/asset.rspackages/react/transform/crates/swc_plugin_element_template/template_definition.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rspackages/rspeedy/lynx-bundle-rslib-config/package.jsonpackages/webpack/react-webpack-plugin/test/cases/element-template/basic/rspack.config.jspackages/webpack/template-webpack-plugin/package.json
✅ Files skipped from review due to trivial changes (13)
- packages/react/runtime/test/element-template/fixtures/render/mixed-children/templates.json.txt
- packages/react/runtime/test/element-template/fixtures/patch/state-update/case.tsx
- packages/react/runtime/test/element-template/fixtures/render/component-slot-content/templates.json.txt
- .changeset/upgrade-tasm-039.md
- packages/rspeedy/lynx-bundle-rslib-config/package.json
- packages/webpack/template-webpack-plugin/package.json
- packages/react/runtime/test/element-template/test-utils/debug/registry.ts
- packages/react/runtime/test/element-template/fixtures/render/child-siblings/templates.json.txt
- packages/webpack/react-webpack-plugin/test/cases/element-template/basic/rspack.config.js
- packages/react/runtime/test/element-template/fixtures/render/mapped-view-children/templates.json.txt
- packages/react/runtime/test/element-template/fixtures/render/multiple-text/templates.json.txt
- packages/react/runtime/test/element-template/fixtures/render/component/templates.json.txt
- packages/react/runtime/test/element-template/runtime/patch/update-fixture-helper.test.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/react/runtime/test/element-template/fixtures/patch/props-update/case.tsx
- packages/react/runtime/test/element-template/fixtures/render/opcodes-into-element-template/_shared.ts
- packages/react/transform/crates/swc_plugin_element_template/template_definition.rs
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
- packages/react/transform/crates/swc_plugin_element_template/asset.rs
- packages/react/runtime/test/element-template/fixtures/render/nested-templates/templates.json.txt
- packages/react/runtime/test/element-template/fixtures/render/react-example/templates.json.txt
Yradex
force-pushed
the
wt/et-template-shape-kind-pr
branch
from
87bc988 to
a492897
Compare
upupming
approved these changes
May 15, 2026
HuJean
approved these changes
May 15, 2026
gaoachao
approved these changes
May 15, 2026
PupilTong
added a commit
to PupilTong/lynx-stack
that referenced
this pull request
May 15, 2026
PR lynx-family#2643 (refactor: simplify template attribute descriptors) changed `CompiledAttributeDescriptor` from { kind: 'attribute' | 'spread'; binding: 'static' | 'slot'; ... } to { kind: 'static' | 'slot' | 'spread'; ... } but the spread-slots test added by lynx-family#2630 was left on the old shape. The new implementation only matches `kind: 'static'` and `kind: 'slot'` and falls through to the spread branch otherwise, so old-shape descriptors with `kind: 'attribute', binding: 'slot'` (carrying a non-record value) silently get skipped, producing wrong results. Update both cases in templateTree.test.ts to the new descriptor shape. This is a drive-by fix on top of my web-core merge: every PR rebased on top of main is currently blocked by this test in the `test-react` job.
upupming
added a commit
to upupming/lynx-stack
that referenced
this pull request
May 15, 2026
…escriptor shape `refactor(react): simplify template attribute descriptors (lynx-family#2643)` on `upstream/main` changed `CompiledAttributeDescriptor` from `{kind: 'attribute' | 'spread', binding: 'static' | 'slot', ...}` to `{kind: 'static' | 'slot' | 'spread', ...}`. The fixtures inside `templateTree.test.ts` (added by lynx-family#2630) still use the old shape and silently fail the "spread → slot" precedence assertion because the `kind: 'attribute'` descriptors no longer match any branch in the refactored `applyCompiledAttributes`. Neither lynx-family#2630 nor lynx-family#2643 has had a Test workflow run on `upstream/main` yet, so the regression was not caught upstream — but my branch picks up both via rebase and the failure blocks this PR's CI. Trivial fixture-only update; no source / behavior change.
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
4 participants
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.
Summary by CodeRabbit
Overview
This PR makes
kindthe single discriminator for template attribute descriptors (static,slot, orspread) and removes the separatebindingfield. It also updates the TASM encoder dependency used by the template bundling packages to@lynx-js/tasm@0.0.39.Key Points
kind: "static", dynamic attribute slots withkind: "slot", and spread slots withkind: "spread".@lynx-js/reactfor the React transform output change, plus@lynx-js/template-webpack-pluginand@lynx-js/lynx-bundle-rslib-configfor the@lynx-js/tasm@0.0.39dependency update.Checklist