refactor(runtime): share initData commit context#2677
Conversation
|
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 (8)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughConsolidates runtime flush-option management into a shared global commit context, migrates initData wiring to core, exposes InitData provider/consumer/hooks from the element-template entrypoint, tightens op handling, updates types for a triggerDataUpdated flush flag, removes test shims, and adds tests and Vitest/TS aliases. ChangesElement Template InitData Public API with Shared Commit Context
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 docstrings
🧪 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 |
🦋 Changeset detectedLatest commit: 0f3c6ce The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/runtime/src/element-template/hooks/react.ts`:
- Around line 63-79: The code currently performs GlobalEventEmitter subscription
side effects inside useMemo (previousArgsRef, eventName, listener,
lynx.getJSModule('GlobalEventEmitter')), which is unsafe; move the
addListener/removeListener logic into a useEffect: remove the useMemo block
entirely, create a useEffect that adds the listener
(lynx.getJSModule('GlobalEventEmitter').addListener(eventName, listener)) when
[eventName, listener] change, store the args in previousArgsRef, and return a
cleanup that removes the previous listener via removeListener using
previousArgsRef; keep the existing teardown-only useEffect (or merge it) so no
subscriptions occur during render and all listeners are removed on unmount or
dependency change.
🪄 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: 034ebfa3-1e19-4833-a142-fa514585d186
📒 Files selected for processing (22)
.changeset/empty-et-init-data-core.mdpackages/react/runtime/__test__/core/commit-context.test.tspackages/react/runtime/__test__/element-template/imports/init-data-api.test.tspackages/react/runtime/__test__/element-template/runtime/background/commit-hook.test.tspackages/react/runtime/__test__/element-template/runtime/patch/element-template-patch.test.tsxpackages/react/runtime/__test__/element-template/test-utils/debug/layeredHooks.tspackages/react/runtime/__test__/element-template/test-utils/debug/layeredReact.tspackages/react/runtime/__test__/element-template/vitest.config.tspackages/react/runtime/__test__/snapshot/commit-context.test.tspackages/react/runtime/src/core/commit-context.tspackages/react/runtime/src/core/initData.tspackages/react/runtime/src/element-template/background/commit-context.tspackages/react/runtime/src/element-template/background/commit-hook.tspackages/react/runtime/src/element-template/hooks/react.tspackages/react/runtime/src/element-template/index.tspackages/react/runtime/src/element-template/internal.tspackages/react/runtime/src/element-template/native/patch-listener.tspackages/react/runtime/src/element-template/protocol/types.tspackages/react/runtime/src/internal.tspackages/react/runtime/src/lynx-api.tspackages/react/runtime/src/snapshot/lifecycle/patch/commit.tspackages/react/runtime/src/snapshot/lynx/component.ts
💤 Files with no reviewable changes (2)
- packages/react/runtime/test/element-template/test-utils/debug/layeredReact.ts
- packages/react/runtime/test/element-template/test-utils/debug/layeredHooks.ts
Merging this PR will improve performance by 12.03%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
44.8 ms | 40 ms | +12.03% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing wt/lynx-stack-20260520-1406 (0f3c6ce) with main (b42a7dc)
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 MTF Example#1632 Bundle Size — 208.74KiB (+0.03%).0f3c6ce(current) vs b42a7dc main#1627(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch wt/lynx-stack-20260520-1406 Project dashboard Generated by RelativeCI Documentation Report issue |
React Example with Element Template#768 Bundle Size — 201.17KiB (-1.64%).0f3c6ce(current) vs b42a7dc main#763(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch wt/lynx-stack-20260520-1406 Project dashboard Generated by RelativeCI Documentation Report issue |
React External#1614 Bundle Size — 697.99KiB (+0.01%).0f3c6ce(current) vs b42a7dc main#1609(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch wt/lynx-stack-20260520-1406 Project dashboard Generated by RelativeCI Documentation Report issue |
Web Explorer#10073 Bundle Size — 903.53KiB (0%).0f3c6ce(current) vs b42a7dc main#10068(baseline) Bundle metrics
|
| Current #10073 |
Baseline #10068 |
|
|---|---|---|
45.06KiB |
45.06KiB |
|
2.22KiB |
2.22KiB |
|
0% |
0% |
|
9 |
9 |
|
11 |
11 |
|
231 |
231 |
|
11 |
11 |
|
27.12% |
27.12% |
|
10 |
10 |
|
0 |
0 |
Bundle size by type no changes
| Current #10073 |
Baseline #10068 |
|
|---|---|---|
499.15KiB |
499.15KiB |
|
402.16KiB |
402.16KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch wt/lynx-stack-20260520-1406 Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8499 Bundle Size — 237.79KiB (+0.02%).0f3c6ce(current) vs b42a7dc main#8494(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch wt/lynx-stack-20260520-1406 Project dashboard Generated by RelativeCI Documentation Report issue |
Summary by CodeRabbit
New Features
Behavioral Improvements
Tests
Overview
Element Template needs initData APIs without depending on Snapshot-private runtime state. This PR moves the initData factory and shared commit context into
core, then wires Snapshot and Element Template through that shared state so data-change flush options can be produced consistently.Key Points
snapshot/compatintocoreand re-exports the APIs from both the existing React runtime entry and the Element Template entry.globalCommitContextthroughcore, while keeping ET-only teardown state out of the cross-thread payload.{ ops: [], flushOptions: { triggerDataUpdated: true } }.@lynx-js/react/hooksentry for ET hooks; existing layer/alias rules resolve it to background or main-thread hook implementations without an ET-only hook facade.Runtime Contract
opswhen onlyflushOptionsneed to reach the main thread.opsis missing or empty, but still calls__FlushElementTree(__page, flushOptions).triggerDataUpdatednow share the same core commit-context storage, with each runtime consuming/resetting it at its own commit boundary.Checklist