feat: add onBackgroundSnapshotInstanceUpdateId event for Preact Devtools
#1173
Conversation
🦋 Changeset detectedLatest commit: efbff2c 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 |
upupming
force-pushed
the
feat/preact-devtools
branch
from
8484b3d to
2283c32
Compare
upupming
force-pushed
the
feat/preact-devtools
branch
from
ddabe23 to
823263c
Compare
CodSpeed Performance ReportMerging #1173 will not alter performanceComparing Summary
|
upupming
force-pushed
the
feat/preact-devtools
branch
from
d2a0b2f to
0da045e
Compare
React Example#3693 Bundle Size — 235.14KiB (0%).4e20e9a(current) vs 44f5b0d main#3680(baseline) Bundle metrics
|
| Current #3693 |
Baseline #3680 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
156 |
156 |
| Duplicate Modules |
63 |
63 |
| Duplicate Code |
45.94% |
45.94% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #3693 |
Baseline #3680 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
89.38KiB |
89.38KiB |
Bundle analysis report Branch upupming:feat/preact-devtools Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#3683 Bundle Size — 342.17KiB (0%).4e20e9a(current) vs 44f5b0d main#3671(baseline) Bundle metrics
|
| Current #3683 |
Baseline #3671 |
|
|---|---|---|
| Initial JS |
142.14KiB |
142.14KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
7 |
7 |
| Assets |
7 |
7 |
 Modules |
