Repository navigation
fix(js): bundle worklets in-process so no esbuild service outlives the build - #5353
Conversation
…e build The worklet Vite plugin bundled with esbuild's build(), which starts esbuild's long-lived service process. Nothing stopped it, so it outlived `vite build` and was reaped by init, taking its CPU out of the build's accounting. Bundle with rolldown, the bundler Vite 8 already runs, which works in-process and leaves no child behind. The worklet consumers swap their esbuild devDependency for rolldown at Vite's resolved version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 27 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughDevelopment dependencies replace esbuild with Rolldown in several packages. The worklet plugin generates a browser-targeted ES module, returns the first output’s code, and closes the bundle. A new integration test checks whether child processes remain after the build and temporary-directory cleanup. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No confirmed issue currently prevents merging after normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: replacing out-of-process esbuild worklet bundling with in-process bundling to prevent lingering services. |
| Description check | |
The description directly explains the problem, implementation, affected dependencies, testing, and impact of the changeset. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (6 skipped: 6 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
Merge summary:
(Written by Claude Opus 5.5) |
Problem
js/common/vite-plugin-worklet.tsbundled each?workletentry with esbuild'sbuild(), which starts esbuild's long-lived service process. Nothing stopped it, so it outlivedvite build. Once the parent exited it was orphaned and reaped by init, so its CPU (about 0.6s) never reached the build's process accounting, and a stray process lingered after the build.Approach
Bundle with rolldown, the bundler Vite 8 already loads, through
rolldown()+generate()+close(). It runs in-process, so there is no child to leave behind. Vite's own?workerhandling nests rolldown builds inside a build the same way.codeSplitting: false), minified for builds only, and thealiasoption maps toresolve.alias. The three worklets come out the same size or smaller.@moq/watch,@moq/publish,demo/boy,demo/web, and the audio-quality and interop JS clients) swap theiresbuilddevDependency forrolldownat^1.2.12, the version Vite 8.3.2 resolves, so the lockfile keeps one copy.esbuildstays in the tree forsh/sizeandworklet-matrix.Tests:
builds leave no child process behindinjs/common/worklet.test.ts: runs a Vite build through the plugin, then assertspgrep -P <test pid>lists nothing. It fails on main, where the esbuild service is still a live child of the test process, and passes here.bun testruns the file's tests in one process, so the esbuild child an earlier build leaves also fails it.vite buildof@moq/watchemitsassets/render-worklet.js; a dev server serves?worklet-blob; in headless Chromium, the render and capture worklets load throughaudioWorklet.addModuleand the capture worker starts, both as blob URLs and as hosted files. Warmvite buildtimes match esbuild's (about 250 to 390ms on a loaded machine).just checkpasses.Impact
esbuildis replaced byrolldownin the six packages above.Alternatives
esbuild.stop()after building: it sends SIGTERM and returns without waiting, so a parent that exits next can still orphan the child. It also stops the service for every other esbuild user in the process.buildSync: on Node it runs the same long-lived service, hosted in a worker thread, so a child is still left behind.Follow-ups
releasebackport is onfix/worklet-inline-esbuild-release. It is a hand port, sincereleasestill hasworkletInlineand no worklet test.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.