feat(et): support spread attributes and events#2630
Merged
Conversation
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 selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds adapter and transform support for spread-style attributes in Element Templates: new spread prop adapter, event-value serialization, attr-slot-plan integration (adaptSpreadAttrSlot), SWC transform emission for spread adapters, runtime/hydration handling, fixtures, and tests covering render, hydration, and compiled cases. ChangesSpread Event Attribute Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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! |
Merging this PR will degrade performance by 16%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 008-many-use-state-destroyBackground |
8 ms | 9.5 ms | -16% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Yradex:slice/element-template/09 (bc5e015) with main (a4692da)
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#1368 Bundle Size — 695.33KiB (0%).bc5e015(current) vs a4692da main#1354(baseline) Bundle metrics
|
| Current #1368 |
Baseline #1354 |
|
|---|---|---|
 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:slice/element-template/09 Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1387 Bundle Size — 208.1KiB (0%).bc5e015(current) vs a4692da main#1373(baseline) Bundle metrics
|
| Current #1387 |
Baseline #1373 |
|
|---|---|---|
| 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 #1387 |
Baseline #1373 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.86KiB |
96.86KiB |
Bundle analysis report Branch Yradex:slice/element-template/09 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9829 Bundle Size — 901.35KiB (0%).bc5e015(current) vs a4692da main#9815(baseline) Bundle metrics
|
| Current #9829 |
Baseline #9815 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
230 |
230 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.22% |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9829 |
Baseline #9815 |
|
|---|---|---|
| JS |
497.08KiB |
497.08KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch Yradex:slice/element-template/09 Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#522 Bundle Size — 199.95KiB (+0.06%).bc5e015(current) vs a4692da main#508(baseline) Bundle metrics
|
| Current #522 |
Baseline #508 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
27.06% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
89 |
89 |
 Duplicate Modules |
