fix(react/runtime): prevent reloadVersion from cancelling the MTRef initial value patch#1757
Conversation
🦋 Changeset detectedLatest commit: 102b4a1 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 |
📝 WalkthroughWalkthroughSends worklet ref init values to the main thread via a new lifecycle event Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
Yradex
force-pushed
the
fix/mtf-reload5
branch
from
df10ef3 to
8e8cf46
Compare
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/react/runtime/src/worklet/ref/workletRefPool.ts (1)
7-7: Type alias naming nitConsider PascalCase for exported types:
WorkletRefInitValuePatchto match TS conventions.Apply locally:
-export type workletRefInitValuePatch = [id: number, value: unknown][]; +export type WorkletRefInitValuePatch = [id: number, value: unknown][];And update imports/usages accordingly.
packages/react/runtime/src/lynx/tt.ts (1)
131-134: Send init values before patchUpdate — good for decoupling from reloadVersionPlacing
sendMTRefInitValueToMainThread()beforepatchUpdateensures init values aren’t dropped byreloadVersionchecks. Consider attaching correlation data (e.g.,commitTaskIdorflowIds) for tracing across platforms, without gating application.Example (requires small change in updateInitValue.ts):
- sendMTRefInitValueToMainThread(); + sendMTRefInitValueToMainThread({ commitTaskId, flowIds: patchList?.flowIds });And adjust the sender to include this metadata for logging only.
packages/react/runtime/src/lifecycle/patch/commit.ts (1)
137-138: Unconditionally sending MT ref init values — validate semanticsSending before building/sending the snapshot patch ensures values won’t be canceled by
reloadVersion. Validate there’s no scenario where init values could apply to a different commit on the main thread when calls are re-ordered by the bridge. If needed, include correlation metadata (not gating).Optional tweak (non-blocking): pass a lightweight context to the sender for logging/trace correlation.
- sendMTRefInitValueToMainThread(); + sendMTRefInitValueToMainThread(/* { commitTaskId } */);packages/react/runtime/__test__/snapshot/workletRef.test.jsx (2)
304-307: Also assert event ordering for robustnessVerifying the event names prevents accidental regressions in call order.
Apply:
- let rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0]; - globalThis[rLynxChange[0]](rLynxChange[1]); - rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[1]; - globalThis[rLynxChange[0]](rLynxChange[1]); + const calls = lynx.getNativeApp().callLepusMethod.mock.calls; + expect(calls[0][0]).toBe('rLynxChangeRefInitValue'); + globalThis[calls[0][0]](calls[0][1]); + expect(calls[1][0]).toBe('rLynxChange'); + globalThis[calls[1][0]](calls[1][1]);
498-501: Assert the two-step insert flow explicitlyMirror the hydrate section: check first call is rLynxChangeRefInitValue, second is rLynxChange.
Apply:
- let rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0]; - globalThis[rLynxChange[0]](rLynxChange[1]); - rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[1]; - globalThis[rLynxChange[0]](rLynxChange[1]); + const calls = lynx.getNativeApp().callLepusMethod.mock.calls; + expect(calls[0][0]).toBe('rLynxChangeRefInitValue'); + globalThis[calls[0][0]](calls[0][1]); + expect(calls[1][0]).toBe('rLynxChange'); + globalThis[calls[1][0]](calls[1][1]);packages/react/runtime/__test__/worklet/workletRef.test.jsx (1)
19-22: Avoid unused variable lint in CompPrefix the local ref to signal intentional non-use.
Apply:
-const Comp = () => { - const ref = useMainThreadRef(233); +const Comp = () => { + const _ref = useMainThreadRef(233); return <view></view>; };packages/react/runtime/src/worklet/ref/updateInitValue.ts (2)
11-16: Harden payload parsing to avoid crashing on malformed dataA defensive parse makes the handler resilient to unexpected inputs.
Apply:
-function updateMTRefInitValue({ data }: { data: string }): void { - // This update ignores reloadVersion check. - // MainThreadRefs created before reloadTemplate may still be referenced by user in some cases after reloadTemplate. - const patch = JSON.parse(data) as workletRefInitValuePatch; - updateWorkletRefInitValueChanges(patch); -} +function updateMTRefInitValue({ data }: { data: string }): void { + // This update ignores reloadVersion check. + // MainThreadRefs created before reloadTemplate may still be referenced by user after reloadTemplate. + try { + const patch = JSON.parse(data) as workletRefInitValuePatch; + if (Array.isArray(patch)) { + updateWorkletRefInitValueChanges(patch); + } + } catch { + // ignore malformed payloads + } +}
22-30: Guard native bridge availabilityAvoid throwing when lynx or getNativeApp isn’t ready (older SDKs/tests).
Apply:
export function sendMTRefInitValueToMainThread(): void { const patch = takeWorkletRefInitValuePatch(); if (patch.length === 0) { return; } const data = JSON.stringify(patch); - lynx.getNativeApp().callLepusMethod(LifecycleConstant.updateMTRefInitValue, { data }); + const app = lynx?.getNativeApp?.(); + if (app?.callLepusMethod) { + app.callLepusMethod(LifecycleConstant.updateMTRefInitValue, { data }); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.changeset/violet-wasps-shop.md(1 hunks)packages/react/runtime/__test__/snapshot/workletRef.test.jsx(3 hunks)packages/react/runtime/__test__/worklet/workletRef.test.jsx(3 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lifecycleConstant.ts(1 hunks)packages/react/runtime/src/lynx.ts(2 hunks)packages/react/runtime/src/lynx/tt.ts(2 hunks)packages/react/runtime/src/worklet/ref/updateInitValue.ts(1 hunks)packages/react/runtime/src/worklet/ref/workletRefPool.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/violet-wasps-shop.md
🧠 Learnings (7)
📚 Learning: 2025-09-12T09:43:04.810Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
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/violet-wasps-shop.md
📚 Learning: 2025-09-12T09:43:04.810Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
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/violet-wasps-shop.mdpackages/react/runtime/src/lynx.ts
📚 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/violet-wasps-shop.md
📚 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/violet-wasps-shop.md
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/runtime/src/lynx.tspackages/react/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/runtime/src/lynx.tspackages/react/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
🧬 Code graph analysis (6)
packages/react/runtime/src/lynx/tt.ts (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
sendMTRefInitValueToMainThread(22-30)
packages/react/runtime/src/lynx.ts (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
packages/react/runtime/src/worklet/ref/workletRefPool.ts (2)
workletRefInitValuePatch(7-7)takeWorkletRefInitValuePatch(27-31)
packages/react/runtime/src/lifecycle/patch/commit.ts (2)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
sendMTRefInitValueToMainThread(22-30)packages/react/runtime/src/lifecycle/patch/snapshotPatch.ts (1)
takeGlobalSnapshotPatch(55-63)
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (2)
packages/react/runtime/src/snapshot.ts (1)
setupPage(46-49)packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
packages/react/runtime/__test__/worklet/workletRef.test.jsx (2)
packages/react/runtime/src/worklet/ref/workletRef.ts (1)
useMainThreadRef(227-231)packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
⏰ 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). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (20)
.changeset/violet-wasps-shop.md (1)
1-6: Changeset is correct and minimalPatch bump for @lynx-js/react with a concise note is appropriate for this bugfix. Nothing else needed.
packages/react/runtime/src/worklet/ref/workletRefPool.ts (1)
27-31: Return type aligns with aliasSignature change is consistent. No runtime impact.
packages/react/runtime/src/lynx.ts (2)
21-21: Import of MT ref init injectorImport path and placement look consistent with the module layout.
33-40: ```shell
#!/bin/bash
set -euo pipefailecho "=== locate injectUpdateMTRefInitValue ==="
rg -n --hidden "injectUpdateMTRefInitValue" -S packages || trueecho
echo "=== show file that defines injectUpdateMTRefInitValue ==="
file=$(rg -n --hidden "function\s+injectUpdateMTRefInitValue|const\s+injectUpdateMTRefInitValue" -S packages -l || true)
if [ -n "$file" ]; then
sed -n '1,240p' "$file"
else
echo "no definition found"
fiecho
echo "=== show packages/react/runtime/src/lynx.ts (first 300 lines) ==="
sed -n '1,300p' packages/react/runtime/src/lynx.ts || trueecho
echo "=== show updateInitValue.ts (full) ==="
sed -n '1,240p' packages/react/runtime/src/worklet/ref/updateInitValue.ts || trueecho
echo "=== search for LifecycleConstant ==="
rg -n --hidden "LifecycleConstant" -S packages || trueecho
echo "=== show files that mention callLepusMethod or getNativeApp ==="
rg -n --hidden "callLepusMethod" -S packages || true
rg -n --hidden "getNativeApp" -S packages || trueecho
echo "=== search for rLynxChangeRefInitValue ==="
rg -n --hidden "rLynxChangeRefInitValue" -S packages || true</blockquote></details> <details> <summary>packages/react/runtime/src/lynx/tt.ts (1)</summary><blockquote> `29-30`: **Import of sender** Import is correct and localizes the new responsibility here. </blockquote></details> <details> <summary>packages/react/runtime/src/lifecycle/patch/updateMainThread.ts (2)</summary><blockquote> `7-7`: **Removed init-value processing import** Cleanup looks correct after moving init values to a dedicated channel. --- `49-57`: **Loop now applies only snapshot patches** Removal of `workletRefInitValuePatch` handling is consistent with the new flow. No functional concerns. </blockquote></details> <details> <summary>packages/react/runtime/src/lifecycle/patch/commit.ts (4)</summary><blockquote> `32-33`: **New dependency on init-value sender** Import is correct for the new channel. --- `143-149`: **Early return path remains correct** If there’s no `snapshotPatch`, we still send init values and then exit, which matches the intent to decouple. Good. --- `186-209`: **Packaging of patch options unchanged** `reloadVersion` continues to apply only to `patchUpdate`. This aligns with the PR goal. --- `239-249`: **Exports remain stable** Public surface for patching helpers looks consistent after the refactor. </blockquote></details> <details> <summary>packages/react/runtime/src/lifecycleConstant.ts (1)</summary><blockquote> `11-12`: **Confirm native (iOS/Android/Lepus) handler for updateMTRefInitValue** - JS registers/calls LifecycleConstant.updateMTRefInitValue and tests reference "rLynxChangeRefInitValue" (files: packages/react/runtime/src/lifecycleConstant.ts; packages/react/runtime/src/worklet/ref/updateInitValue.ts — registers on globalThis and calls lynx.getNativeApp().callLepusMethod; tests in packages/react/runtime/__test__/worklet/workletRef.test.jsx and packages/react/runtime/__test__/snapshot/workletRef.test.jsx). - I did not find native/engine-side handler registrations for 'rLynxChangeRefInitValue' in this repo — ensure iOS, Android and Lepus register a handler for this key so calls aren’t dropped. </blockquote></details> <details> <summary>packages/react/runtime/__test__/snapshot/workletRef.test.jsx (3)</summary><blockquote> `17-17`: **Enable MT ref init-value injection in snapshots** Import looks correct and localizes the behavior to tests. --- `22-22`: **Call the injector before replaceCommitHook** Good ordering; ensures the handler is present before any patches fire. --- `273-276`: **Snapshot now asserts the dedicated init‑value event** Locking on rLynxChangeRefInitValue and its stringified payload is appropriate and exercises the new path. </blockquote></details> <details> <summary>packages/react/runtime/__test__/worklet/workletRef.test.jsx (4)</summary><blockquote> `17-17`: **Inject init‑value handler in worklet tests** Import is correct and aligns with the new lifecycle event. --- `27-28`: **Initialize handlers before patching** Good: injectUpdateMTRefInitValue precedes replaceCommitHook. --- `104-107`: **Correct: the init‑value data is isolated from rLynxChange** Asserting rLynxChangeRefInitValue with "[[1,233]]" validates the decoupled path. --- `218-279`: **Reload template scenario covered — wiring verified** Test proves init values still dispatch (reloadVersion=1) and aren’t double-applied; verified wiring exists: lifecycleConstant defines updateMTRefInitValue = 'rLynxChangeRefInitValue' (packages/react/runtime/src/lifecycleConstant.ts:11), injectUpdateMTRefInitValue() is invoked during main-thread init (packages/react/runtime/src/lynx.ts:34–36), and sendMTRefInitValueToMainThread() is called before the patch update in hydration (packages/react/runtime/src/lynx/tt.ts:132). </blockquote></details> <details> <summary>packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)</summary><blockquote> `18-20`: **Global injection looks good** Idempotent and consistent with existing injector patterns. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
React Example#5240 Bundle Size — 237.73KiB (+0.09%).102b4a1(current) vs 588819b main#5234(baseline) Bundle metrics
|
| Current #5240 |
Baseline #5234 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
38.63% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
165(+1.85%) |
162 |
 Duplicate Modules |
67(+3.08%) |
65 |
| Duplicate Code |
46.76%(+0.11%) |
46.71% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #5240 |
Baseline #5234 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.97KiB (+0.22%) |
91.76KiB |
Bundle analysis report Branch Yradex:fix/mtf-reload5 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5233 Bundle Size — 364.39KiB (0%).102b4a1(current) vs 588819b main#5227(baseline) Bundle metrics
|
| Current #5233 |
Baseline #5227 |
|
|---|---|---|
| Initial JS |
145.41KiB |
145.41KiB |
| Initial CSS |
31.89KiB |
31.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
220(+0.46%) |
219 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.37% |
3.37% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5233 |
Baseline #5227 |
|
|---|---|---|
| JS |
238.48KiB |
238.48KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch Yradex:fix/mtf-reload5 Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1757 will improve performances by 7.33%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
46.2 ms | 43 ms | +7.33% |
Yradex
force-pushed
the
fix/mtf-reload5
branch
from
8e8cf46 to
4806e78
Compare
… initial value patch
Yradex
force-pushed
the
fix/mtf-reload5
branch
from
4806e78 to
102b4a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/testing-library/testing-environment/src/index.ts (1)
96-110: Make thread restoration exception-safe and guard callback typeGood null-guard. Add try/finally so thread state is always restored, and avoid TypeError if a non-function sneaks in.
- callLepusMethod: (...rLynxChange: any[]) => { - const isBackground = !__MAIN_THREAD__; - - globalThis.lynxTestingEnv.switchToMainThread(); - globalThis[rLynxChange[0]](rLynxChange[1]); - - globalThis.lynxTestingEnv.switchToBackgroundThread(); - rLynxChange[2]?.(); - globalThis.lynxTestingEnv.switchToMainThread(); - - // restore the original thread state - if (isBackground) { - globalThis.lynxTestingEnv.switchToBackgroundThread(); - } - }, + callLepusMethod: (...rLynxChange: any[]) => { + const wasBackground = !__MAIN_THREAD__; + try { + globalThis.lynxTestingEnv.switchToMainThread(); + globalThis[rLynxChange[0]](rLynxChange[1]); + globalThis.lynxTestingEnv.switchToBackgroundThread(); + if (typeof rLynxChange[2] === 'function') { + rLynxChange[2](); + } + } finally { + if (wasBackground) { + globalThis.lynxTestingEnv.switchToBackgroundThread(); + } else { + globalThis.lynxTestingEnv.switchToMainThread(); + } + } + },
🧹 Nitpick comments (7)
packages/react/runtime/src/lifecycle/patch/updateMainThread.ts (3)
66-77: Always restore EOM flush and avoid shadowing parameter name.If anything throws outside the per‑task try/catch, EOM flush could remain disabled. Also, the loop variable shadows the outer
dataparam.Apply this diff:
if (delayedRunOnMainThreadData && isMtsEnabled()) { - setEomShouldFlushElementTree(false); - for (const data of delayedRunOnMainThreadData) { - try { - runRunOnMainThreadTask(data.worklet, data.params as ClosureValueType[], data.resolveId); - /* v8 ignore next 3 */ - } catch (e) { - lynx.reportError(e as Error); - } - } - setEomShouldFlushElementTree(true); + setEomShouldFlushElementTree(false); + try { + for (const task of delayedRunOnMainThreadData) { + try { + runRunOnMainThreadTask(task.worklet, task.params as ClosureValueType[], task.resolveId); + /* v8 ignore next 3 */ + } catch (e) { + lynx.reportError(e as Error); + } + } + } finally { + setEomShouldFlushElementTree(true); + } }
31-35: Small profiling guards to avoid undefined/empty flowIds usage.Protect profileStart/profileEnd when flowIds is empty.
- if (flowIds) { + if (flowIds && flowIds.length > 0) { lynx.performance.profileStart('ReactLynx::patch', { flowId: flowIds[0], // @ts-expect-error flowIds is not defined in the type, for now flowIds, }); } @@ - if (flowIds) { + if (flowIds && flowIds.length > 0) { lynx.performance.profileEnd(); }Also applies to: 83-85
58-66: Consider running applyRefQueue in a finally to avoid ref leaks on patch exceptions.If snapshotPatchApply throws, refs may remain unflushed. Move applyRefQueue() into the finally block after hydration flag reset.
Would you like a follow‑up diff for this if tests permit?
packages/testing-library/testing-environment/src/index.ts (1)
103-104: Nice: avoid calling missing ackThe optional call aligns with new rLynxChangeRefInitValue events that omit the ack. Consider the typeof check above for extra safety.
packages/react/testing-library/src/__tests__/worklet.test.jsx (1)
442-445: Prefer structured assertions over string snapshots for init‑value payloadAsserting
datavia JSON string snapshot is brittle. Parse and assert the shape to reduce flakiness.Example:
const [evtName, payload] = callLepusMethodCalls[0]; expect(evtName).toBe('rLynxChangeRefInitValue'); expect(JSON.parse(payload.data)).toEqual([[1, null], [2, 0]]);packages/react/runtime/__test__/snapshot/workletRef.test.jsx (2)
273-277: Strengthen expectations: assert event type and decode payloadYou already snapshot the two calls. Add explicit checks so regressions (e.g., event name/shape) fail loudly.
Add after the snapshot:
expect(lynx.getNativeApp().callLepusMethod).toHaveBeenCalledTimes(2); const [initEvt, patchEvt] = lynx.getNativeApp().callLepusMethod.mock.calls; expect(initEvt[0]).toBe('rLynxChangeRefInitValue'); expect(() => JSON.parse(initEvt[1].data)).not.toThrow();Optionally also verify the handler was invoked:
expect(globalThis.lynxWorkletImpl._refImpl.updateWorkletRefInitValueChanges).toHaveBeenCalled();
497-501: Process both events explicitly and assert call countMake the two-step handling more robust and self‑documenting.
- let rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0]; - globalThis[rLynxChange[0]](rLynxChange[1]); - rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[1]; - globalThis[rLynxChange[0]](rLynxChange[1]); + const calls = lynx.getNativeApp().callLepusMethod.mock.calls; + expect(calls.length).toBe(2); + const [initEvt, patchEvt] = calls; + expect(initEvt[0]).toBe('rLynxChangeRefInitValue'); + globalThis[initEvt[0]](initEvt[1]); + globalThis[patchEvt[0]](patchEvt[1]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.changeset/violet-wasps-shop.md(1 hunks)packages/react/runtime/__test__/snapshot/workletRef.test.jsx(3 hunks)packages/react/runtime/__test__/worklet/workletRef.test.jsx(3 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lifecycleConstant.ts(1 hunks)packages/react/runtime/src/lynx.ts(2 hunks)packages/react/runtime/src/lynx/tt.ts(2 hunks)packages/react/runtime/src/worklet/ref/updateInitValue.ts(1 hunks)packages/react/runtime/src/worklet/ref/workletRefPool.ts(2 hunks)packages/react/testing-library/src/__tests__/worklet.test.jsx(1 hunks)packages/react/testing-library/src/vitest-global-setup.js(2 hunks)packages/testing-library/testing-environment/src/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/violet-wasps-shop.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/react/runtime/src/lifecycleConstant.ts
- packages/react/runtime/src/worklet/ref/workletRefPool.ts
- packages/react/runtime/src/lynx/tt.ts
- packages/react/runtime/src/lynx.ts
- packages/react/runtime/src/lifecycle/patch/commit.ts
- packages/react/runtime/test/worklet/workletRef.test.jsx
- packages/react/runtime/src/worklet/ref/updateInitValue.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/testing-library/testing-environment/src/index.tspackages/react/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/testing-library/src/vitest-global-setup.js
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
🧬 Code graph analysis (3)
packages/testing-library/testing-environment/src/index.ts (1)
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (12)
rLynxChange(90-90)rLynxChange(101-101)rLynxChange(160-160)rLynxChange(304-304)rLynxChange(402-402)rLynxChange(487-487)rLynxChange(498-498)rLynxChange(559-559)rLynxChange(669-669)rLynxChange(765-765)rLynxChange(776-776)rLynxChange(812-812)
packages/react/testing-library/src/vitest-global-setup.js (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
🔇 Additional comments (6)
packages/react/runtime/src/lifecycle/patch/updateMainThread.ts (3)
6-6: Init‑value path decoupled from patch updates — import cleanup looks correct.Removing the old init‑value handler here fits the new rLynxChangeRefInitValue flow handled elsewhere.
Please confirm the new injectUpdateMTRefInitValue is registered before any hydration/patch begins so init values are not missed on first paint.
25-27: Reload guard: verify it no longer drops MTRef init‑value events.This early return still short‑circuits stale patch batches. Ensure rLynxChangeRefInitValue delivery is not gated by reloadVersion, and add a test where reloadVersion increments between scheduling the init‑value send and the first commit.
49-57: Patch loop now processes snapshotPatch only — aligns with intent.Good separation: worklet ref init‑values no longer ride the main patch list. Please double‑check that clearAttachedLists/flush boundaries don’t interfere with out‑of‑band init‑value application ordering.
packages/react/testing-library/src/vitest-global-setup.js (1)
7-8: Initialize MT ref init‑value handler earlyImport and init look correct; ordering after injectUpdateMainThread() and before replaceCommitHook() is appropriate and idempotent.
Please run the affected test suites to confirm no load‑order regressions around lifecycle injection.
Also applies to: 32-33
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (2)
17-17: Import addition is correctBrings the new injector into scope for tests using MT ref init values.
22-23: Good init orderCalling injectUpdateMTRefInitValue() after injectUpdateMainThread() and before replaceCommitHook() matches runtime wiring.
If this file can be loaded multiple times across suites, confirm the injector is idempotent (it should be via Object.assign).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/react/runtime/src/lynx/tt.ts (1)
131-134: Ordering-critical call before patchUpdate: keep this invariant.This placement enforces that init-value updates land before the patchUpdate (so reloadVersion can’t drop them). Add a brief comment to prevent regressions.
- const obj = commitPatchUpdate(patchList, { isHydration: true }); - sendMTRefInitValueToMainThread(); + const obj = commitPatchUpdate(patchList, { isHydration: true }); + // Must precede patchUpdate so reloadVersion gating can't drop the init-value patch. + sendMTRefInitValueToMainThread();packages/react/runtime/__test__/snapshot/workletRef.test.jsx (3)
270-297: Great: asserts ordering of rLynxChangeRefInitValue before rLynxChange. Add coverage for reloadVersion > 0.Current snapshot shows reloadVersion: 0. Add a case simulating a non‑zero reloadVersion to lock the fix.
I can draft a test that stubs patchOptions.reloadVersion > 0 (via a commit hook or helper) and asserts init‑value changes still apply. Want me to add it?
304-307: Avoid brittle indexing; replay all Lepus calls in order.- let rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0]; - globalThis[rLynxChange[0]](rLynxChange[1]); - rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[1]; - globalThis[rLynxChange[0]](rLynxChange[1]); + for (const [method, payload] of lynx.getNativeApp().callLepusMethod.mock.calls.slice(0, 2)) { + globalThis[method](payload); + }
498-501: Same here: iterate instead of fixed indexes.- let rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[0]; - globalThis[rLynxChange[0]](rLynxChange[1]); - rLynxChange = lynx.getNativeApp().callLepusMethod.mock.calls[1]; - globalThis[rLynxChange[0]](rLynxChange[1]); + for (const [method, payload] of lynx.getNativeApp().callLepusMethod.mock.calls) { + globalThis[method](payload); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.changeset/violet-wasps-shop.md(1 hunks)packages/react/runtime/__test__/snapshot/workletRef.test.jsx(3 hunks)packages/react/runtime/__test__/worklet/workletRef.test.jsx(3 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(2 hunks)packages/react/runtime/src/lifecycle/patch/updateMainThread.ts(2 hunks)packages/react/runtime/src/lifecycleConstant.ts(1 hunks)packages/react/runtime/src/lynx.ts(2 hunks)packages/react/runtime/src/lynx/tt.ts(2 hunks)packages/react/runtime/src/worklet/ref/updateInitValue.ts(1 hunks)packages/react/runtime/src/worklet/ref/workletRefPool.ts(2 hunks)packages/react/testing-library/src/__tests__/worklet.test.jsx(1 hunks)packages/react/testing-library/src/vitest-global-setup.js(2 hunks)packages/testing-library/testing-environment/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/react/runtime/src/lifecycleConstant.ts
- packages/testing-library/testing-environment/src/index.ts
- packages/react/runtime/src/lynx.ts
- .changeset/violet-wasps-shop.md
- packages/react/testing-library/src/vitest-global-setup.js
- packages/react/runtime/src/worklet/ref/updateInitValue.ts
- packages/react/runtime/src/lifecycle/patch/commit.ts
- packages/react/runtime/src/lifecycle/patch/updateMainThread.ts
- packages/react/testing-library/src/tests/worklet.test.jsx
- packages/react/runtime/test/worklet/workletRef.test.jsx
- packages/react/runtime/src/worklet/ref/workletRefPool.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#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/runtime/__test__/snapshot/workletRef.test.jsx
🧬 Code graph analysis (2)
packages/react/runtime/src/lynx/tt.ts (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
sendMTRefInitValueToMainThread(22-30)
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (1)
packages/react/runtime/src/worklet/ref/updateInitValue.ts (1)
injectUpdateMTRefInitValue(18-20)
⏰ 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). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (3)
packages/react/runtime/src/lynx/tt.ts (2)
29-29: Import is correct and scoped. LGTM.
131-134: Verify main-thread injection occurs before firstScreen.rg in your last run failed due to incorrect escaping; lifecycleConstant already defines updateMTRefInitValue → 'rLynxChangeRefInitValue' (packages/react/runtime/src/lifecycleConstant.ts:11). Confirm injectUpdateMTRefInitValue() is invoked during runtime init (e.g., lynx.ts) before the code in packages/react/runtime/src/lynx/tt.ts (around lines 131–134) calls sendMTRefInitValueToMainThread / LifecycleConstant.patchUpdate.
Run these corrected checks:
rg -n --type=ts -F 'injectUpdateMTRefInitValue(' packages || true rg -n --type=ts -F 'sendMTRefInitValueToMainThread(' packages/react/runtime/src || true rg -n --type=ts 'LifecycleConstant\.updateMTRefInitValue|rLynxChangeRefInitValue' packages | sed -n '1,200p'If injectUpdateMTRefInitValue() is not registered during runtime init, add the injection to the runtime initialization (lynx.ts) so the main-thread handler exists before firstScreen fires.
packages/react/runtime/__test__/snapshot/workletRef.test.jsx (1)
17-23: Test env wiring for new event looks good.
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/react@0.114.0 ### Minor Changes - Partially fix the "cannot read property 'update' of undefined" error. ([#1771](#1771)) This error happens when rendering a JSX expression in a [background-only](https://lynxjs.org/react/thinking-in-reactlynx.html) context. See [#894](#894) for more details. ### Patch Changes - Reduce extra snapshot when children are pure text ([#1562](#1562)) - feat: Support `SelectorQuery` `animation` APIs ([#1768](#1768)) - Fix spread props inside list-item caused redundant snapshot patch ([#1760](#1760)) - fix: `ref is not initialized` error on template reload ([#1757](#1757)) ## @lynx-js/react-rsbuild-plugin@0.11.0 ### Minor Changes - **BREAKING CHANGE:** Remove the `enableParallelElement` and `pipelineSchedulerConfig` options. ([#1705](#1705)) Since the thread element resolution is still in experimental stage and may have stability risks, it will be disabled by default after this change. - **BREAKING CHANGE**: Remove the `enableICU` option. ([#1800](#1800)) ### Patch Changes - Be compat with `@lynx-js/react` v0.114.0 ([#1781](#1781)) - Updated dependencies \[[`24100ab`](24100ab), [`24100ab`](24100ab), [`d0ef559`](d0ef559)]: - @lynx-js/template-webpack-plugin@0.9.0 - @lynx-js/react-webpack-plugin@0.7.1 - @lynx-js/css-extract-webpack-plugin@0.6.3 - @lynx-js/react-alias-rsbuild-plugin@0.11.0 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.4 ## @lynx-js/tailwind-preset@0.4.0 ### Minor Changes - Enable `lynxUIPlugins` (incl. `uiVariants`) by default. Fills the gap left by missing pseudo- and data-attribute selectors in Lynx, offering state and structural variants out of the box. ([#1774](#1774)) Opt-out globally or per plugin: ```ts // disable all UI plugins createLynxPreset({ lynxUIPlugins: false }); // or disable one plugin createLynxPreset({ lynxUIPlugins: { uiVariants: false } }); ``` ### Patch Changes - Fixed transform-related CSS variables previously defined on `:root`; they are now reset on `*` to prevent inheritance between parent and child elements. ([#1773](#1773)) ## @lynx-js/web-core@0.17.0 ### Minor Changes - break(web): temporary remove support for chunk split ([#1739](#1739)) Since the global variables cannot be accessed in the splited chunk, we temporary remove supporting for chunk spliting Developers could easily remove the chunk Split settings in Rspeedy for migration import { defineConfig } from '@lynx-js/rspeedy' export default defineConfig({ performance: { chunkSplit: { strategy: 'all-in-one', }, }, }) ### Patch Changes - fix: lazy component load error ([#1794](#1794)) Some special version template may have chunk loading error. We fixed it. - fix: avoid duplicate style transformation ([#1748](#1748)) After this commit, we use DAG methods to handle the styleInfos - fix: add sandbox attribute to iframe for enhanced security ([#1709](#1709)) - fix: the default template loader won't fetch twice for one url ([#1709](#1709)) - Updated dependencies \[[`721635d`](721635d), [`93d707b`](93d707b), [`d150ed4`](d150ed4)]: - @lynx-js/web-mainthread-apis@0.17.0 - @lynx-js/web-constants@0.17.0 - @lynx-js/web-worker-runtime@0.17.0 - @lynx-js/web-worker-rpc@0.17.0 ## @lynx-js/template-webpack-plugin@0.9.0 ### Minor Changes - **BREAKING CHANGE:** Remove the `enableParallelElement` and `pipelineSchedulerConfig` options. ([#1705](#1705)) Since the thread element resolution is still in experimental stage and may have stability risks, it will be disabled by default after this change. - **BREAKING CHANGE**: Remove the `enableICU` option. ([#1800](#1800)) ## @lynx-js/rspeedy@0.11.3 ### Patch Changes - Use `output.chunkLoading: 'lynx'` for `environments.web`. ([#1737](#1737)) - Support `resolve.extensions` ([#1759](#1759)) - Set the default value of `output.cssModules.localIdentName` to `[local]-[hash:base64:6]`. ([#1783](#1783)) ## @lynx-js/web-constants@0.17.0 ### Patch Changes - fix: avoid duplicate style transformation ([#1748](#1748)) After this commit, we use DAG methods to handle the styleInfos - Updated dependencies \[]: - @lynx-js/web-worker-rpc@0.17.0 ## @lynx-js/web-elements@0.8.7 ### Patch Changes - The img within svg does not inherit the position ([#1769](#1769)) - Updated dependencies \[]: - @lynx-js/web-elements-template@0.8.7 ## @lynx-js/web-mainthread-apis@0.17.0 ### Patch Changes - fix: \_\_QueryComponentImpl in mts should execute only once for same url ([#1763](#1763)) - fix: avoid duplicate style transformation ([#1748](#1748)) After this commit, we use DAG methods to handle the styleInfos - feat: support lazy bundle with CSSOG(`enableCSSSelector: false`). ([#1770](#1770)) - Updated dependencies \[[`93d707b`](93d707b)]: - @lynx-js/web-constants@0.17.0 - @lynx-js/web-style-transformer@0.17.0 ## @lynx-js/web-worker-runtime@0.17.0 ### Patch Changes - Updated dependencies \[[`721635d`](721635d), [`93d707b`](93d707b), [`d150ed4`](d150ed4)]: - @lynx-js/web-mainthread-apis@0.17.0 - @lynx-js/web-constants@0.17.0 - @lynx-js/web-worker-rpc@0.17.0 ## @lynx-js/css-extract-webpack-plugin@0.6.3 ### Patch Changes - Supports `@lynx-js/template-webpack-plugin` 0.9.0. ([#1705](#1705)) ## @lynx-js/react-webpack-plugin@0.7.1 ### Patch Changes - Supports `@lynx-js/template-webpack-plugin` 0.9.0. ([#1705](#1705)) ## create-rspeedy@0.11.3 ## @lynx-js/react-alias-rsbuild-plugin@0.11.0 ## upgrade-rspeedy@0.11.3 ## @lynx-js/web-core-server@0.17.0 ## @lynx-js/web-elements-template@0.8.7 ## @lynx-js/web-style-transformer@0.17.0 ## @lynx-js/web-worker-rpc@0.17.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
Bug Fixes
Improvements
Chores
Checklist