feat(egg-bundler): generate snapshot entry/prelude + egg-bin snapshot command#5998
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds snapshot build, restore, and snapshot-boot process handling across ChangesSnapshot build, restore, and process control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Deploying egg with
|
| Latest commit: |
a7ec16b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a8828be2.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-egg-bundler-snapshot-en.egg-cci.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5998 +/- ##
==========================================
- Coverage 84.89% 84.88% -0.01%
==========================================
Files 669 674 +5
Lines 19942 20261 +319
Branches 3964 4037 +73
==========================================
+ Hits 16929 17199 +270
- Misses 2588 2631 +43
- Partials 425 431 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
a7ec16b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://05435721.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-egg-bundler-snapshot-en.egg-v3.pages.dev |
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for V8 startup snapshots in both @eggjs/egg-bundler and egg-bin. It adds a new snapshot command to egg-bin for building and starting snapshots, extracts shared bundle options, and updates the bundler to force single-file output and prepend a snapshot prelude. Additionally, the entry generator is updated to support snapshot build and restore modes. Feedback on these changes suggests improving termination signal forwarding in the spawned process to correctly report exit status, and adding a fallback for process.getBuiltinModule to maintain compatibility with Node.js versions prior to 22.3.0.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
PR2 符合性评审 ✅ 符合,可合入对照 PR2 要求(snapshot entry/prelude 生成机制 + egg-bin 命令,依赖 PR1 单文件输出)逐项核对:
边界遵守正确:lazy/native-binding stub 机制确实未泄漏到本 PR —— prelude body 为空占位,EntryGenerator 只装了 restore-importer(属 PR2 restore-main 范畴), 额外硬化(均在 PR2 scope 内,加分项):
测试: 无需改动。结论:符合 PR2,可合入;PR3(lazy-external + 默认网络栈列表 + delete-globals prelude body)可在此基础上接续。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/egg-bundler/src/lib/EntryGenerator.ts (1)
388-391: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRestore-time synchronous setup is outside the error boundary.
process.getBuiltinModule('node:module')andcreateRequire(...)run before the async IIFE that has the.catch(...). If either throws during deserialize, it surfaces as an unhandled exception with no diagnostic, unlike the deferred work below which logs and exits cleanly. Consider moving these two lines inside thetry/catch-guarded async block (or wrapping them) so restore failures get the same'[egg-bundler] failed to restore snapshot:'treatment.🤖 Prompt for 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. In `@tools/egg-bundler/src/lib/EntryGenerator.ts` around lines 388 - 391, The restore-time setup in EntryGenerator is happening outside the async error boundary, so failures from process.getBuiltinModule('node:module') or createRequire(...) bypass the existing snapshot restore handling. Move the require initialization into the same try/catch-guarded async flow that already wraps the restore work, or explicitly wrap these statements so any deserialize-time failure is reported through the existing '[egg-bundler] failed to restore snapshot:' path. Keep the assignments to globalThis.__RUNTIME_REQUIRE and globalThis.__EGG_MODULE_IMPORTER__ in that guarded section.
🤖 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 `@tools/egg-bin/src/commands/snapshot.ts`:
- Around line 141-148: The `SnapshotCommand` flow logs a successful blob write
even when `flags['dry-run']` skips spawning and the `fs.access` existence check,
so update the logic around `#spawnNode` and the final `this.log` call to only
report “snapshot blob written” when a blob was actually produced. In dry-run
mode, either suppress that success message or replace it with a dry-run-specific
message, while keeping the existing error path for missing blobs when not in
dry-run.
---
Nitpick comments:
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 388-391: The restore-time setup in EntryGenerator is happening
outside the async error boundary, so failures from
process.getBuiltinModule('node:module') or createRequire(...) bypass the
existing snapshot restore handling. Move the require initialization into the
same try/catch-guarded async flow that already wraps the restore work, or
explicitly wrap these statements so any deserialize-time failure is reported
through the existing '[egg-bundler] failed to restore snapshot:' path. Keep the
assignments to globalThis.__RUNTIME_REQUIRE and
globalThis.__EGG_MODULE_IMPORTER__ in that guarded section.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2138bb0-d971-426f-9fe0-e88f489b8117
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
packages/typings/src/global.tstools/egg-bin/package.jsontools/egg-bin/src/bundleOptions.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/prelude.test.ts
357ea82 to
5c11204
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tools/egg-bin/test/bundleOptions.test.ts (2)
39-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the default framework fallback.
getBundleFrameworkSpecifier()also has a fallback branch that returns'egg'when no usable framework is found, but this suite only locks down the explicit and happy-path fixture cases. Covering that branch would make the new helper contract much harder to regress.🤖 Prompt for 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. In `@tools/egg-bin/test/bundleOptions.test.ts` around lines 39 - 47, Add a test case in getBundleFrameworkSpecifier coverage for the fallback path where no explicit framework and no usable egg.framework are found, and assert that it resolves to 'egg'. Place it alongside the existing getBundleFrameworkSpecifier tests in bundleOptions.test.ts, using a fixture or temporary baseDir that lacks a valid framework value so the default branch is exercised.
1-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this file to lowercase hyphen form.
bundleOptions.test.tsbreaks the repo filename convention. A name likebundle-options.test.tswould match the rule and keep imports consistent. As per coding guidelines, "Keep file names lowercase with hyphens".🤖 Prompt for 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. In `@tools/egg-bin/test/bundleOptions.test.ts` around lines 1 - 49, Rename the test file to follow the lowercase hyphenated filename convention by changing the bundleOptions.test.ts test module to bundle-options.test.ts, and update any imports or references that point to this test file so they continue to resolve correctly; keep the existing test contents and symbols like getBundleMode, parsePackAliases, and getBundleFrameworkSpecifier unchanged.Source: Coding guidelines
tools/egg-bin/test/commands/snapshot.test.ts (1)
106-138: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover the signal-exit branches too.
These tests validate the non-zero exit path, but
tools/egg-bin/src/commands/snapshot.tsnow also distinguishes between signal-killed children and user-initiated termination. A focused test forcode === nullwould lock down the new failure/cleanup behavior that this PR added.🤖 Prompt for 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. In `@tools/egg-bin/test/commands/snapshot.test.ts` around lines 106 - 138, Add a focused snapshot test for the signal-exit path in Snapshot.run, alongside the existing non-zero exit case in snapshot.test.ts. Mock the spawned child so the exit handler receives code as null and a signal value, then assert the command follows the new cleanup/failure behavior introduced in tools/egg-bin/src/commands/snapshot.ts for signal-killed children rather than the normal exit-code path. Use the existing Snapshot.run, spawnMock, and spawnArgs helpers to keep the test aligned with the current build/start coverage.tools/egg-bin/src/bundleOptions.ts (1)
1-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this new file to match the repo naming convention.
bundleOptions.tsintroduces a second filename style into the CLI code. Renaming it to a lowercase hyphenated path now is much cheaper than carrying the exception forward through more imports.As per coding guidelines,
**/*: Keep file names lowercase with hyphens.🤖 Prompt for 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. In `@tools/egg-bin/src/bundleOptions.ts` around lines 1 - 45, The new bundle options module uses a camelCase filename that conflicts with the repository’s lowercase-hyphen naming convention. Rename the file containing getBundleMode, parsePackAliases, and getBundleFrameworkSpecifier to a lowercase hyphenated name, then update every import/re-export that references bundleOptions so the CLI code continues to resolve it correctly.Source: Coding guidelines
🤖 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 `@tools/egg-bundler/src/lib/Bundler.ts`:
- Around line 456-460: The snapshot entry lookup in Bundler should fail fast for
the required worker entry instead of skipping missing files. Update the
read/continue logic around the fs.readFile try/catch so that ENOENT is no longer
ignored in this path, and let the missing worker entry surface as an immediate
error from the snapshot build flow. Use the Bundler entry-loading code and the
worker entry handling in this loop to locate the change.
---
Nitpick comments:
In `@tools/egg-bin/src/bundleOptions.ts`:
- Around line 1-45: The new bundle options module uses a camelCase filename that
conflicts with the repository’s lowercase-hyphen naming convention. Rename the
file containing getBundleMode, parsePackAliases, and getBundleFrameworkSpecifier
to a lowercase hyphenated name, then update every import/re-export that
references bundleOptions so the CLI code continues to resolve it correctly.
In `@tools/egg-bin/test/bundleOptions.test.ts`:
- Around line 39-47: Add a test case in getBundleFrameworkSpecifier coverage for
the fallback path where no explicit framework and no usable egg.framework are
found, and assert that it resolves to 'egg'. Place it alongside the existing
getBundleFrameworkSpecifier tests in bundleOptions.test.ts, using a fixture or
temporary baseDir that lacks a valid framework value so the default branch is
exercised.
- Around line 1-49: Rename the test file to follow the lowercase hyphenated
filename convention by changing the bundleOptions.test.ts test module to
bundle-options.test.ts, and update any imports or references that point to this
test file so they continue to resolve correctly; keep the existing test contents
and symbols like getBundleMode, parsePackAliases, and
getBundleFrameworkSpecifier unchanged.
In `@tools/egg-bin/test/commands/snapshot.test.ts`:
- Around line 106-138: Add a focused snapshot test for the signal-exit path in
Snapshot.run, alongside the existing non-zero exit case in snapshot.test.ts.
Mock the spawned child so the exit handler receives code as null and a signal
value, then assert the command follows the new cleanup/failure behavior
introduced in tools/egg-bin/src/commands/snapshot.ts for signal-killed children
rather than the normal exit-code path. Use the existing Snapshot.run, spawnMock,
and spawnArgs helpers to keep the test aligned with the current build/start
coverage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc684d41-f36e-497d-aac7-6f514cc46e17
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
packages/typings/src/global.tstools/egg-bin/package.jsontools/egg-bin/src/bundleOptions.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/bundleOptions.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/prelude.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/egg-bin/src/commands/bundle.ts
- tools/egg-bundler/test/prelude.test.ts
- packages/typings/src/global.ts
- tools/egg-bundler/src/index.ts
- tools/egg-bin/package.json
- tools/egg-bundler/src/lib/EntryGenerator.ts
- tools/egg-bundler/test/EntryGenerator.test.ts
- tools/egg-bundler/src/lib/prelude.ts
5c11204 to
52b0ae5
Compare
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 `@tools/egg-bundler/src/lib/Bundler.ts`:
- Around line 459-462: The fs.readFile error handling in Bundler’s snapshot
prelude path is over-catching and rewrites every failure as a missing entry;
update the catch so only ENOENT is converted into the custom “was not found”
error, using the existing rel/filename context, and rethrow any other
NodeJS.ErrnoException unchanged so permission and I/O issues preserve their
original details.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14f7d31f-5be0-41cf-bd3f-5cdaf97d854e
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
packages/typings/src/global.tstools/egg-bin/package.jsontools/egg-bin/src/bundleOptions.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/bundleOptions.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/prelude.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/typings/src/global.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- tools/egg-bin/test/bundleOptions.test.ts
- tools/egg-bin/package.json
- tools/egg-bin/src/commands/bundle.ts
- tools/egg-bin/src/bundleOptions.ts
- tools/egg-bundler/test/Bundler.test.ts
- tools/egg-bundler/src/lib/EntryGenerator.ts
- tools/egg-bundler/src/lib/prelude.ts
- tools/egg-bin/src/commands/snapshot.ts
- tools/egg-bundler/test/EntryGenerator.test.ts
- tools/egg-bundler/src/index.ts
- tools/egg-bundler/test/prelude.test.ts
- tools/egg-bin/test/commands/snapshot.test.ts
52b0ae5 to
60e2067
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/egg-bin/src/bundleOptions.ts (1)
1-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this helper file to match the repo filename convention.
This new module introduces a camelCase path (
bundleOptions.ts), so the new snapshot command imports already depend on a non-conforming filename. Renaming it to something likebundle-options.tsnow keeps the new surface aligned before it spreads further. As per coding guidelines,**/*: Keep file names lowercase with hyphens.🤖 Prompt for 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. In `@tools/egg-bin/src/bundleOptions.ts` around lines 1 - 45, The new helper module name does not follow the repo’s lowercase-with-hyphens convention, so rename the bundle-options helper file to a hyphenated lowercase filename and update any imports that reference it. Keep the exported helpers unchanged, but ensure the new snapshot command and any other consumers import from the renamed file so the shared bundle parsing symbols remain reachable.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@tools/egg-bin/src/bundleOptions.ts`:
- Around line 1-45: The new helper module name does not follow the repo’s
lowercase-with-hyphens convention, so rename the bundle-options helper file to a
hyphenated lowercase filename and update any imports that reference it. Keep the
exported helpers unchanged, but ensure the new snapshot command and any other
consumers import from the renamed file so the shared bundle parsing symbols
remain reachable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3c474d3-d1f7-4c2d-8db1-ca77e91f0800
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
packages/typings/src/global.tstools/egg-bin/package.jsontools/egg-bin/src/bundleOptions.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/bundleOptions.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/prelude.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- tools/egg-bundler/test/prelude.test.ts
- tools/egg-bin/test/bundleOptions.test.ts
- packages/typings/src/global.ts
- tools/egg-bin/src/commands/bundle.ts
- tools/egg-bundler/test/EntryGenerator.test.ts
- tools/egg-bin/test/commands/snapshot.test.ts
- tools/egg-bin/package.json
- tools/egg-bundler/src/index.ts
- tools/egg-bundler/src/lib/EntryGenerator.ts
- tools/egg-bundler/src/lib/prelude.ts
60e2067 to
2da1f5d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tools/egg-bin/src/bundleOptions.ts (1)
1-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this file to match the repo filename convention.
bundleOptions.tsbreaks the lowercase-hyphen rule for new files. Renaming it tobundle-options.tskeeps the new shared helper aligned with the rest of the repo. As per coding guidelines,**/*: Keep file names lowercase with hyphens.🤖 Prompt for 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. In `@tools/egg-bin/src/bundleOptions.ts` around lines 1 - 46, The shared helper file name does not follow the repo’s lowercase-hyphen convention, so rename bundleOptions.ts to bundle-options.ts and update any imports/usages that reference the BundleMode helpers, parsePackAliases, or getBundleFrameworkSpecifier so the new path is used consistently across egg-bin bundle and snapshot build.Source: Coding guidelines
🤖 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 `@tools/scripts/src/commands/start.ts`:
- Around line 134-141: The snapshot early return in start() skips the shared
startup option and environment normalization, so move the common env/argv
preparation before the snapshot-blob branch and keep only the final command
selection different. Ensure startFromSnapshot() and the regular start-cluster
path both receive the same processed eggScriptsConfig values, including require,
node-options flags, sourcemap handling, PATH adjustments, and the
EGG_TS_ENABLE=false hardening.
In `@tools/scripts/src/commands/stop.ts`:
- Around line 49-58: The snapshot process matching in stop.ts is too broad and
can match unrelated snapshot builds or partial title substrings. Update the
matching logic in the stop command’s command-checking path so the snapshot
branch only targets snapshot-started servers by parsing argv boundaries instead
of using raw substring checks, and require a real --title= argument when
flags.title is absent. Keep the existing clusterMatched/snapshotMatched
structure, but make the snapshot criteria precise enough to avoid matching
egg-bin snapshot build or titles like egg-server-foo-bar when stopping
egg-server-foo.
---
Nitpick comments:
In `@tools/egg-bin/src/bundleOptions.ts`:
- Around line 1-46: The shared helper file name does not follow the repo’s
lowercase-hyphen convention, so rename bundleOptions.ts to bundle-options.ts and
update any imports/usages that reference the BundleMode helpers,
parsePackAliases, or getBundleFrameworkSpecifier so the new path is used
consistently across egg-bin bundle and snapshot build.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 968e581f-f0de-4dd5-8e56-0dd18b614793
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (18)
packages/typings/src/global.tstools/egg-bin/package.jsontools/egg-bin/src/bundleOptions.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/bundleOptions.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/prelude.test.tstools/scripts/src/commands/start.tstools/scripts/src/commands/stop.tstools/scripts/test/snapshot-start.test.tstools/scripts/test/snapshot-stop.test.ts
✅ Files skipped from review due to trivial changes (1)
- tools/egg-bin/test/bundleOptions.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/egg-bin/package.json
- packages/typings/src/global.ts
- tools/egg-bundler/test/prelude.test.ts
- tools/egg-bundler/src/lib/prelude.ts
- tools/egg-bin/src/commands/bundle.ts
- tools/egg-bundler/test/EntryGenerator.test.ts
- tools/egg-bundler/src/lib/EntryGenerator.ts
- tools/egg-bundler/src/index.ts
2da1f5d to
0488de0
Compare
… command Add the V8 startup snapshot entry-generation mechanism to @eggjs/egg-bundler (building on the single-file output default from #5997) plus egg-bin commands. - EntryGenerator emits a 3-mode worker entry driven by EGG_BUNDLE_SNAPSHOT: - normal: startEgg + listen (unchanged) - snapshot-build (EGG_BUNDLE_SNAPSHOT=build): startEgg({snapshot:true}), run snapshotWillSerialize hooks, register v8 setDeserializeMainFunction - restore main: setImmediate-deferred (ESM loader not ready at deserialize), installs require-based __EGG_MODULE_IMPORTER__/__RUNTIME_REQUIRE hooks so the egg loader avoids the missing dynamic import() callback, then resumes snapshotDidDeserialize and listens - Add a snapshot prelude generator; Bundler prepends it before the bundle IIFE in snapshot mode (skeleton placeholder; PR3 fills the lazy/stub mechanism) - BundlerConfig.snapshot forces single-file output and prelude prepend - egg-bin: add `snapshot build` (bundle + node --build-snapshot) and `snapshot start` (node --snapshot-blob); spawn (not fork) to avoid an IPC handle that would break --build-snapshot - Declare __RUNTIME_REQUIRE in @eggjs/typings global Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0488de0 to
a7ec16b
Compare
Fill the snapshot prelude skeleton from eggjs#5998 with the lazy-external / native-binding stub mechanism, so a snapshot bundle stays serializable. The Node network stack (http/https/http2/tls/dns) produces native bindings (HTTPParser, nghttp2 settingsBuffer, tls SecureContext, dns ChannelWrap) that V8 cannot serialize, and WebAssembly is disabled under --build-snapshot. They must not load while the snapshot is built, then load for real at restore. prelude.ts (renderSnapshotPrelude now carries the lazy id set): - deletes Node's lazy web globals with `delete` (redefining the accessor would trigger the undici load we avoid); - installs `__LAZY_EXT` + `__makeLazyExt`. The factory returns a Proxy that is a stub at build time (hardcoded http METHODS/STATUS_CODES/maxHeaderSize so a library's top-level `[...http.METHODS]` does not force a load) and forwards to the real module via `globalThis.__RUNTIME_REQUIRE` once the generated restore entry installs it. Structural traps delegate to the target to keep Proxy invariants. - new `injectExternalRequireLazyHook` and `resolveSnapshotLazyModules`. Bundler (snapshot mode): resolves the lazy id set (network-stack default + app `egg.snapshot.lazyModules` from package.json), keeps those ids external so @utoo/pack emits externalRequire, injects the lazy dispatch into externalRequire, then prepends the prelude carrying that id set. Third-party business deps (leoric/@elastic/...) are out of scope — apps can add them via egg.snapshot.lazyModules but stub-completeness is a separate RFC. Tests: lazy-module resolution, filled prelude, externalRequire injection, vm-based runtime proxy behavior (build stub vs restore), a Bundler wiring test, and a real @utoo/pack build proving http is a stub at build and the real module after __RUNTIME_REQUIRE. eggjs#5998's prelude/Bundler tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fill the snapshot prelude skeleton from eggjs#5998 with the lazy-external / native-binding stub mechanism, so a snapshot bundle stays serializable. The Node network stack (http/https/http2/tls/dns) produces native bindings (HTTPParser, nghttp2 settingsBuffer, tls SecureContext, dns ChannelWrap) that V8 cannot serialize, and WebAssembly is disabled under --build-snapshot. They must not load while the snapshot is built, then load for real at restore. prelude.ts (renderSnapshotPrelude now carries the lazy id set): - deletes Node's lazy web globals with `delete` (redefining the accessor would trigger the undici load we avoid); - installs `__LAZY_EXT` + `__makeLazyExt`. The factory returns a Proxy that is a stub at build time (hardcoded http METHODS/STATUS_CODES/maxHeaderSize so a library's top-level `[...http.METHODS]` does not force a load) and forwards to the real module via `globalThis.__RUNTIME_REQUIRE` once the generated restore entry installs it. Structural traps delegate to the target to keep Proxy invariants. - new `injectExternalRequireLazyHook` and `resolveSnapshotLazyModules`. Bundler (snapshot mode): resolves the lazy id set (network-stack default + app `egg.snapshot.lazyModules` from package.json), keeps those ids external so @utoo/pack emits externalRequire, injects the lazy dispatch into externalRequire, then prepends the prelude carrying that id set. Third-party business deps (leoric/@elastic/...) are out of scope — apps can add them via egg.snapshot.lazyModules but stub-completeness is a separate RFC. Tests: lazy-module resolution, filled prelude, externalRequire injection, vm-based runtime proxy behavior (build stub vs restore), a Bundler wiring test, and a real @utoo/pack build proving http is a stub at build and the real module after __RUNTIME_REQUIRE. eggjs#5998's prelude/Bundler tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fill the snapshot prelude skeleton from eggjs#5998 with the lazy-external / native-binding stub mechanism, so a snapshot bundle stays serializable. The Node network stack (http/https/http2/tls/dns) produces native bindings (HTTPParser, nghttp2 settingsBuffer, tls SecureContext, dns ChannelWrap) that V8 cannot serialize, and WebAssembly is disabled under --build-snapshot. They must not load while the snapshot is built, then load for real at restore. prelude.ts (renderSnapshotPrelude now carries the lazy id set): - deletes Node's lazy web globals with `delete` (redefining the accessor would trigger the undici load we avoid); - installs `__LAZY_EXT` + `__makeLazyExt`. The factory returns a Proxy that is a stub at build time (hardcoded http METHODS/STATUS_CODES/maxHeaderSize so a library's top-level `[...http.METHODS]` does not force a load) and forwards to the real module via `globalThis.__RUNTIME_REQUIRE` once the generated restore entry installs it. Structural traps delegate to the target to keep Proxy invariants. - new `injectExternalRequireLazyHook` and `resolveSnapshotLazyModules`. Bundler (snapshot mode): resolves the lazy id set (network-stack default + app `egg.snapshot.lazyModules` from package.json), keeps those ids external so @utoo/pack emits externalRequire, injects the lazy dispatch into externalRequire, then prepends the prelude carrying that id set. Third-party business deps (leoric/@elastic/...) are out of scope — apps can add them via egg.snapshot.lazyModules but stub-completeness is a separate RFC. Tests: lazy-module resolution, filled prelude, externalRequire injection, vm-based runtime proxy behavior (build stub vs restore), a Bundler wiring test, and a real @utoo/pack build proving http is a stub at build and the real module after __RUNTIME_REQUIRE. eggjs#5998's prelude/Bundler tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Motivation #5998 wired the V8 snapshot mechanism for `@eggjs/egg-bundler` (3-mode entry, prelude prepend, `egg-bin snapshot` / `egg-scripts start --snapshot-blob`) and left the snapshot prelude as an **intentional no-op skeleton** with the note *"PR3 fills the lazy / native-binding stub mechanism."* This PR fills it. A V8 startup snapshot cannot serialize the native bindings the Node network stack allocates — `node:http`/`node:https` `HTTPParser` (llhttp), `node:http2` `nghttp2` settingsBuffer, `node:tls` `SecureContext`, `node:dns` `ChannelWrap` — and `WebAssembly` is disabled under `--build-snapshot`. Egg's loader phase touches that stack (HttpClient, agents…), so it must **not** load while the snapshot is built, then load for real at restore. ## What this does **`prelude.ts` — `renderSnapshotPrelude(lazyModules)` now emits a real body:** - deletes Node's lazy web globals (`fetch`/`Headers`/`Request`/…) with `delete` — redefining the lazy accessor would itself trigger the undici load we avoid; - installs `globalThis.__LAZY_EXT` (the lazy id set) and `globalThis.__makeLazyExt`. The factory returns a `Proxy` that is a **stub at build time** (hardcoded `http` `METHODS`/`STATUS_CODES`/`maxHeaderSize` so a library's top-level `[...http.METHODS]` doesn't force a load) and **forwards to the real module via `globalThis.__RUNTIME_REQUIRE`** once #5998's restore entry installs it — so `http.createServer` is the genuine builtin and the app truly listens. Structural traps (`ownKeys`/`getOwnPropertyDescriptor`) delegate to the target to keep `Proxy` invariants. - adds `injectExternalRequireLazyHook` (idempotent) and `resolveSnapshotLazyModules`. **`Bundler.ts` (snapshot mode):** resolves the lazy id set (network-stack default + the app's `egg.snapshot.lazyModules` from `package.json`), keeps those ids **external** so `@utoo/pack` emits an `externalRequire` call, injects the lazy dispatch into `externalRequire`, and prepends the prelude carrying that id set — all in a single pass over the worker output. ### Default lazy list `http`, `https`, `http2` (+ `node:` variants), `tls`/`node:tls`, `dns`/`node:dns`. ### Application extension point ```jsonc // package.json { "egg": { "snapshot": { "lazyModules": ["leoric", "@elastic/elasticsearch"] } } } ``` appended (deduped) to the default list. ## Boundary This PR covers the **Node built-in network stack** (fully functional at runtime). Third-party business deps (leoric/@elastic/…) can be added via `egg.snapshot.lazyModules`, but completing their stubs is a separate RFC. Note the lazy mechanism relies on **CommonJS `require`** (how urllib/undici/node internals load the stack); ESM `import *` can't defer through a static namespace, which is an inherent ESM limitation, not specific to this PR. ## Test evidence - `snapshot-lazy-external.test.ts` — lazy-module resolution, filled prelude content, `externalRequire` injection (incl. idempotency), and **vm-evaluated runtime proxy behavior** (build-time stub vs restore-time forward, Proxy invariants). - `snapshot-lazy-bundler.test.ts` — Bundler wiring: hook injected, lazy ids kept external, `egg.snapshot.lazyModules` merged, no-op when snapshot off. - `snapshot-lazy.realbuild.test.ts` — **real `@utoo/pack` build** proving `node:http` is a stub at build (hardcoded METHODS, `createServer()` no-ops) and the real module after `__RUNTIME_REQUIRE` is installed. - #5998's `prelude.test.ts` and `Bundler.test.ts` snapshot cases stay green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Snapshot builds now support lazy handling for selected external modules, including app-provided overrides from configuration. * Snapshot output embeds runtime helpers and automatically patches external module loading to route through the lazy dispatch path. * Expanded public exports for snapshot-lazy utilities to support custom integrations. * **Bug Fixes** * Improved reliability and idempotency of snapshot patching, with stronger validation and clearer failure modes. * Updated manifest externals behavior to preserve default network externals while adding configured lazy modules. * **Tests** * Added unit and real-build regression coverage for lazy-external wiring, fallback behavior, and error handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd cnpmcore e2e (#6003) ## Motivation Restoring a V8 startup snapshot requires **Node.js >= 24**: Node.js 22 aborts during deserialization of a non-trivial Egg heap with the native fatal `Check failed: current == end_slot_index` (a V8 bug). Building a snapshot still works on Node.js >= 22. Today nothing enforces or documents this, and there is no regression coverage. This PR adds a runtime gate, documentation, and an e2e regression — without changing the snapshot mechanism itself. Builds on the snapshot work already on `next` (#5998 entry/prelude + `egg-bin snapshot` command, #5999 lazy-external network stack, #6001 logger reopen, #6002 module-loader hooks). ## Scope **Runtime gate (restore ≥ 24; build stays ≥ 22)** - `@eggjs/scripts`: `egg-scripts start --snapshot-blob` refuses to launch on Node.js < 24 with a clear error *before* spawning, checking the major version of the resolved `--node` target binary (not just the egg-scripts runtime). Also adds `allowNo: true` to the `sourcemap` flag so `--no-sourcemap` is accepted. - `@eggjs/egg-bundler`: a defense-in-depth guard in the generated deserialize-main for direct `node --snapshot-blob` launches that manage to deserialize on an unsupported runtime. - `@eggjs/bin`: `snapshot build` prints a note that restoring needs Node.js >= 24. **Docs** - Enrich `site/docs/advanced/snapshot.md` (EN + ZH): Node version requirements, the CLI workflow (`egg-bin snapshot build` → `egg-scripts start --snapshot-blob`), how it works (load module graph → run to `configWillLoad` → freeze; restore = `didReady` + listen), performance (~233ms vs ~942ms, ~4× on cnpmcore), and known limitations. - Wire the page into the VitePress sidebar (EN + ZH) — it existed but was unreachable. **CI** - Add a blocking `cnpmcore-snapshot` ecosystem-ci e2e (Node 24): snapshot build → restore via `egg-scripts start --snapshot-blob` → `curl /-/ping` == 200 → stop. Wires `repo.json`, `patch-project.ts`, `.gitignore`. - Extract the shared health-check poll into `ecosystem-ci/wait-health.sh`, used by both the `cnpmcore` and `cnpmcore-snapshot` jobs. ## Test evidence - `pnpm --filter=@eggjs/scripts --filter=@eggjs/egg-bundler --filter=@eggjs/bin run typecheck` — clean. - `@eggjs/scripts` `snapshot-start.test.ts` (4 tests, incl. a Node<24 gate test and a `--no-sourcemap` parse regression test) + `start-unit.test.ts` — pass. - `@eggjs/bin` `snapshot.test.ts` — pass. - `@eggjs/egg-bundler` `EntryGenerator` canonical snapshot regenerated for the new guard; the rest of the suite matches the pre-change baseline (a few pre-existing macOS/Node-22 path-resolution failures are unrelated). - A multi-agent diff review was run and all confirmed findings addressed (most notably: `--no-sourcemap` is now parseable via `allowNo: true` — without it the e2e job would have failed 100%). ## Notes - The `cnpmcore-snapshot` job is correct-by-construction but **could not be validated on the author's machine** (Node 22, no MySQL+cnpmcore build); it relies on the supported path (lazy-external from #5999, no manual stubs) and is validated by this PR's CI run. - The job is intentionally a **separate matrix project** (not folded into the existing cnpmcore job) for failure isolation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added “V8 Startup Snapshot” documentation navigation. * Added support for snapshot project configuration (shared project-root patching). * **Bug Fixes** * Enforced Node.js version gating for V8 snapshot restore (requires Node.js ≥ 24) and improved snapshot restore safety. * Improved snapshot lazy-external behavior, including correct external named-export handling. * Updated the start command to allow `--no-sourcemap`. * **Documentation** * Expanded snapshot docs with requirements, workflow details, performance, and limitations. * **Tests** * Updated snapshot start/version-gating and lazy-external readiness assertions. * **Chores** * Improved E2E readiness checks with consistent polling, timeouts, and error log output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Motivation Building a V8 startup snapshot serializes the whole heap, so any dependency that opens a socket, starts a timer, or initializes a native binding at module-evaluation time can make the blob fail to build — or build and then crash on restore. There was no guide for finding which module is responsible or how to fix it. ## What New dedicated page `advanced/snapshot-troubleshooting.md` (EN + zh-CN): - **The serializability rule** — what cannot survive the round-trip (native bindings / libuv handles / lazy web-global getters) and when a dependency trips it. - **Failure surfaces** — build-time vs restore-time, with the exact error strings each emits (`killed by signal SIGSEGV`, `no blob was written`, `Check failed: current == end_slot_index`, `Aop Advice not found`, `Cannot find module`, …). - **Find the offending module** — `NODE_DEBUG` namespaces, a clean `NODE_OPTIONS`, `--dry-run`, `--skip-bundle` bisecting, `--force-external` confirmation. - **Fixes** — `--force-external`, `egg.snapshot.lazyModules`, the snapshot lifecycle hooks, deferring work out of module scope, avoiding the web globals. - **Failure modes in detail** (tegg `@Advice` filePath, the lazy-external member proxy, runtime-asset `ENOENT`) and a configuration reference table. Also documents the previously-undocumented `egg.snapshot.lazyModules` config in `advanced/snapshot.md`, cross-links the new page from it, and wires the page into the English and Chinese advanced sidebars. Docs-only; no runtime code change. Builds on the snapshot feature already on `next` (#5998 / #5999 / #6001 / #6003). ## Test evidence - `vitepress build site` passes clean — VitePress's dead-link/anchor check is green, so both new pages render and every cross-file and in-page anchor resolves (the two detail headings use explicit ASCII `{#…}` ids because the VitePress slugifier retains CJK + fullwidth `「」`). - Every technical claim was adversarially verified against the `egg-bundler` / `egg-bin` / `egg-scripts` source — exact error strings, flag names, debug namespaces, and the default lazy-module set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new Snapshot Troubleshooting guide in English and Chinese, covering common build and restore failure symptoms, how to diagnose them, and recommended fixes. * Expanded the Snapshot guide with guidance on configuring additional lazy-loaded modules, plus clearer troubleshooting links. * Updated the sidebar navigation to include the new troubleshooting page in both languages. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Motivation
@eggjs/egg-bundlernow defaults to single-file (snapshot-eligible) output (#5997). This PR adds the mechanism that turns that bundle into a V8 startup snapshot: a 3-mode generated entry, an auto-prepended runtime prelude, anegg-bin snapshot buildcommand, andegg-scripts start --snapshot-blobto boot from the blob.egg core already has the snapshot lifecycle (
buildSnapshot/restoreSnapshot,snapshotWillSerialize/snapshotDidDeserialize,snapshot:truestopping atconfigWillLoad). This wires egg-bundler + egg-bin + egg-scripts to drive it.Scope
EntryGenerator), selected at runtime byEGG_BUNDLE_SNAPSHOT:startEgg+listen(unchanged behavior).EGG_BUNDLE_SNAPSHOT=build) —startEgg({ snapshot: true }), runsnapshotWillSerializehooks, thenv8.startupSnapshot.setDeserializeMainFunction(...).setImmediate(the Node ESM loader isn't ready when the callback runs), installsrequire-based__EGG_MODULE_IMPORTER__/__RUNTIME_REQUIREhooks (viaprocess.getBuiltinModule('node:module'), with an eval-require fallback for Node < 22.3) so the egg loader avoids the missing dynamicimport()callback, resumessnapshotDidDeserialize,listens, and emits anegg-readyIPC message for daemon readiness.prelude.ts) —Bundlerprepends it before the bundle IIFE in snapshot mode so it runs before any module loads. Skeleton/placeholder; PR3 fills the lazy / native-binding-stub mechanism.BundlerConfig.snapshot— forces single-file output (even ifpack.singleFile: false) and triggers the prelude prepend.egg-bin snapshot build— bundle in snapshot mode, thennode --snapshot-blob X --build-snapshot worker.js(clean env so the ts-node loader isn't applied to the bundle; verifies the blob exists). Usesspawn(notfork) so no IPC handle breaks--build-snapshot.egg-scripts start --snapshot-blob <blob>— boots the single self-contained snapshot process (no egg-cluster), reusing the existing daemon / stdout-stderr / signal lifecycle.egg-scripts stoprecognizes snapshot processes by--snapshot-blob+--title.__RUNTIME_REQUIREdeclared in@eggjs/typingsglobal; shared bundle-option helpers extracted tobundleOptions.ts(deduped frombundle.ts).Boundary
This PR is the entry/prelude skeleton + commands. The lazy/stub mechanism that converges non-serializable native bindings lands in PR3, so a live
egg-scripts start --snapshot-blobmay still hit native-binding errors aftersetDeserializeMainFunctionuntil then. The egg-scripts daemon/stop path is unit-tested at the spawn-arg / process-match level; end-to-end daemon boot is verifiable once restore works (PR3).Test evidence
tools/egg-bundler:prelude.test.ts;Bundler.test.tssnapshot-mode cases (forces single-file even withpack.singleFile:false, prepends prelude, fails fast on missing worker.js);EntryGenerator.test.ts3-mode + egg-ready assertions + regenerated canonical snapshot.tools/egg-bin:snapshot.test.ts(build bundles + spawns--build-snapshotwith clean env;--skip-bundle; custom--blob;--dry-run; non-zero exit / missing-blob rejections);bundleOptions.test.ts; existingbundle.test.tsgreen after the helper extraction.tools/scripts:snapshot-start.test.ts(boots vianode --snapshot-blob, no cluster bin;--port→ PORT env);snapshot-stop.test.ts(stop matches snapshot processes by title).Pre-existing, unrelated to this PR: a few
EntryGenerator/ManifestLoadertests fail locally on macOS due to/var→/private/varrealpath (identical on the base branch).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
snapshot build, and snapshot boot support for the start/stop flow.--pack-alias).Bug Fixes
PORT=0is treated as an explicit value.Tests