fix: <list-item /> with dynamic key & spread attributes
#1948
Conversation
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
🦋 Changeset detectedLatest commit: 9baef9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
gaoachao
changed the title
<list-item /> with dynamic key & spread attributes<list-item /> with dynamic key & spread attributes
Contributor
📝 WalkthroughWalkthroughIntroduces an Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/puny-parents-call.md(1 hunks)packages/react/runtime/src/snapshot/spread.ts(2 hunks)packages/react/transform/crates/swc_plugin_snapshot/lib.rs(9 hunks)packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js(1 hunks)packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js(1 hunks)packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.js(1 hunks)packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/puny-parents-call.md
🧠 Learnings (23)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/runtime/src/snapshot/spread.tspackages/react/transform/crates/swc_plugin_snapshot/lib.rs.changeset/puny-parents-call.mdpackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-11-04T10:15:14.965Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/runtime/__test__/snapshotPatch.test.jsx:725-749
Timestamp: 2025-11-04T10:15:14.965Z
Learning: In packages/react/runtime/src/snapshot.ts, the snapshotCreatorMap type signature uses `Record<string, (uniqId: string) => string>` (returning string) rather than void for backward compatibility. Old lazy bundles still use the pattern `const snapshot_xxx = createSnapshot(...)` directly, which requires createSnapshot to return a value. The snapshotCreatorMap creators that wrap createSnapshot calls must maintain the same return type to support these legacy bundles.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/runtime/src/snapshot/spread.tspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js.changeset/puny-parents-call.mdpackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-19T12:49:38.940Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/template-webpack-plugin/src/LynxCacheEventsSetupListRuntimeModule.ts:110-112
Timestamp: 2025-08-19T12:49:38.940Z
Learning: The GlobalEventEmitter API in the Lynx framework is designed to pass arguments as an array (single parameter) rather than spreading individual arguments. When replaying cached events with emitter.emit(listenerKey, action.data.args), the args array should be passed as-is, not spread with the spread operator.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js.changeset/puny-parents-call.mdpackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.js
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js.changeset/puny-parents-call.mdpackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.js
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-14T12:54:51.143Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: .changeset/brave-melons-add.md:1-7
Timestamp: 2025-08-14T12:54:51.143Z
Learning: In the lynx-family/lynx-stack repository, packages use 0.x.x versioning where minor version bumps indicate breaking changes (not major bumps), following pre-1.0 semantic versioning conventions.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
.changeset/puny-parents-call.mdpackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.jspackages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.js
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-10-11T06:16:12.517Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js
🧬 Code graph analysis (3)
packages/react/runtime/src/snapshot/spread.ts (2)
packages/react/runtime/src/utils.ts (1)
pick(48-56)packages/react/runtime/src/snapshot/platformInfo.ts (1)
platformInfoAttributes(70-70)
packages/react/transform/crates/swc_plugin_snapshot/lib.rs (1)
packages/react/transform/crates/swc_plugins_shared/jsx_helpers.rs (1)
jsx_is_list_item(256-261)
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js (3)
packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js (4)
pageId(4-4)el(5-5)el1(6-6)__snapshot_da39a_test_1(3-19)packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.js (3)
pageId(2-2)el(3-3)__snapshot_da39a_test_1(1-11)packages/react/runtime/src/snapshot/spread.ts (13)
index(70-72)index(112-114)index(115-118)index(128-130)index(147-149)index(161-163)index(164-167)index(212-214)index(215-218)index(228-230)index(247-249)index(261-263)index(264-266)
🪛 markdownlint-cli2 (0.18.1)
.changeset/puny-parents-call.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
.changeset/puny-parents-call.md (1)
1-3: Check if@lynx-js/reactbump should bepatchinstead ofminor.This reads like a non‑breaking bug fix; for 0.x versioning, you usually reserve
minorfor breaking changes and usepatchfor bugfixes. If this change is intentionally treated as breaking, keepingminoris fine, otherwise consider switching topatch.packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread.js (1)
16-16:updateSpreadsnapshot call updated for non-list spread.Passing
falseas the final argument is appropriate here for a non‑list spread and matches the newupdateSpreadsignature; regenerated snapshot looks consistent.packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_set_attribute_for_text_node.js (1)
33-33: ConsistentupdateSpreadusage with new boolean flag.The added
falseargument correctly marks this spread as a non‑list‑item case and aligns with the updated runtime API; generated snapshot looks fine.packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/basic_spread_list_item.js (1)
1-39: List-item spread snapshots correctly markisListItemand list children.The inner snapshot marks the
"list-item"spread as(…, 0, true), and the outer snapshot usessnapshotCreateListwith__DynamicPartListChildren, so the newisListItemflag is exercised in exactly the list‑item + spread scenario described in the changeset.packages/react/runtime/src/snapshot/spread.ts (1)
43-82: Update Page.ts line 24 to pass the newisListItemparameter; incomplete migration.The transform plugin and generated test snapshots correctly pass all 5 arguments to
updateSpread, but packages/react/components/src/Page.ts line 24 still uses a 4-arg call:updateSpread(snapshot, index, oldValue, 0). Since the root Page element is not a list item, this should be updated to passfalse:updateSpread(snapshot, index, oldValue, 0, false). While 4-arg calls remain runtime-compatible (the boolean will beundefined), migrating the source call site maintains consistency.Additionally, the behavioral concern from the original review—that the new
else if (isListItem)branch always assignssnapshot.__listItemPlatformInfo = platformInfoeven whenpickreturns an empty object, whereas the list-holder branch only updates on actual changes—remains unresolved. Verify whether this distinction is intentional or if a guard likeif (!isEmptyObject(platformInfo))should be added to keep semantics aligned.packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/mode_development_spread.js (1)
8-8: Snapshot reflects newupdateSpreadarity correctlyThe added
falseargument matches the newupdateSpread(snapshot, index, oldValue, elementIndex, isListItem)signature and is correct for a non‑list-item<view>element. Assuming this snapshot was regenerated via the transformer tests (and not hand‑edited), this change looks good.
Based on learningspackages/react/transform/crates/swc_plugin_snapshot/lib.rs (4)
107-111: End‑to‑end wiring ofis_list_itemflag for spread dynamic parts looks consistent
- Extending
DynamicPart::Spreadto include abool(Line 110) plus the newbool_to_exprhelper (Lines 124‑129) cleanly mirrors the existingi32_to_exprpattern.- The LEPUS/MIXED
to_updaterarm forDynamicPart::Spreadnow invokes$runtime_id.updateSpread(snapshot, index, oldValue, $element_index, $is_list_item)withelement_indexandis_list_itemserialized viai32_to_expr/bool_to_expr(Lines 240‑245), which aligns with the described new runtime signature and preserves call shape for all existing spreads (non‑list items just passfalse).I don’t see correctness issues here; the API change is threaded through coherently.
Also applies to: 124-129, 240-245
454-501: List‑item + spread handling correctly delegates platform info toupdateSpread
- For
list-itemwithout spread, you keep the existing behavior: platform‑specific attributes (reuse-identifier,item-key,sticky-*,estimated-*,recyclable) are stripped from the JSX element and bundled into aDynamicPart::Attr(..., AttrName::ListItemPlatformInfo)(Lines 454‑499).- When a
list-itemhas a spread, you now skip that extraction and instead emit aDynamicPart::Spread(Expr::Object(spread_obj), self.element_index, jsx_is_list_item(n))(Lines 521‑535), i.e. the runtime receives the merged props object and an explicitis_list_item: trueflag.This split behavior makes sense for fixing the “dynamic key + spread on
<list-item />” case, letting the runtime inspect spread props for list‑item platform info while retaining the old fast path when there’s no spread. Please just double‑check thatupdateSpread’s new logic doesn’t duplicate theListItemPlatformInfohandling in the non‑spread path.Also applies to: 521-535
1213-1221: Dynamic part partitioning, ref/spread index tracking, and values all updated for newSpreadshape
- The partition between “attr‑like” dynamic parts and children now treats
DynamicPart::Spread(_, _, _)alongsideDynamicPart::Attr(Line 1217), which keeps spreads in the same pipeline for updater and value generation as before.- In the dynamic‑part loop, the
snapshot_refs_and_spread_indexlogic still tracks bothAttr(..., AttrName::Ref)and allSpreadparts (Lines 1259‑1265), so spread indices remain discoverable at runtime.snapshot_dynamic_part_defpushes the updater built fromdynamic_part.to_updater(..)for spreads exactly once, withexp_indexmatching the inserted position (Lines 1267‑1274).snapshot_valuesnow handlesDynamicPart::Spread(value, _, _)by pushing the spread value expression unchanged (Lines 1323‑1337), and the children/slot processing continues to ignore spreads as non‑child parts (Lines 1363‑1371).Overall, the additional boolean payload on
Spreadis fully accounted for in the matcher patterns, and the existing invariants for indices and values appear preserved.Also applies to: 1258-1267, 1323-1337, 1363-1371
2409-2453: Newbasic_spread_list_itemtest nicely covers the bug scenarioThe new test that transforms:
<list> <list-item key="hello" item-key="world" {...obj}>!!!</list-item> </list>under
TransformMode::Testand default LEPUS target directly exercises the “<list-item />with dynamickeyplus spread attributes” path. This is a good regression test for the newly addedis_list_itemflag onDynamicPart::Spreadand the updatedupdateSpreadbehavior.
.changeset/puny-parents-call.md
Outdated
Comment on lines
9
to
14
| ``` | ||
| Native error: | ||
| code: 220201 | ||
| message: Error for illegal list item-key in parse insert with indexes: [0, ] | ||
| fix_suggestion: Please check the legality of the item-key. | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add fenced code language to fix markdownlint MD040.
markdownlint-cli2 flagged this block; you can fix it by specifying a language, e.g. text:
-```
+```text
Native error:
code: 220201
message: Error for illegal list item-key in parse insert with indexes: [0, ]
fix_suggestion: Please check the legality of the item-key.
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In .changeset/puny-parents-call.md around lines 9 to 14 the fenced code block
lacks a language specifier which triggers markdownlint MD040; update the opening
fence to include a language (e.g., change "```" to "```text") so the block
becomes a fenced code block with the language specified, then save the file.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/puny-parents-call.md(1 hunks)packages/react/components/src/Page.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/puny-parents-call.md
🧠 Learnings (12)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-14T12:54:51.143Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: .changeset/brave-melons-add.md:1-7
Timestamp: 2025-08-14T12:54:51.143Z
Learning: In the lynx-family/lynx-stack repository, packages use 0.x.x versioning where minor version bumps indicate breaking changes (not major bumps), following pre-1.0 semantic versioning conventions.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-19T12:49:38.940Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/template-webpack-plugin/src/LynxCacheEventsSetupListRuntimeModule.ts:110-112
Timestamp: 2025-08-19T12:49:38.940Z
Learning: The GlobalEventEmitter API in the Lynx framework is designed to pass arguments as an array (single parameter) rather than spreading individual arguments. When replaying cached events with emitter.emit(listenerKey, action.data.args), the args array should be passed as-is, not spread with the spread operator.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
.changeset/puny-parents-call.md
📚 Learning: 2025-11-04T10:15:14.965Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/runtime/__test__/snapshotPatch.test.jsx:725-749
Timestamp: 2025-11-04T10:15:14.965Z
Learning: In packages/react/runtime/src/snapshot.ts, the snapshotCreatorMap type signature uses `Record<string, (uniqId: string) => string>` (returning string) rather than void for backward compatibility. Old lazy bundles still use the pattern `const snapshot_xxx = createSnapshot(...)` directly, which requires createSnapshot to return a value. The snapshotCreatorMap creators that wrap createSnapshot calls must maintain the same return type to support these legacy bundles.
Applied to files:
packages/react/components/src/Page.ts
🔇 Additional comments (2)
packages/react/components/src/Page.ts (1)
24-24: LGTM - correct isListItem value for Page component.The addition of
falseas the fifth parameter correctly indicates thatPageis not a list-item component, aligning with the PR's fix for list-item spread handling..changeset/puny-parents-call.md (1)
9-14: Markdown linting issue resolved.The fenced code block now correctly specifies
textas the language, addressing the previous markdownlint MD040 warning.
| @@ -0,0 +1,14 @@ | |||
| --- | |||
| "@lynx-js/react": minor | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Consider using "patch" instead of "minor" for this bug fix.
Based on the repository's 0.x.x versioning convention, minor bumps indicate breaking changes while patch bumps are for bug fixes and non-breaking changes. Since this PR fixes an error without modifying public APIs or introducing breaking changes, a "patch" bump may be more appropriate.
🌐 Web query:
semantic versioning 0.x.x minor vs patch for bug fixes
💡 Result:
Short answer
- SemVer 2.0.0 treats 0.y.z as "initial development; anything may change" and its strict patch/minor rules only apply when MAJOR > 0. [1]
- Practically: use a patch (0.y.z+1) for pure backwards‑compatible bug fixes; use a minor (0.(y+1).0) for added functionality or when you want to signal larger/non‑trivial changes. For breaking API changes during 0.y.z the spec allows anything (many projects bump the minor for breaking changes). [1][2][3]
Recommendation
- If the change only fixes incorrect behavior and doesn't change the public API users depend on, bump the patch.
- If you add features, deprecate or make bigger internal changes (or you want to signal possible instability), bump the minor.
- Document your policy in your README/CHANGELOG so consumers know how you handle 0.x releases.
Sources
- SemVer 2.0.0 spec. [1]
- Community discussion / Q&A on handling 0.x bumping. [2][3]
Change "minor" to "patch" in the changeset.
The web search confirms that for 0.x.x versioning, patch bumps should be used for pure backwards-compatible bug fixes that don't change the public API, while minor bumps are for added functionality or non-trivial changes. Since this PR fixes a bug without modifying public APIs or introducing breaking changes, "patch" is the correct version bump.
🤖 Prompt for AI Agents
.changeset/puny-parents-call.md around line 2: the changeset currently specifies
"@lynx-js/react": minor but this PR is a backwards-compatible bug fix, so update
the version bump from "minor" to "patch" on that line; save the file ensuring
the changeset now reads "@lynx-js/react": patch and commit the updated
changeset.
HuJean
reviewed
Nov 24, 2025
| updateListItemPlatformInfo(fakeSnapshot, index, oldPlatformInfo, elementIndex); | ||
| } | ||
| } else if (isListItem) { | ||
| const platformInfo = pick(newValue, platformInfoAttributes); |
Collaborator
There was a problem hiding this comment.
How to handle other attributes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
close #1920
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist