refactor(react/runtime): reorganize worklet dir #1753
Conversation
🦋 Changeset detectedLatest commit: 5c01e9a 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 |
📝 WalkthroughWalkthroughThe PR restructures import and export paths across worklet-related modules, splitting call and ref utilities into subdirectories. Tests and runtime files update their imports accordingly. Public re-exports in lynx-api.ts and internal.ts now point to the new locations. A changeset file is added. No functional or API signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks✅ Passed checks (3 passed)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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: 0
🧹 Nitpick comments (2)
packages/react/runtime/src/worklet/destroy.ts (1)
5-11: Nit: make the side‑effect call explicit.We ignore the return from takeWorkletRefInitValuePatch for its clearing side‑effect; make that intention obvious.
takeWorkletRefInitValuePatch(); + // Drain and clear any buffered init values. + void takeWorkletRefInitValuePatch();packages/react/runtime/src/worklet/call/functionCall.ts (1)
35-40: Harden return-value handler against late or malformed events.A late event during teardown or malformed payload would throw on the non‑null assertions. Add guards; no behavior change for valid paths.
-function onFunctionCallRet(event: RuntimeProxy.Event): void { - const data = JSON.parse(event.data as string) as RunWorkletCtxRetData; - const resolve = resolveMap!.get(data.resolveId)!; - resolveMap!.remove(data.resolveId); - resolve(data.returnValue); -} +function onFunctionCallRet(event: RuntimeProxy.Event): void { + if (!resolveMap) return; // may be torn down via destroyTasks + let data: RunWorkletCtxRetData; + try { + data = JSON.parse(String(event.data)) as RunWorkletCtxRetData; + } catch { + return; + } + const resolve = resolveMap.get(data.resolveId); + if (!resolve) return; // late/duplicate event; nothing to do + resolveMap.remove(data.resolveId); + resolve(data.returnValue); +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.changeset/eager-feet-kick.md(1 hunks)packages/react/runtime/__test__/snapshot/workletRefBG.jsx(1 hunks)packages/react/runtime/__test__/snapshot/workletRefMT.jsx(1 hunks)packages/react/runtime/__test__/utils/envManager.ts(1 hunks)packages/react/runtime/__test__/worklet/execIdMap.test.js(1 hunks)packages/react/runtime/__test__/worklet/runOnBackground.test.js(1 hunks)packages/react/runtime/__test__/worklet/runOnMainThread.test.jsx(1 hunks)packages/react/runtime/__test__/worklet/transform.test.js(1 hunks)packages/react/runtime/__test__/worklet/workletRef.test.jsx(1 hunks)packages/react/runtime/src/internal.ts(1 hunks)packages/react/runtime/src/lifecycle/patch/commit.ts(1 hunks)packages/react/runtime/src/lynx-api.ts(1 hunks)packages/react/runtime/src/lynx/tt.ts(1 hunks)packages/react/runtime/src/worklet/call/execMap.ts(1 hunks)packages/react/runtime/src/worklet/call/functionCall.ts(1 hunks)packages/react/runtime/src/worklet/call/runOnBackground.ts(1 hunks)packages/react/runtime/src/worklet/call/runOnMainThread.ts(1 hunks)packages/react/runtime/src/worklet/ctx.ts(1 hunks)packages/react/runtime/src/worklet/destroy.ts(1 hunks)packages/react/runtime/src/worklet/ref/workletRef.ts(1 hunks)packages/react/runtime/src/worklet/ref/workletRefPool.ts(1 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/eager-feet-kick.md
🧠 Learnings (5)
📓 Common learnings
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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
📚 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:
packages/react/runtime/src/lynx-api.ts
📚 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-api.tspackages/react/runtime/src/internal.tspackages/react/runtime/src/lynx/tt.ts
📚 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/internal.tspackages/react/runtime/src/lynx/tt.ts
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/react/runtime/src/internal.ts
⏰ 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 (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (21)
packages/react/runtime/src/lynx/tt.ts (1)
24-27: Repointed delayedRunOnMainThreadData import is consistent with the new call/ layout.No behavioral change; usage below remains intact.
packages/react/runtime/src/worklet/ref/workletRef.ts (1)
10-10: UpdateduseMemorelative path is correct for the new folder depth.No API or runtime changes; constructor memoization stays the same.
packages/react/runtime/__test__/worklet/runOnBackground.test.js (1)
9-9: Import moved toworklet/call/runOnBackground.Test expectations unchanged; path aligns with the reorg.
packages/react/runtime/__test__/utils/envManager.ts (1)
14-14: Ref import updated toworklet/ref/workletRef.Consistent with the new structure; env reset still clears ref IDs.
packages/react/runtime/src/worklet/call/execMap.ts (1)
6-6: IndexMap import corrected to parent directory.Matches file relocation; no functional impact.
packages/react/runtime/src/internal.ts (1)
62-62: Re-export now targetsworklet/call/transformToWorklet.Public surface remains stable; good alignment with the reorg.
Cannot confirm there are no direct consumers importing the old path — ripgrep returned "No files were searched". Run these from the repository root and paste results:
rg -nS --hidden '\bworklet/transformToWorklet\b' -g '!**/node_modules/**' rg -nS --hidden '\btransformToWorklet\b' -g '!**/node_modules/**' rg -nP --hidden -S "from\s+['\"][^'\"]*worklet/transformToWorklet" -g '!**/node_modules/**'.changeset/eager-feet-kick.md (1)
1-3: Replace empty changeset with a 'none' entry for the private package @lynx-js/react-runtimepackages/react/runtime/package.json reports name="@lynx-js/react-runtime" and private=true — replace .changeset/eager-feet-kick.md with a 'none' changeset for @lynx-js/react-runtime and a short note (e.g., "chore(runtime): internal directory reorg for worklet/{call,ref}").
⛔ Skipped due to learnings
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.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.Learnt from: colinaaa PR: lynx-family/lynx-stack#1558 File: .changeset/solid-squids-fall.md:2-2 Timestamp: 2025-08-19T11:25:36.127Z Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).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#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.packages/react/runtime/__test__/worklet/transform.test.js (1)
7-7: ```shell
#!/bin/bash
set -euo pipefailecho "PWD: $(pwd)"
echo "Top-level files (first 200 lines):"
ls -la | sed -n '1,200p' || true
echoChoose file enumeration strategy (git tracked files if available, otherwise find)
if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
echo "Using git ls-files to enumerate tracked files..."
FILE_ENUM_CMD="git ls-files -z"
else
echo "Git not available — using find to enumerate files (excluding node_modules)..."
FILE_ENUM_CMD="find . -type f -not -path './node_modules/*' -print0"
fiecho
echo "Searching enumerated files for 'worklet' occurrences..."
eval "$FILE_ENUM_CMD" | xargs -0 -r rg -n --no-ignore -S --hidden "worklet" -C2 || trueecho
echo "Searching enumerated files for 'src/worklet/' occurrences (all matches):"
eval "$FILE_ENUM_CMD" | xargs -0 -r rg -n --no-ignore -S --hidden "src/worklet/" -C2 || trueecho
echo "Matches under 'src/worklet/' excluding 'src/worklet/call/' and 'src/worklet/ref/' (potential stale imports):"
eval "$FILE_ENUM_CMD" | xargs -0 -r rg -n --no-ignore -S --hidden "src/worklet/" -C2 | rg -v "src/worklet/call/" | rg -v "src/worklet/ref/" || trueecho
echo "Searching for specific moved symbols (transformToWorklet, runOnBackground, runOnMainThread, workletRef):"
eval "$FILE_ENUM_CMD" | xargs -0 -r rg -n --no-ignore -S --hidden -e "transformToWorklet" -e "runOnBackground" -e "runOnMainThread" -e "workletRef" -C2 || trueecho
echo "Done."</blockquote></details> <details> <summary>packages/react/runtime/src/worklet/call/functionCall.ts (1)</summary><blockquote> `8-9`: **Parent-path imports look right after folder split.** No issues with the move to ../destroy.js and ../indexMap.js. </blockquote></details> <details> <summary>packages/react/runtime/__test__/snapshot/workletRefBG.jsx (1)</summary><blockquote> `6-6`: **Approve: worklet import moved to worklet/ref — verification passed.** Verification script output: "OK: no stale imports detected in runtime." No further changes required. </blockquote></details> <details> <summary>packages/react/runtime/__test__/worklet/runOnMainThread.test.jsx (1)</summary><blockquote> `12-12`: **LGTM — import relocation verified.** Sanity check: no stale runOnMainThread imports found in packages/react/runtime. </blockquote></details> <details> <summary>packages/react/runtime/__test__/worklet/execIdMap.test.js (1)</summary><blockquote> `6-6`: **execMap path updated correctly — approved.** No behavior change in the test; verification found no stale execMap imports. </blockquote></details> <details> <summary>packages/react/runtime/__test__/snapshot/workletRefMT.jsx (1)</summary><blockquote> `9-9`: **Approve: workletRef import moved to worklet/ref — no stale imports remain.** Matches BG snapshot; ripgrep check output: OK: no stale workletRef imports. </blockquote></details> <details> <summary>packages/react/runtime/src/worklet/ref/workletRefPool.ts (1)</summary><blockquote> `5-5`: **Approve — functionality import path fix verified** Confirmed no remaining './functionality.js' imports under packages/react/runtime/src/worklet/ref. </blockquote></details> <details> <summary>packages/react/runtime/src/worklet/ctx.ts (1)</summary><blockquote> `6-6`: **Approve: runOnBackground import path updated correctly.** No semantic changes in onPostWorkletCtx. Sweep found no remaining './runOnBackground.js' imports in packages/react/runtime/src. </blockquote></details> <details> <summary>packages/react/runtime/src/lifecycle/patch/commit.ts (2)</summary><blockquote> `38-41`: **Delayed runOnMainThread data imports aligned** Imports now come from worklet/call; logic unchanged. Good. --- `32-32`: **Verify import target and cycle-freedom — workletRefPool not found** packages/react/runtime/src/lifecycle/patch/commit.ts imports '../../worklet/ref/workletRefPool.js' (line 32) but no matching file was found under packages/react/runtime/src; the dependency scan cannot confirm whether this introduced a cycle. Confirm the correct file path (or update the import to the actual source, e.g., workletRefPool.ts) and re-run the cycle check. </blockquote></details> <details> <summary>packages/react/runtime/__test__/worklet/workletRef.test.jsx (1)</summary><blockquote> `15-15`: **Test import path updated to ref/* — good** Matches the new public re-export and internal layout. Snapshot expectations remain valid. </blockquote></details> <details> <summary>packages/react/runtime/src/worklet/call/runOnMainThread.ts (1)</summary><blockquote> `7-12`: **Import path realignment LGTM — rerun scan (rg PCRE2 errors made results inconclusive)** Paths look correct and no functional changes were introduced, but your run showed PCRE2 compilation errors so the scan is inconclusive. Re-run using fixed-string ripgrep checks below: ```shell #!/bin/bash set -euo pipefail echo "Scanning for outdated deep imports (fixed-string search)..." rg -nF --hidden --glob '!**/dist/**' --glob '!**/build/**' "worklet/runOnMainThread" || echo "No matches: runOnMainThread" rg -nF --hidden --glob '!**/dist/**' --glob '!**/build/**' "worklet/runOnBackground" || echo "No matches: runOnBackground" rg -nF --hidden --glob '!**/dist/**' --glob '!**/build/**' "worklet/workletRef.js" || echo "No matches: workletRef"packages/react/runtime/src/worklet/call/runOnBackground.ts (1)
13-16: Updated internal imports look correctMoving destroyTasks up a level matches the new directory shape — destroyTasks is exported from packages/react/runtime/src/worklet/destroy.ts and imported by packages/react/runtime/src/worklet/call/runOnBackground.ts (and functionCall.ts). No behavior changes.
packages/react/runtime/src/lynx-api.ts (1)
462-465: Public re-exports updated — no changeset required (package is private)Re-exports now target worklet/{call,ref}; verified no stray public re-exports and packages/react/runtime/package.json has private: true, so no patch changeset is necessary.
React Example#5230 Bundle Size — 238.9KiB (0%).5c01e9a(current) vs 1a720e0 main#5227(baseline) Bundle metrics
|
| Current #5230 |
Baseline #5227 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
164 |
164 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.78% |
46.78% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5230 |
Baseline #5227 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
93.14KiB |
93.14KiB |
Bundle analysis report Branch Yradex:fix/mtf-reload4 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5223 Bundle Size — 364.39KiB (0%).5c01e9a(current) vs 1a720e0 main#5220(baseline) Bundle metrics
|
| Current #5223 |
Baseline #5220 |
|
|---|---|---|
| Initial JS |
145.41KiB |
145.41KiB |
| Initial CSS |
31.89KiB |
31.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
219(+0.46%) |
218 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.37% |
3.37% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5223 |
Baseline #5220 |
|
|---|---|---|
| JS |
238.48KiB |
238.48KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch Yradex:fix/mtf-reload4 Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1753 will degrade performances by 11.62%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ❌ | transform 1000 effects |
30.4 ms | 34.4 ms | -11.62% |
2 participants
Summary by CodeRabbit
Checklist