27(+3.85%) |
26 |
| Duplicate Code |
40.06%(+0.12%) |
40.01% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #522 |
Baseline #508 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
54.19KiB (+0.22%) |
54.08KiB |
Bundle analysis report Branch Yradex:slice/element-template/09 Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8254 Bundle Size — 237.15KiB (0%).bc5e015(current) vs a4692da main#8240(baseline) Bundle metrics
|
| Current #8254 |
Baseline #8240 |
|
|---|---|---|
| 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 #8254 |
Baseline #8240 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.39KiB |
91.39KiB |
Bundle analysis report Branch Yradex:slice/element-template/09 Project dashboard
Generated by RelativeCI Documentation Report issue
![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: 2
🧹 Nitpick comments (2)
packages/react/runtime/src/element-template/prop-adapters/spread.ts (1)
19-21: 💤 Low value
isSpreadRecordmay accept non-plain objects.The type guard excludes arrays but permits any object type, including
Date,RegExp,Error, class instances, etc. Spreading these into attributes could produce unexpected serialization results (e.g.,Dateobjects serialized as ISO strings or empty objects depending on the bridge).If only plain objects should be spread, consider strengthening the check:
♻️ Stricter plain-object check
function isSpreadRecord(value: unknown): value is Record<string, unknown> { - return value !== null && typeof value === 'object' && !Array.isArray(value); + return ( + value !== null && + typeof value === 'object' && + !Array.isArray(value) && + Object.getPrototypeOf(value) === Object.prototype + ); }🤖 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/prop-adapters/spread.ts` around lines 19 - 21, The current isSpreadRecord type guard allows any non-array object (e.g., Date, RegExp, class instances) to be treated as a spreadable record; change the predicate so it only returns true for plain objects by checking that value is non-null, typeof value === 'object', not an array, and has a plain prototype (e.g., Object.getPrototypeOf(value) === Object.prototype or === null) or by using Object.prototype.toString.call(value) === '[object Object]'; update the isSpreadRecord function to use one of these stricter checks so only plain plain-object literals and objects with null prototype are accepted for spreading.packages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.ts (1)
7-37: ⚡ Quick winTest relies on object iteration order for
classvsclassName.Lines 17-18 include both
className: 'primary'andclass: 'final', and line 32 expectsclass: 'final'. While modern JavaScript engines preserve string-key insertion order, relying on iteration order when both properties are present makes the test (and the underlying implementation) subtly fragile. Users might reasonably expectclassto always take precedence overclassName, or vice versa.Consider either:
- Documenting the precedence behavior explicitly, or
- Updating
prepareSpreadAttrSlotto explicitly check for both keys and apply a deterministic precedence rule (e.g.,classalways wins).♻️ Example deterministic precedence
+let classValue: unknown = undefined; + for (const key in value) { const spreadValue = value[key]; // ... other checks ... if (key === 'class' || key === 'className') { - prepared['class'] = (spreadValue ?? '') as SerializableValue; + if (key === 'class' || classValue === undefined) { + classValue = spreadValue; + } continue; } // ... } + +if (classValue !== undefined) { + prepared['class'] = (classValue ?? '') as SerializableValue; +}🤖 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/prop-adapters/spread.test.ts` around lines 7 - 37, The test relies on object iteration order for deciding between className and class; make the behavior deterministic by updating prepareSpreadAttrSlot to explicitly check for both "class" and "className" and resolve precedence (e.g., always prefer "class" over "className" or vice versa), ensure the returned prepared object sets the chosen key only, and update this test's expectation to match that explicit rule; reference prepareSpreadAttrSlot and the test case in spread.test.ts when making the change so the assertion (expect(prepared).toEqual(...)) reflects the deterministic precedence.
🤖 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/prop-adapters/spread.ts`:
- Line 64: The unchecked cast "prepared[key] = spreadValue as SerializableValue"
can admit non-serializable values; replace it with a runtime guard that
validates spreadValue before assigning: implement or call an
isSerializable(value) helper (reject functions, Symbols, Promises, DOM nodes,
and objects containing function properties or circular references), or attempt a
safe JSON.stringify in a try/catch to detect unserializable values, and only
assign to prepared[key] when the guard passes; otherwise skip the key or
log/throw a clear error. Ensure the guard is used where spreadValue is checked
(around the existing checks for undefined/functions/special keys) and reference
SerializableValue/prepared/spreadValue in the change.
- Around line 41-44: The current assignment in spread.ts blindly casts
spreadValue for keys 'class'/'className' into prepared['class']; instead
validate the type first: if spreadValue is a string or number, set
prepared['class'] = String(spreadValue); if it's an array, join its string
elements with spaces (e.g., spreadValue.filter(Boolean).map(String).join(' '));
otherwise set prepared['class'] = '' (or omit) to avoid passing objects through.
Update the code at the conditional handling of key === 'class' || key ===
'className' to perform these type checks before assigning prepared['class'].
---
Nitpick comments:
In
`@packages/react/runtime/__test__/element-template/runtime/prop-adapters/spread.test.ts`:
- Around line 7-37: The test relies on object iteration order for deciding
between className and class; make the behavior deterministic by updating
prepareSpreadAttrSlot to explicitly check for both "class" and "className" and
resolve precedence (e.g., always prefer "class" over "className" or vice versa),
ensure the returned prepared object sets the chosen key only, and update this
test's expectation to match that explicit rule; reference prepareSpreadAttrSlot
and the test case in spread.test.ts when making the change so the assertion
(expect(prepared).toEqual(...)) reflects the deterministic precedence.
In `@packages/react/runtime/src/element-template/prop-adapters/spread.ts`:
- Around line 19-21: The current isSpreadRecord type guard allows any non-array
object (e.g., Date, RegExp, class instances) to be treated as a spreadable
record; change the predicate so it only returns true for plain objects by
checking that value is non-null, typeof value === 'object', not an array, and
has a plain prototype (e.g., Object.getPrototypeOf(value) === Object.prototype
or === null) or by using Object.prototype.toString.call(value) === '[object
Object]'; update the isSpreadRecord function to use one of these stricter checks
so only plain plain-object literals and objects with null prototype are accepted
for spreading.
🪄 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: 4504495a-69a6-4b47-955a-c5d08c049365
⛔ Files ignored due to path filters (2)
packages/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_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
packages/react/runtime/__test__/element-template/fixtures/background/event/spread-event/index.tsxpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/attrs.nullish-values/output.txtpackages/react/runtime/__test__/element-template/runtime/background/event/compiled-fixtures.test.tsxpackages/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/prop-adapters/event.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/test-utils/mock/mockNativePapi/templateTree.test.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/event-value.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
💤 Files with no reviewable changes (1)
- packages/react/runtime/test/element-template/fixtures/hydrate/background-hydrate/attrs.nullish-values/output.txt
Yradex
force-pushed
the
slice/element-template/09
branch
from
632713c to
4d217ed
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/runtime/src/element-template/prop-adapters/spread.ts (1)
35-36: ⚡ Quick winPrefer
Object.entries()for consistent spread semantics.The
for...inloop at line 35 can include inherited enumerable properties, whereas object spread only uses own properties. WhileObject.entries(value)would make the intent clearer and match spread behavior, consider that the function already filters special keys (__spread, __self, __source, ref, functions). This pattern is used throughout the codebase; standardizing onObject.entries()would be a beneficial consistency improvement.Suggested fix
- for (const key in value) { - const spreadValue = value[key]; + for (const [key, spreadValue] of Object.entries(value)) {🤖 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/prop-adapters/spread.ts` around lines 35 - 36, Replace the for...in iteration in the spread adapter with an Object.entries() iteration so only own enumerable properties are processed (matching object spread semantics); in the function that currently uses "for (const key in value) { const spreadValue = value[key]; }" iterate over Object.entries(value) and keep the existing special-key filters (__spread, __self, __source, ref, functions) and handling logic unchanged.
🤖 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/element-template/prop-adapters/spread.ts`:
- Around line 35-36: Replace the for...in iteration in the spread adapter with
an Object.entries() iteration so only own enumerable properties are processed
(matching object spread semantics); in the function that currently uses "for
(const key in value) { const spreadValue = value[key]; }" iterate over
Object.entries(value) and keep the existing special-key filters (__spread,
__self, __source, ref, functions) and handling logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d5cc575-d3a6-46ac-a104-b912d11653ee
⛔ Files ignored due to path filters (2)
packages/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_keep_code_and_template_attribute_slots_in_sync_for_spread.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
packages/react/runtime/__test__/element-template/fixtures/background/event/spread-event/index.tsxpackages/react/runtime/__test__/element-template/fixtures/hydrate/background-hydrate/attrs.nullish-values/output.txtpackages/react/runtime/__test__/element-template/runtime/background/event/compiled-fixtures.test.tsxpackages/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/prop-adapters/event.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/test-utils/mock/mockNativePapi/templateTree.test.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/prop-adapters/event-value.tspackages/react/runtime/src/element-template/prop-adapters/spread.tspackages/react/runtime/src/element-template/runtime/template/attr-slot-plan.tspackages/react/transform/crates/swc_plugin_element_template/lib.rspackages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
💤 Files with no reviewable changes (1)
- packages/react/runtime/test/element-template/fixtures/hydrate/background-hydrate/attrs.nullish-values/output.txt
✅ Files skipped from review due to trivial changes (2)
- packages/react/runtime/src/element-template/internal.ts
- packages/react/runtime/test/element-template/runtime/background/hydrate.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/react/runtime/test/element-template/test-utils/mock/mockNativePapi/templateTree.test.ts
- packages/react/runtime/test/element-template/runtime/render/render-opcodes-into-element-template.et.test.tsx
- packages/react/runtime/src/element-template/runtime/template/attr-slot-plan.ts
- packages/react/runtime/src/element-template/prop-adapters/event-value.ts
- packages/react/runtime/test/element-template/fixtures/background/event/spread-event/index.tsx
- packages/react/transform/crates/swc_plugin_element_template/lib.rs
- packages/react/runtime/test/element-template/runtime/background/instance.test.ts
- packages/react/transform/crates/swc_plugin_element_template/tests/element_template_contract.rs
- packages/react/runtime/test/element-template/runtime/prop-adapters/event.test.ts
Yradex
force-pushed
the
slice/element-template/09
branch
from
4d217ed to
bc5e015
Compare
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.
3 tasks
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
3 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
New Features
Bug Fixes
Tests
Overview
This PR adds Element Template support for spread attributes in the experimental ET transform/runtime path. Spread attrs now reserve a dedicated attribute slot with a spread adapter, so the main-thread template create path, serialized hydration payload, and background update path can all preserve the same slot ordering while expanding ordinary attrs and event values through the adapter boundary.
Key Points
prop-adapters/spread, keeping event preparation on the existing event adapter path and preserving static/dynamic attr slot ordering.Checklist