fix: withInitDataInState wrong state after 2nd defaultDataProcessor #1478
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: 239d017 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughThis update addresses the state management and event handling logic for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
packages/react/runtime/src/compat/initData.ts (1)
118-118: Handler type mismatch: widenhto accept event args
this.his assigned a function with rest args, but it's declared as() => void. Update the type to avoid TS incompatibility and to match the listener signature.- h?: () => void; + h?: (...args: unknown[]) => void;
🧹 Nitpick comments (3)
.changeset/free-colts-listen.md (1)
5-5: Polish changeset wording for clarityRephrase for grammar and clarity. This reads better in release notes.
-fix `withInitDataInState` got wrong state in 2nd or more times `defaultDataProcessor`, now it will keep its own state. +fix: withInitDataInState passed wrong state to `defaultDataProcessor` on subsequent updates. It now preserves and applies the component’s own state correctly across multiple updates.packages/react/runtime/__test__/compat/initData.test.jsx (2)
55-55: Pass explicit empty args toemitfor clarity and future type safetyBe explicit to match the new
emitsignature that expects an args array.- lynx.getJSModule('GlobalEventEmitter').emit('onDataChanged'); + lynx.getJSModule('GlobalEventEmitter').emit('onDataChanged', []);
81-81: Typo in test titleFix “componnet” → “component”.
- it('should inject `__initData` to `state` of componnet', async () => { + it('should inject `__initData` to `state` of component', async () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/free-colts-listen.md(1 hunks)packages/react/runtime/__test__/compat/initData.test.jsx(2 hunks)packages/react/runtime/__test__/utils/jsModule.ts(1 hunks)packages/react/runtime/src/compat/initData.ts(1 hunks)packages/react/runtime/src/lynx/tt.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/free-colts-listen.md
📚 Learning: 2025-08-07T04:00:59.627Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.627Z
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/free-colts-listen.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/free-colts-listen.md
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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/runtime/src/lynx/tt.tspackages/react/runtime/src/compat/initData.tspackages/react/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-07-18T04:27:18.291Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Applied to files:
packages/react/runtime/src/compat/initData.tspackages/react/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
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/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/runtime/__test__/compat/initData.test.jsx
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1478 will not alter performanceComparing Summary
|
Web Explorer#4483 Bundle Size — 366.68KiB (0%).239d017(current) vs d33c1d2 main#4478(baseline) Bundle metrics
|
| Current #4483 |
Baseline #4478 |
|
|---|---|---|
 Initial JS |
143.49KiB |
143.49KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
 Modules |
210(-0.94%) |
212 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.89% |
3.89% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #4483 |
Baseline #4478 |
|
|---|---|---|
| JS |
234.68KiB |
234.68KiB |
| Other |
100.16KiB |
100.16KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch upupming:fix/withInitDataInState... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#4490 Bundle Size — 237.07KiB (~+0.01%).239d017(current) vs d33c1d2 main#4485(baseline) Bundle metrics
|
| Current #4490 |
Baseline #4485 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
38.51% |
38.51% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
158 |
158 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.86%(+0.07%) |
45.83% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 regression
| Current #4490 |
Baseline #4485 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.31KiB (~+0.01%) |
91.3KiB |
Bundle analysis report Branch upupming:fix/withInitDataInState... Project dashboard
Generated by RelativeCI Documentation Report issue
|
Please rebase ur branch XD |
upupming
force-pushed
the
fix/withInitDataInState-merge
branch
from
8602206 to
dfe3db3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/react/runtime/__test__/compat/initData.test.jsx (2)
80-82: Create the wrapped component per-test to avoid cross-test leakage
const _App = withInitDataInState(App);is declared once for the wholedescribeblock, yet the classAppcapturesappinto a shared variable.
IfwithInitDataInStatestores static metadata, reusing the same wrapper can leak listeners between tests.
Move the wrapping into eachitor a freshbeforeEach:-const _App = withInitDataInState(App); -it('should inject …', () => { - render(<_App />, scratch); +it('should inject …', () => { + const Wrapped = withInitDataInState(App); + render(<Wrapped />, scratch);This keeps every test fully isolated.
134-135: Use.not.toEqualfor clarityChai’s
not.eqworks, but the more idiomatic Vitest/Jest style reads better for objects:-expect(app.state).not.eq(lynx.__initData); +expect(app.state).not.toEqual(lynx.__initData);Pure readability nit – ignore if the team prefers the Chai alias.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/free-colts-listen.md(1 hunks)packages/react/runtime/__test__/compat/initData.test.jsx(2 hunks)packages/react/runtime/src/compat/initData.ts(1 hunks)packages/react/runtime/src/lynx/tt.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react/runtime/src/lynx/tt.ts
- .changeset/free-colts-listen.md
- packages/react/runtime/src/compat/initData.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
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/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-07-18T04:27:18.291Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Applied to files:
packages/react/runtime/__test__/compat/initData.test.jsx
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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/runtime/__test__/compat/initData.test.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: zizmor
hzy
approved these changes
Aug 22, 2025
colinaaa
pushed a commit
that referenced
this pull request
Aug 26, 2025
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - fix `withInitDataInState` got wrong state in 2nd or more times `defaultDataProcessor`, now it will keep its own state. ([#1478](#1478)) - change `__CreateElement('raw-text')` to `__CreateRawText('')` to avoid `setNativeProps` not working ([#1570](#1570)) - Fix wrong render result when using expression as `key`. ([#1541](#1541)) See [#1371](#1371) for more details. - fix: `Cannot read properties of undefined` error when using `Suspense` ([#1569](#1569)) - Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively ([#1534](#1534)) ```ts import type { MainThread } from "@lynx-js/types"; function startAnimation(ele: MainThread.Element) { "main thread"; const animation = ele.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 3000, }); // Can also be paused // animation.pause() } ``` ## @lynx-js/[email protected] ### Patch Changes - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Support exporting `Promise` and function in `lynx.config.ts`. ([#1590](#1590)) - Fix missing `publicPath` using when `rspeedy dev --mode production`. ([#1310](#1310)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## [email protected] ### Patch Changes - Add `@lynx-js/preact-devtools` by default. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Bump @clack/prompts to v1.0.0-alpha.4 ([#1559](#1559)) ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Updated dependencies \[[`f0d483c`](f0d483c), [`e4d116b`](e4d116b), [`d33c1d2`](d33c1d2)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Alias `@lynx-js/preact-devtools` to `false` to reduce an import of empty webpack module. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Support `lynx.createSelectorQuery().select()` and `setNativeProps` API ([#1570](#1570)) ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: fake uidisappear event ([#1539](#1539)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add new `LynxCacheEventsPlugin`, which will cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Always inline the background script that contains rspack runtime module. ([#1582](#1582)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add `lynxCacheEventsSetupList` and `lynxCacheEvents` to RuntimeGlobals. It will be used to cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Checklist