211(-0.47%) |
212 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
4.02% |
4.02% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #3683 |
Baseline #3671 |
|
|---|---|---|
| JS |
227.22KiB |
227.22KiB |
| Other |
83.11KiB |
83.11KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch upupming:feat/preact-devtools Project dashboard
Generated by RelativeCI Documentation Report issue
upupming
force-pushed
the
feat/preact-devtools
branch
from
806c35c to
03f911e
Compare
❌ 6 Tests Failed:
View the top 2 failed test(s) by shortest run time
View the full list of 1 ❄️ flaky tests
To view more test analytics, go to the Test Analytics Dashboard |
📝 WalkthroughWalkthroughA new development-only event, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
|
upupming
force-pushed
the
feat/preact-devtools
branch
from
75d8d95 to
cd454f2
Compare
upupming
force-pushed
the
feat/preact-devtools
branch
from
cd454f2 to
e75603c
Compare
upupming
changed the title
onBackgroundSnapshotInstanceUpdateId event for Preact Devtools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/testing-library/testing-environment/src/env/vitest/index.ts (1)
19-21: Consider removing obsolete global.jsdom cleanup.The teardown function still deletes
global.jsdom, but this global is no longer being set during setup. Consider removing this line to avoid confusion.teardown(global) { delete global.lynxTestingEnv; - delete global.jsdom; },packages/react/runtime/src/snapshot.ts (1)
152-162: LGTM! Well-implemented development event emission.The conditional event emission for Preact Devtools is properly implemented:
- Correctly gated behind
__DEV__to avoid production overhead- Emits before state changes so devtools receive current state
- Includes comprehensive event payload with all necessary information
- Clear comment explaining the purpose
Consider adding error handling for the event emitter access:
if (__DEV__) { - lynx.getJSModule('GlobalEventEmitter').emit('onBackgroundSnapshotInstanceUpdateId', [ + try { + lynx.getJSModule('GlobalEventEmitter')?.emit('onBackgroundSnapshotInstanceUpdateId', [ { backgroundSnapshotInstance: si, oldId: id, newId, }, ]); + } catch (e) { + // Ignore if GlobalEventEmitter is not available + } }packages/react/runtime/__test__/utils/globals.js (1)
81-81: Consider a more descriptive variable name.The EventEmitter instance creation is correct, but
eecould be more descriptive for better code readability.-const ee = new EventEmitter(); +const globalEventEmitter = new EventEmitter();Then update the reference in the getJSModule method:
if (moduleName === 'GlobalEventEmitter') { - return ee; + return globalEventEmitter; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.changeset/lucky-boats-rhyme.md(1 hunks).changeset/rare-mice-leave.md(1 hunks)packages/react/runtime/__test__/utils/globals.js(3 hunks)packages/react/runtime/src/snapshot.ts(1 hunks)packages/testing-library/testing-environment/README.md(1 hunks)packages/testing-library/testing-environment/etc/test-environment.api.md(0 hunks)packages/testing-library/testing-environment/etc/testing-environment.api.md(1 hunks)packages/testing-library/testing-environment/src/env/vitest/index.ts(2 hunks)packages/testing-library/testing-environment/src/index.ts(4 hunks)packages/webpack/react-refresh-webpack-plugin/test/hotCases/hook/useContext/index.jsx(1 hunks)packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/value/index.jsx(1 hunks)packages/webpack/react-refresh-webpack-plugin/test/setup-env.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/testing-library/testing-environment/etc/test-environment.api.md
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
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.
.changeset/rare-mice-leave.md (2)
Learnt from: colinaaa
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.
Learnt from: colinaaa
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.
.changeset/lucky-boats-rhyme.md (2)
Learnt from: colinaaa
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.
Learnt from: colinaaa
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.
packages/webpack/react-refresh-webpack-plugin/test/setup-env.js (1)
Learnt from: PupilTong
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.
packages/testing-library/testing-environment/src/index.ts (3)
Learnt from: colinaaa
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.
Learnt from: colinaaa
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.
Learnt from: PupilTong
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.
packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/value/index.jsx (4)
Learnt from: colinaaa
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.
Learnt from: colinaaa
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.
Learnt from: PupilTong
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.
Learnt from: PupilTong
PR: #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.
packages/webpack/react-refresh-webpack-plugin/test/hotCases/hook/useContext/index.jsx (1)
Learnt from: PupilTong
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.
🧬 Code Graph Analysis (1)
packages/webpack/react-refresh-webpack-plugin/test/setup-env.js (1)
packages/web-platform/web-constants/src/types/NativeApp.ts (1)
EventEmitter(23-42)
⏰ 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: Validate PR title
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (16)
.changeset/rare-mice-leave.md (1)
1-6: LGTM! Clear changeset for meaningful bugfix.The changeset appropriately documents a patch-level fix for the JSDOM initialization issue. The description clearly explains the problem and impact for end-users.
packages/testing-library/testing-environment/README.md (1)
11-13: LGTM! Documentation correctly reflects API changes.The README example has been properly updated to show the new constructor signature requiring an explicit JSDOM instance, making the API usage clear for developers.
packages/testing-library/testing-environment/etc/testing-environment.api.md (1)
91-91: LGTM! API report correctly documents constructor signature change.The generated API report accurately reflects the new constructor signature requiring a JSDOM parameter.
packages/testing-library/testing-environment/src/env/vitest/index.ts (2)
3-3: LGTM! Proper import added for explicit JSDOM usage.
14-14: LGTM! Constructor correctly updated to pass JSDOM instance.The change properly passes the JSDOM instance from the vitest environment setup to the LynxTestingEnv constructor.
packages/testing-library/testing-environment/src/index.ts (4)
411-411: LGTM! JSDoc example correctly updated.The constructor example in the JSDoc properly demonstrates the new API requiring a JSDOM instance.
433-433: LGTM! JSDoc example correctly updated.
449-449: LGTM! JSDoc example correctly updated.
459-462: LGTM! Constructor properly refactored to require explicit JSDOM.The constructor signature change correctly addresses the initialization issue by:
- Making the JSDOM dependency explicit through the parameter
- Storing the JSDOM instance for later use
- Removing reliance on potentially undefined global.jsdom
This is a solid fix that improves reliability and makes the API more explicit.
packages/webpack/react-refresh-webpack-plugin/test/hotCases/hook/useContext/index.jsx (1)
19-23: LGTM! Simplified mocking approach.The change from
vi.stubGlobalto direct assignment oflynx.getNativeAppsimplifies the test setup while maintaining the same functionality. This approach is cleaner and more direct..changeset/lucky-boats-rhyme.md (1)
1-6: LGTM! Clear changeset documentation.The changeset appropriately documents the new development event for Preact Devtools integration. The description clearly explains the purpose and context for the change.
packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/value/index.jsx (1)
16-20: LGTM! Consistent mocking approach.This change aligns with the pattern used in other test files, replacing
vi.stubGlobalwith direct assignment tolynx.getNativeApp. The approach is cleaner and maintains consistency across the test suite.packages/webpack/react-refresh-webpack-plugin/test/setup-env.js (2)
4-6: LGTM! EventEmitter setup for development events.The addition of Node.js EventEmitter creates the necessary infrastructure for the new development-time event system.
15-21: LGTM! Clean global EventEmitter integration.The
getJSModulemethod properly provides access to the EventEmitter instance for the 'GlobalEventEmitter' module, enabling the new development event system while maintaining a clean API.packages/react/runtime/__test__/utils/globals.js (2)
5-5: LGTM! Clean EventEmitter import.The import uses the modern
node:protocol prefix, which is the recommended approach for Node.js built-in modules.
130-134: No action needed:getJSModuleoverride fully covers test usageThe overridden
getJSModuleonly needs to handle"GlobalEventEmitter"because:
jsModule.tsdefines no other modules, and- all test code invokes
getJSModule('GlobalEventEmitter')exclusively.No fallback to the original implementation is required.
Likely an incorrect or invalid review comment.
upupming
force-pushed
the
feat/preact-devtools
branch
from
9a9031f to
460f586
Compare
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 - Supports `recyclable` attribute in `<list-item>` to control whether the list item is recyclable. The `recyclable` attribute depends on Lynx Engine 3.4 or later. ([#1388](#1388)) ```jsx <list-item recyclable={false} /> ``` - feat: Support using a host element as direct child of Suspense ([#1455](#1455)) - Add profile in production build: ([#1336](#1336)) 1. `diff:__COMPONENT_NAME__`: how long ReactLynx diff took. 2. `render:__COMPONENT_NAME__`: how long your render function took. 3. `setState`: an instant trace event, indicate when your setState was called. NOTE: `__COMPONENT_NAME__` may be unreadable when minified, setting `displayName` may help. - Add `onBackgroundSnapshotInstanceUpdateId` event on dev for Preact Devtools to keep the correct snapshotInstanceId info. ([#1173](#1173)) - fix: Prevent error when spreading component props onto an element ([#1459](#1459)) - fix: Correctly check for the existence of background functions in MTS ([#1416](#1416)) ```ts function handleTap() { "main thread"; // The following check always returned false before this fix if (myHandleTap) { runOnBackground(myHandleTap)(); } } ``` ## @lynx-js/[email protected] ### Patch Changes - Remove the experimental `provider` option. ([#1432](#1432)) - Add `output.filename.wasm` and `output.filename.assets` options. ([#1449](#1449)) - fix deno compatibility ([#1412](#1412)) - Should call the `api.onCloseBuild` hook after the build finished. ([#1446](#1446)) - Bump Rsbuild v1.4.15. ([#1423](#1423)) - Support using function in `output.filename.*`. ([#1449](#1449)) ## [email protected] ### Patch Changes - Support ESLint for ReactLynx templates ([#1274](#1274)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`c8ce6aa`](c8ce6aa)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the `Package subpath './compat' is not defined by "exports"` error. ([#1460](#1460)) ## @lynx-js/[email protected] ### Patch Changes - Fix `GlobalEventEmitter` type definition, the `emit(eventName: string, data: unknown)` function should recevie an array typed `data` and pass as param list of listeners. ([#1479](#1479)) ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - fix: when `onNativeModulesCall` is delayed in mounting, the NativeModules execution result may be undefined. ([#1457](#1457)) - fix: `onNativeModulesCall` && `onNapiModulesCall` use getter to get. ([#1466](#1466)) - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. ## @lynx-js/[email protected] ### Patch Changes - feat: add autocomplete attribute support for x-input component ([#1444](#1444)) Implements autocomplete attribute forwarding from the x-input custom element to the internal HTML input element in the shadow DOM. This enables standard browser autocomplete functionality for x-input elements. - Add referrerpolicy attribute support to x-image web component ([#1420](#1420)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: load main-thread chunk in ESM format ([#1437](#1437)) See [nodejs/node#59362](nodejs/node#59362) for more details. - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support path() for `createQuerySelector` ([#1456](#1456)) - Added `getPathInfo` API to `NativeApp` and its cross-thread handler for retrieving the path from a DOM node to the root. - Implemented endpoint and handler registration in both background and UI threads. - Implemented `nativeApp.getPathInfo()` - Updated dependencies \[[`29434ae`](29434ae), [`fb7096b`](fb7096b)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## [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>
3 participants
Summary
We are adding Preact Devtools Support for ReactLynx, this PR contains related changes for PreactDevtools to work properly.
After this we can inspect ReactLynx App in LynxDevtool just like any Preact Web App:
Related PR: lynx-family/lynx-devtool#56
Checklist
Summary by CodeRabbit
New Features
Tests