Draft
Conversation
Add V8 startup snapshot support for eggjs to dramatically reduce cold start time. Snapshot pre-loads all framework modules (295+) into a V8 heap snapshot that can be restored in ~236ms vs ~1632ms for a normal cluster cold start (4-7x speedup). ## Changes by module ### packages/koa - Defer AsyncLocalStorage creation during snapshot building via `v8.startupSnapshot.isBuildingSnapshot()` - Restore ALS in deserialize callback ### packages/core - Add `snapshot` option to `EggCoreOptions` and `LifecycleOptions` - Snapshot mode stops lifecycle after `didLoad` phase (skips willReady/didReady) ### packages/egg - Metadata-only loading: skip timers, event listeners, process handlers in snapshot mode - `registerSnapshotCallbacks()` for serialize/deserialize hooks (loggers, messenger, agent keep-alive timer) - New `buildSnapshot()` / `restoreSnapshot()` APIs in `src/lib/snapshot.ts` - New `startEggForSnapshot()` in `src/lib/start.ts` ### packages/utils - Snapshot module registry support in `importModule()` — use pre-loaded modules from `globalThis.__snapshotModuleRegistry` instead of dynamic imports - Handle restricted `requireForUserSnapshot` in snapshot builder context ### tools/scripts (eggctl) - `eggctl snapshot-build` — new command: generate entry → esbuild bundle to CJS → `node --build-snapshot` - `eggctl start --snapshot-blob <path>` — start from pre-built snapshot blob - `eggctl start --single` — single-process mode (no cluster) - Replace `checkStatus()` polling (1s sleep) with Promise-based IPC wait - Stop command supports snapshot processes (matches `egg-server` title prefix) ### CI - New `test-snapshot` job in GitHub Actions (Node.js 22+24, Ubuntu) ## Performance (21 plugins, 295 modules, Apple Silicon) | Mode | Avg | vs Cold Start | |---------------------------|--------|---------------| | Cold start (cluster) | 1632ms | baseline | | Snapshot via eggctl daemon| 398ms | 4.1x faster | | Snapshot direct node | 236ms | 6.9x faster | ## Known Limitations - `--single` mode with TS source + tegg decorators requires emitDecoratorMetadata (not supported by tsx/esbuild). Use compiled dist/ in production. - Snapshot blob (~30MB) must be rebuilt when app code changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughIntroduces V8 startup snapshot support to Egg.js, enabling faster app startup by serializing the framework heap state. Changes include design documentation, snapshot-aware lifecycle modifications across core/egg/koa packages, snapshot build/restore utility functions, new CLI commands and helper scripts for snapshot generation and execution, test fixtures, and end-to-end test coverage with CI integration. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (snapshot-build)
participant Gen as Generator Script
participant Bundle as esbuild Bundler
participant Builder as Node Build Snapshot
participant V8 as V8 Snapshot API
CLI->>Gen: Generate entry module & registry
Gen->>Gen: Scan framework/app/plugin modules
Gen->>Gen: Create snapshot-entry.mjs with imports
Gen-->>CLI: Output entry & utoopack.json
CLI->>Bundle: Bundle entry module via esbuild
Bundle->>Bundle: Apply custom plugins (http, urllib stubs)
Bundle->>Bundle: Polyfill import.meta & inject globals
Bundle-->>CLI: Output snapshot-bundle.cjs
CLI->>Builder: Execute with --build-snapshot flag
Builder->>V8: Call startupSnapshot.setBuildSnapshot()
V8->>V8: Execute bundle, populate heap with app state
V8->>V8: Serialize heap to .blob file
V8-->>Builder: Complete
Builder-->>CLI: Write snapshot.blob
sequenceDiagram
participant Node as Node.js
participant V8 as V8 Snapshot API
participant Deserialize as Deserialize Main Fn
participant App as Application/Agent
participant Server as HTTP Server
Node->>V8: Start with --snapshot-blob
V8->>V8: Deserialize heap
V8->>Deserialize: Invoke deserialize main function
Deserialize->>App: Restore app instance
Deserialize->>App: Reinit messenger & listeners
Deserialize->>App: Restore ready callbacks
Deserialize->>Server: Create http.Server from app.callback()
Deserialize->>Server: Listen on port
Server-->>V8: Ready
V8-->>Node: Process started
Node->>Server: Handle incoming requests
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
dce7349
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bfc6c4e3.egg-cci.pages.dev |
| Branch Preview URL: | https://worktree-snapshot-poc.egg-cci.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces V8 startup snapshot support for Egg.js, aimed at significantly reducing cold-start times by serializing the initialized application heap. The changes include making the core framework and Koa integration snapshot-aware by deferring non-serializable resources like AsyncLocalStorage, file handles, and timers. A new snapshot-build command has been added to egg-scripts which uses esbuild to bundle a generated entry file containing static imports of all discovered modules, and the start command now supports restoring from these snapshots. Review feedback indicates that the design documentation needs to be updated to match the final implementation of the build command and that the programmatic snapshot APIs should be clarified or consolidated to avoid redundancy.
Comment on lines
+272
to
+284
| #### New command: `snapshot-build` | ||
|
|
||
| Add `tools/scripts/src/commands/snapshot-build.ts`: | ||
|
|
||
| ``` | ||
| egg-scripts snapshot-build [--baseDir] [--framework] [--env] | ||
| ``` | ||
|
|
||
| This command: | ||
|
|
||
| 1. Runs `utoo bundle` to produce a single-file bundle of the app | ||
| 2. Spawns `node --snapshot-blob snapshot.blob --build-snapshot scripts/snapshot-builder.mjs` | ||
| 3. Outputs `snapshot.blob` in the app directory |
Contributor
There was a problem hiding this comment.
This section of the design document appears to be out of sync with the implementation in tools/scripts/src/commands/snapshot-build.ts. The document describes a process using utoo bundle and scripts/snapshot-builder.mjs, but the command implementation uses a different, more complex flow involving scripts/generate-snapshot-entry.mjs and esbuild.
To avoid confusion for future maintainers, could you please update this design document to accurately reflect the final implementation of the snapshot-build command?
Comment on lines
+29
to
+49
| export async function buildSnapshot(options: SnapshotEggOptions = {}): Promise<void> { | ||
| const app = await startEggForSnapshot(options); | ||
|
|
||
| // Register snapshot callbacks on agent and application. | ||
| // These handle cleanup of non-serializable resources (file handles, timers) | ||
| // before snapshot and restoration after deserialize. | ||
| if (app.agent && typeof app.agent.registerSnapshotCallbacks === 'function') { | ||
| app.agent.registerSnapshotCallbacks(); | ||
| } | ||
| app.registerSnapshotCallbacks(); | ||
|
|
||
| v8.startupSnapshot.setDeserializeMainFunction( | ||
| (snapshotData: SnapshotData) => { | ||
| // This function runs when restoring from snapshot. | ||
| // The application object is available via snapshotData. | ||
| // Users should call restoreSnapshot() to get it. | ||
| globalThis.__egg_snapshot_app = snapshotData.app; | ||
| }, | ||
| { app } as SnapshotData, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
The buildSnapshot and restoreSnapshot APIs in this file seem to provide a programmatic way to create snapshots. However, this approach appears to be different from and unused by the main eggctl snapshot-build command, which uses a more elaborate process with generate-snapshot-entry.mjs and esbuild.
This parallel implementation can be confusing. Could you clarify the intended use case for this API? If it's meant for a different purpose, adding some documentation here would be helpful. If it's a remnant of a previous implementation strategy, it might be worth considering its removal to simplify the codebase.
Deploying egg-v3 with
|
| Latest commit: |
dce7349
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5b5cac63.egg-v3.pages.dev |
| Branch Preview URL: | https://worktree-snapshot-poc.egg-v3.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds V8 startup snapshot support across the Egg.js stack to reduce cold-start time by preloading framework/app metadata into a V8 snapshot blob and restoring from it at runtime.
Changes:
- Introduces snapshot build/start workflows in
@eggjs/scripts(snapshot-build,start --snapshot-blob,--single) and adds CI coverage for snapshot tests. - Makes framework lifecycle and runtime components snapshot-aware (stop after
didLoad, defer/restore resources like AsyncLocalStorage, loggers, messenger, keepalive timers). - Enhances module loading to support a snapshot module registry and snapshot-builder constraints.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/scripts/tsdown.config.ts | Configure tsdown unused-deps checking to ignore runtime-only deps (tsx/runscript). |
| tools/scripts/test/snapshot.test.ts | Adds integration tests for snapshot build and starting from a snapshot blob. |
| tools/scripts/test/fixtures/snapshot-app/rebuild.mjs | Fixture helper to rebuild snapshot bundle using esbuild with stubs/deferrals. |
| tools/scripts/test/fixtures/snapshot-app/package.json | Minimal fixture app package manifest. |
| tools/scripts/test/fixtures/snapshot-app/config/config.default.js | Fixture app config for snapshot tests. |
| tools/scripts/test/fixtures/snapshot-app/app/router.js | Fixture route responding with JSON for snapshot validation. |
| tools/scripts/test/fixtures/snapshot-app/.gitignore | Ignores fixture node_modules. |
| tools/scripts/src/index.ts | Exports the new SnapshotBuild command entry. |
| tools/scripts/src/helper.ts | Expands process discovery to include snapshot/title-based processes. |
| tools/scripts/src/commands/stop.ts | Updates stop logic to detect snapshot-started processes. |
| tools/scripts/src/commands/start.ts | Adds --single and snapshot flags, plus IPC-based readiness waiting. |
| tools/scripts/src/commands/snapshot-build.ts | New command to generate entry, bundle to CJS, and run node --build-snapshot. |
| tools/scripts/scripts/start-single.mjs | New ESM single-process start script that sends egg-ready via IPC. |
| tools/scripts/scripts/start-single.cjs | New CJS single-process start script that sends egg-ready via IPC. |
| tools/scripts/scripts/snapshot-builder.mjs | Snapshot builder entry (register callbacks and deserialize main function). |
| tools/scripts/scripts/generate-snapshot-entry.mjs | Generates a snapshot entry module + registry by scanning app/framework/plugins. |
| tools/scripts/package.json | Adds esbuild/tsx deps and exports the new snapshot-build command. |
| pnpm-lock.yaml | Locks new dependencies for tools/scripts (esbuild/tsx). |
| packages/utils/src/import.ts | Adds snapshot module registry support + snapshot-builder require/import handling. |
| packages/koa/test/application/snapshot.test.ts | Tests AsyncLocalStorage deferral/restore behavior during snapshot build. |
| packages/koa/src/application.ts | Defers AsyncLocalStorage creation during snapshot build and restores on deserialize. |
| packages/egg/test/snapshots/index.test.ts.snap | Updates export snapshot to include new snapshot APIs. |
| packages/egg/src/lib/start.ts | Adds startEggForSnapshot() metadata-only loader mode. |
| packages/egg/src/lib/snapshot.ts | Adds buildSnapshot() and restoreSnapshot() APIs. |
| packages/egg/src/lib/egg.ts | Adds snapshot option and serialize/deserialize callbacks for core resources. |
| packages/egg/src/lib/application.ts | Skips event binding during snapshot metadata-only loading. |
| packages/egg/src/lib/agent.ts | Skips keepalive interval in snapshot mode; restores it after deserialization. |
| packages/egg/src/index.ts | Re-exports snapshot utilities from the main egg entrypoint. |
| packages/core/src/lifecycle.ts | Adds snapshot mode to stop lifecycle after didLoad. |
| packages/core/src/egg.ts | Threads snapshot option into lifecycle creation. |
| docs/utoo-bundling.md | Adds documentation on bundling constraints and strategies (utoo). |
| docs/snapshot-test-plan.md | Adds snapshot testing plan and CI strategy. |
| docs/snapshot-design.md | Adds snapshot design document and architecture rationale. |
| .github/workflows/ci.yml | Adds test-snapshot job (Node 22/24) and gates done on it. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comment on lines
+376
to
+380
| child.on('exit', (code) => { | ||
| if (code) { | ||
| clearTimeout(timer); | ||
| reject(new Error(`Child process exited with code ${code}`)); | ||
| } |
There was a problem hiding this comment.
waitForReady() only rejects on exit when code is truthy. If the child exits before sending egg-ready with code === 0 (or exits via signal so code === null), the start command will wait until the timeout instead of failing fast. Treat any exit before readiness as a failure and include the signal/code in the error.
Suggested change
| child.on('exit', (code) => { | |
| if (code) { | |
| clearTimeout(timer); | |
| reject(new Error(`Child process exited with code ${code}`)); | |
| } | |
| child.on('exit', (code, signal) => { | |
| // Treat any exit before readiness as a failure | |
| if (this.isReady) return; | |
| clearTimeout(timer); | |
| let reason: string; | |
| if (code !== null && code !== undefined) { | |
| reason = `code ${code}`; | |
| } else if (signal) { | |
| reason = `signal ${signal}`; | |
| } else { | |
| reason = 'unknown reason'; | |
| } | |
| reject(new Error(`Child process exited before ready with ${reason}`)); |
| this.logToStderr('ignore tail error: %s', tailErr); | ||
| } | ||
| if (this.flags['ignore-stderr']) { | ||
| return; // User opted to ignore stderr errors |
There was a problem hiding this comment.
When --ignore-stderr is set and stderr has content, waitForReady() returns early without calling child.unref() / child.disconnect(). In daemon mode this can keep the CLI process alive due to the IPC handle (and behaves differently from the normal success path). Even if stderr is ignored, the IPC handle should be cleaned up before returning.
Suggested change
| return; // User opted to ignore stderr errors | |
| // User opted to ignore stderr errors; detach child so CLI can exit | |
| try { | |
| child.unref(); | |
| } catch { | |
| // ignore if child is already unrefed or exited | |
| } | |
| try { | |
| child.disconnect(); | |
| } catch { | |
| // ignore if child has no IPC channel or is already disconnected | |
| } | |
| return; |
Comment on lines
+51
to
+58
| const isEggProcess = | ||
| cmd.includes('start-cluster') || | ||
| cmd.includes('start-single') || | ||
| cmd.includes('--snapshot-blob') || | ||
| cmd.startsWith('egg-server'); | ||
| const matched = flags.title | ||
| ? cmd.includes('start-cluster') && cmd.includes(format(osRelated.titleTemplate, flags.title)) | ||
| : cmd.includes('start-cluster'); | ||
| ? isEggProcess && cmd.includes(format(osRelated.titleTemplate, flags.title)) | ||
| : isEggProcess; |
There was a problem hiding this comment.
With --title, the master-process matcher requires the command line to include the JSON "title":"..." template. Snapshot-started processes set the title via process.title and won't include that template in argv, so eggctl stop --title <...> will fail to stop snapshot processes. Consider also matching cmd.startsWith(flags.title) / cmd.includes(flags.title) when --snapshot-blob is present (or when the cmd starts with egg-server).
Comment on lines
+386
to
+401
| const registry = (globalThis as any).__snapshotModuleRegistry as Map<string, any> | undefined; | ||
| if (registry) { | ||
| const cached = registry.get(moduleFilePath); | ||
| if (cached) { | ||
| debug('[importModule:snapshot] found in registry: %s', moduleFilePath); | ||
| let obj = cached; | ||
| if (obj?.default?.__esModule === true && obj.default && 'default' in obj.default) { | ||
| obj = obj.default; | ||
| } | ||
| if (options?.importDefaultOnly) { | ||
| if (obj && 'default' in obj) { | ||
| obj = obj.default; | ||
| } | ||
| } | ||
| return obj; | ||
| } |
There was a problem hiding this comment.
Snapshot registry lookup uses const cached = registry.get(moduleFilePath); if (cached) { ... }. This will incorrectly treat valid falsy exports (e.g. export default 0, '', false) as “not found”. Use registry.has(moduleFilePath) (or compare against undefined) to distinguish missing keys from falsy module values.
Comment on lines
+522
to
+534
| private spawnProcess(command: string, args: string[], options: SpawnOptions): Promise<void> { | ||
| return new Promise<void>((resolve, reject) => { | ||
| debug('spawn: %s %o', command, args); | ||
| const child = spawn(command, args, options); | ||
| child.on('exit', (code) => { | ||
| if (code !== 0) { | ||
| reject(this.error(`Command failed with exit code ${code}`)); | ||
| } else { | ||
| resolve(); | ||
| } | ||
| }); | ||
| child.on('error', reject); | ||
| }); |
There was a problem hiding this comment.
spawnProcess() uses reject(this.error(...)). In oclif, this.error() throws immediately, so the Promise may never reject/resolve correctly and can surface as an unhandled exception from the event handler. Prefer rejecting with a normal Error (or throw from the caller) and handle both code !== 0 and termination by signal (code === null).
Comment on lines
+383
to
+388
| const title = flags.title || `egg-server-${this.pkg.name}`; | ||
| const output = path.resolve(flags.output); | ||
|
|
||
| // Ensure output directory exists | ||
| await mkdir(path.dirname(output), { recursive: true }); | ||
|
|
There was a problem hiding this comment.
const output = path.resolve(flags.output) resolves relative to the current working directory, not the target baseDir. This is inconsistent with start --snapshot-blob (which resolves relative to baseDir) and will surprise users running eggctl snapshot-build from outside the app directory. Consider resolving relative to baseDir (and still supporting absolute paths).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
tools/scripts/tsdown.config.ts (1)
3-9: Consider typed intermediate variable with JSDoc (optional).While the current direct export pattern is consistent with ~94% of tsdown.config.ts files in the repo, the typed intermediate variable pattern improves type clarity and documentation. If adopting this pattern:
♻️ Proposed refactor
import { defineConfig } from 'tsdown'; +import type { UserConfig } from 'tsdown'; -export default defineConfig({ +/** tsdown build config for tools/scripts. */ +const config: UserConfig = defineConfig({ unused: { level: 'error', // tsx is used at runtime via --import=tsx/esm in spawned processes, not directly imported ignore: ['tsx', 'runscript'], }, }); + +export default config;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/scripts/tsdown.config.ts` around lines 3 - 9, The export currently calls defineConfig(...) inline; create a typed intermediate variable (e.g., const tsdownConfig) annotated with a JSDoc type for clarity and documentation, assign the object to that variable (preserving unused.ignore entries and values), and then export it as the default (export default tsdownConfig) so the code uses defineConfig only when typing the variable and the exported symbol is clearly typed and documented.tools/scripts/test/fixtures/snapshot-app/rebuild.mjs (1)
7-35: Consider expanding STATUS_CODES for completeness.The
STATUS_CODESobject in the HTTP stub is missing several commonly used status codes that could be accessed during snapshot building (e.g.,101 Switching Protocolsfor WebSocket upgrades,206 Partial Content,422 Unprocessable Entity,429 Too Many Requests). While the proxy falls back togetReal()for missing properties, accessing these codes would trigger eager loading ofnode:http.♻️ Suggested additions
-const STATUS_CODES = {100:'Continue',101:'Switching Protocols',200:'OK',201:'Created',202:'Accepted',204:'No Content',206:'Partial Content',301:'Moved Permanently',302:'Found',303:'See Other',304:'Not Modified',307:'Temporary Redirect',308:'Permanent Redirect',400:'Bad Request',401:'Unauthorized',403:'Forbidden',404:'Not Found',405:'Method Not Allowed',408:'Request Timeout',409:'Conflict',410:'Gone',413:'Payload Too Large',414:'URI Too Long',415:'Unsupported Media Type',416:'Range Not Satisfiable',422:'Unprocessable Entity',429:'Too Many Requests',500:'Internal Server Error',501:'Not Implemented',502:'Bad Gateway',503:'Service Unavailable',504:'Gateway Timeout'}; +const STATUS_CODES = {100:'Continue',101:'Switching Protocols',102:'Processing',103:'Early Hints',200:'OK',201:'Created',202:'Accepted',203:'Non-Authoritative Information',204:'No Content',205:'Reset Content',206:'Partial Content',207:'Multi-Status',208:'Already Reported',226:'IM Used',300:'Multiple Choices',301:'Moved Permanently',302:'Found',303:'See Other',304:'Not Modified',305:'Use Proxy',307:'Temporary Redirect',308:'Permanent Redirect',400:'Bad Request',401:'Unauthorized',402:'Payment Required',403:'Forbidden',404:'Not Found',405:'Method Not Allowed',406:'Not Acceptable',407:'Proxy Authentication Required',408:'Request Timeout',409:'Conflict',410:'Gone',411:'Length Required',412:'Precondition Failed',413:'Payload Too Large',414:'URI Too Long',415:'Unsupported Media Type',416:'Range Not Satisfiable',417:'Expectation Failed',418:"I'm a Teapot",421:'Misdirected Request',422:'Unprocessable Entity',423:'Locked',424:'Failed Dependency',425:'Too Early',426:'Upgrade Required',428:'Precondition Required',429:'Too Many Requests',431:'Request Header Fields Too Large',451:'Unavailable For Legal Reasons',500:'Internal Server Error',501:'Not Implemented',502:'Bad Gateway',503:'Service Unavailable',504:'Gateway Timeout',505:'HTTP Version Not Supported',506:'Variant Also Negotiates',507:'Insufficient Storage',508:'Loop Detected',510:'Not Extended',511:'Network Authentication Required'};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/scripts/test/fixtures/snapshot-app/rebuild.mjs` around lines 7 - 35, The STATUS_CODES map in the HTTP_STUB proxy is missing several common HTTP status entries (e.g., 101, 206, 422, 429) which forces getReal()/node:http to be loaded when tests access them; update the STATUS_CODES constant (inside the HTTP_STUB string) to include the missing codes such as 101:'Switching Protocols', 206:'Partial Content', 422:'Unprocessable Entity', 429:'Too Many Requests' (and any other commonly used codes you deem necessary) so STATIC contains a more complete set and the Proxy can return them without calling getReal() or loading node:http. Ensure you modify the STATUS_CODES symbol inside the HTTP_STUB template string so the Proxy behavior (get, has, getOwnPropertyDescriptor) continues to use STATIC values..github/workflows/ci.yml (1)
305-311: Consider pinningactions/upload-artifactto a specific commit hash.Other actions in this workflow (checkout, setup-node, codecov) are pinned to specific commit hashes for security and reproducibility. The
actions/upload-artifact@v4tag could be pinned similarly for consistency.♻️ Suggested change
- name: Upload snapshot blob on failure if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: snapshot-blob-${{ matrix.node }} path: tools/scripts/test/fixtures/snapshot-app/*.blob retention-days: 3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 305 - 311, The workflow step named "Upload snapshot blob on failure" currently references actions/upload-artifact@v4; pin this action to a specific commit SHA for reproducibility and security by replacing the tag with the commit hash (e.g., uses: actions/upload-artifact@<sha>) in that step definition so the "uses: actions/upload-artifact@v4" entry is updated to the exact commit reference.packages/egg/src/lib/start.ts (1)
22-29: Add JSDoc for the new public options type.
SnapshotEggOptionsis now part of the exported snapshot API surface, but it is the only new public type here without a top-level doc block.As per coding guidelines "Document all exported functions, classes, and configuration properties with JSDoc comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/egg/src/lib/start.ts` around lines 22 - 29, Add a top-level JSDoc block for the exported interface SnapshotEggOptions describing its purpose as the options shape for snapshot startup (used by the public snapshot API), and document each property: framework (string; absolute path or npm package), baseDir (string; defaults to process.cwd()), env (string), and plugins (EggPlugin). Place the JSDoc immediately above the SnapshotEggOptions declaration so the comment is included in generated docs and IDE tooltips.docs/snapshot-test-plan.md (1)
100-102: Add a CommonJS app to the snapshot test matrix.The plan currently calls out only a TypeScript example or a dedicated fixture. These changes affect module loading and process startup, so we should explicitly run the same workflow against a CommonJS app as well.
Based on learnings "Add regression cases that exercise both CommonJS and TypeScript example apps when features affect HTTP or process orchestration".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/snapshot-test-plan.md` around lines 100 - 102, Update the snapshot test matrix in snapshot-integration.test.ts to include a CommonJS example alongside the existing "helloworld-typescript" and fixture app; add a new test case that runs the same workflow (build/start/request/shutdown) against a CommonJS app (e.g., "helloworld-commonjs") using the same helper(s) used for the TypeScript case so module loading and process startup are exercised; ensure the new case is included in the matrix loop or test table and that any fixtures/setup/teardown functions used by the TypeScript test are reused for the CommonJS run.tools/scripts/test/snapshot.test.ts (1)
223-323: The single-process startup path is still untested.Both suites that would exercise
start --singleare skipped, sostart-single.(mjs|cjs)never runs in CI from this file.Based on learnings: Add regression cases that exercise both CommonJS and TypeScript example apps when features affect HTTP or process orchestration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/scripts/test/snapshot.test.ts` around lines 223 - 323, Tests that exercise the "--single" startup path are skipped (describe.skip for 'start with --single (no snapshot)' and 'snapshot startup performance'), so start-single.(mjs|cjs) never runs in CI; remove the .skip on those suites and add regression cases that explicitly run the single-process start for both CommonJS and TypeScript example apps (use the existing "it('should start in single process mode')" pattern and the snapshot performance flow) — reuse detectPort(), request(), coffee.fork(...) with '--single' and '--snapshot-blob' invocations, and ensure afterEach/afterAll cleanup (coffee.fork(..., ['stop', ...]), cleanup(fixturePath), fs.rm for blobs/logs) so the CommonJS (start-single.cjs) and TS (start-single.mjs or compiled) variants are actually exercised in CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/snapshot-design.md`:
- Around line 40-54: The fenced code blocks containing the BUILD PHASE / RESTORE
PHASE examples (the triple-backtick blocks that start with "BUILD PHASE:" and
"RESTORE PHASE:") are missing language identifiers and are flagged by
markdownlint (MD040); add an appropriate language id (e.g., ```text or ```bash)
to each of those fenced code blocks (and the other similar blocks mentioned) so
the linter stops flagging them, leaving the content unchanged and only adding
the language token to each opening ``` fence.
- Around line 93-105: Summary: The doc incorrectly states bundling is
unnecessary and that startEgg() runs directly during snapshot build; update it
to match the shipped pipeline which bundles the snapshot entry and then runs
Node with --build-snapshot. Fix: Replace the paragraphs that claim "Full
single-file bundling of egg is NOT feasible" and the "Approach: Run egg's full
loader during snapshot build" flow with the actual pipeline description: explain
that generate-snapshot-entry.mjs is produced, esbuild produces a CJS bundle of
that entry, and node --build-snapshot is executed on the bundle (and that
startEgg() executes inside the bundled entry during snapshot runtime, not as an
unbundled loader step), and remove or revise the lines asserting no bundling
step and that the build machine needs full app source/unbundled module
resolution; also update the other similar sections that repeat this erroneous
claim so all references reflect the real flow (generate-snapshot-entry.mjs →
esbuild CJS bundle → node --build-snapshot and where startEgg() actually runs).
In `@docs/snapshot-test-plan.md`:
- Around line 83-86: Update the Koa test path string referenced in the "1.4 Koa:
Snapshot Serialize/Deserialize Round-Trip" section and the summary table so it
points to the actual test location under the application subdirectory (i.e.,
change the referenced test file path that currently omits the application folder
to include it); locate the section header "### 1.4 Koa: Snapshot
Serialize/Deserialize Round-Trip" and the summary table entries and replace the
outdated test path with the correct one that includes the application directory.
In `@docs/utoo-bundling.md`:
- Around line 217-223: Replace the runtime startup call in the example: instead
of using startEgg() in snapshot-build.ts, point the snippet at the metadata-only
snapshot bootstrap by invoking startEggForSnapshot() and then calling
buildSnapshot() to construct the V8 snapshot; update references and surrounding
comments to reflect that this flow avoids running ready hooks and other runtime
side effects.
In `@packages/egg/src/lib/application.ts`:
- Around line 72-74: The load() path skips this.#bindEvents() when snapshot mode
is enabled, so update the deserialize callback registered via
addDeserializeCallback in egg.ts to call this.#bindEvents() after
deserialization; specifically, in the callback ensure you invoke the Application
instance's private `#bindEvents`() (so cookieLimitExceed handler and the server ->
onServer() WebSocket listener are re-registered) once the snapshot is restored.
In `@packages/egg/src/lib/egg.ts`:
- Around line 215-239: Restore the teardown registration even when
this.options.snapshot is true: ensure the same lifecycle.registerBeforeClose
callback (which closes all cluster clients using closeClusterClient over
this.#clusterClients, clears this.#clusterClients, closes the embedded agent via
this.agent!.close() for application single mode, closes each logger in
this.loggers, calls this.messenger.close(), and removes the process
'unhandledRejection' listener bound to this._unhandledRejectionHandler) is
registered for snapshot/deserialized apps as well while still reattaching the
unhandled rejection handler; update the logic around this.options.snapshot to
register that beforeClose handler in both normal and snapshot flows.
In `@packages/egg/src/lib/snapshot.ts`:
- Around line 40-48: The deserialize callback registered with
v8.startupSnapshot.setDeserializeMainFunction currently only assigns
snapshotData.app to globalThis.__egg_snapshot_app; change it so it completes the
normal startup sequence: stash snapshotData.app on globalThis as before, then
call the same logic restoreSnapshot() would run to bootstrap the app (e.g.,
invoke the app start/listen or ready hooks, bind the HTTP server, emit the
'server' event on the application instance, and send the IPC readiness signal
like process.send('egg-ready')). Update the callback body (the function passed
to setDeserializeMainFunction) to perform those startup steps so the snapshot
path becomes ready.
In `@packages/utils/src/import.ts`:
- Around line 403-409: The current snapshot branch in importModule (check on
globalThis.__EGG_SNAPSHOT_CJS_BUNDLE__) returns an empty object which masks
missing modules; change it to fail-fast by throwing a descriptive error
(including moduleFilePath) when a module is not present in the snapshot
registry, or alternatively implement a strict allowlist check and only return {}
for explicitly whitelisted module paths; update the logic in the importModule
function (the branch that logs "[importModule:snapshot] skipping load in
snapshot mode") to either throw the error or consult the allowlist before
returning an empty module so missing modules are caught during snapshot
construction.
In `@tools/scripts/scripts/generate-snapshot-entry.mjs`:
- Around line 508-518: The broadcast of the "egg-ready" event is happening
before server.listen's callback, allowing serverDidReady hooks to run before the
socket is bound; move the call to app.messenger.broadcast("egg-ready") into the
server.listen callback (after computing addr/url and after process.send) so the
broadcast only occurs once listen() has succeeded; remove the earlier pre-listen
broadcast to avoid duplicate/early emissions and ensure you reference
server.listen, process.send and app.messenger.broadcast("egg-ready") in the
change.
- Around line 97-115: The project misses app-level plugins because
discoverPlugins is only called with [frameworkSrcDir]; update every call to
discoverPlugins (the invocation near extractPluginPackages and the other calls
referenced at 147-209 and 325-327) to include the app/config/plugin.* location
(or appSrcDir/config) so app plugin files are traversed as well as framework
files; then expand extractPluginPackages to also recognize "path:" plugin
entries and non-@eggjs imports by (a) adding a regex/pattern to capture package:
path: '...'/path: "..." entries and (b) loosening the importRegex to accept any
import source (not just `@eggjs/`*) so local/unscoped plugin factories are
included; ensure these changes prevent registry misses that currently fall back
to {} in packages/utils/src/import.ts during snapshot restore.
In `@tools/scripts/scripts/snapshot-builder.mjs`:
- Around line 103-120: The snapshot builder currently broadcasts readiness
before the server actually binds; move the app.messenger.broadcast('egg-ready')
(and any other readiness notifications) into the server.listen callback after
computing address (the same place process.send is called) so the 'egg-ready'
event is emitted only once the listen callback runs and the server.address()
(and address.port) are available; update any related usage in the server.listen
callback block (server.listen(...), process.send, server.address()) to ensure
broadcast happens after successful binding.
- Around line 21-31: Replace the normal startEgg() + server creation path with
the snapshot-aware build path: call the snapshot-specific start helper (e.g.
startEggSnapshotBuild or the project's startEgg variant for snapshot builds)
instead of startEgg(), and remove the runtime server creation (the
http.createServer(app.callback()) and app.emit('server', server)) from this
build script so the HTTP server is created later in the deserialize/main
lifecycle gate; ensure the snapshot start helper is used with the same options
(baseDir, framework, env, mode: 'single') so runtime resources are not pulled
into pre-serialize.
In `@tools/scripts/src/commands/snapshot-build.ts`:
- Around line 489-503: The NODE_ENV value is being hard-coded to 'production' in
the env object regardless of --env; update the logic so NODE_ENV uses flags.env
when provided (i.e., set NODE_ENV = flags.env if flags.env is truthy, otherwise
default to 'production'), and keep the existing setting of EGG_SERVER_ENV when
flags.env is present; change the env construction around the env constant (and
reference to flags.env) to derive NODE_ENV dynamically rather than always
'production'.
In `@tools/scripts/test/snapshot.test.ts`:
- Around line 109-115: The snapshot-build invocation in beforeAll uses the real
homedir because the outer beforeEach hasn't set the mocked HOME yet; update the
beforeAll to run coffee.fork for snapshot-build with the same mocked HOME
environment used by the tests (i.e. pass an env that spreads process.env and
overrides HOME to the test fixture home path), so snapshot-build resolves
homedir() against the test fixture (use the same fixture home value you use
elsewhere) and keeps blobPath/fixturePath outputs inside the mocked HOME.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 305-311: The workflow step named "Upload snapshot blob on failure"
currently references actions/upload-artifact@v4; pin this action to a specific
commit SHA for reproducibility and security by replacing the tag with the commit
hash (e.g., uses: actions/upload-artifact@<sha>) in that step definition so the
"uses: actions/upload-artifact@v4" entry is updated to the exact commit
reference.
In `@docs/snapshot-test-plan.md`:
- Around line 100-102: Update the snapshot test matrix in
snapshot-integration.test.ts to include a CommonJS example alongside the
existing "helloworld-typescript" and fixture app; add a new test case that runs
the same workflow (build/start/request/shutdown) against a CommonJS app (e.g.,
"helloworld-commonjs") using the same helper(s) used for the TypeScript case so
module loading and process startup are exercised; ensure the new case is
included in the matrix loop or test table and that any fixtures/setup/teardown
functions used by the TypeScript test are reused for the CommonJS run.
In `@packages/egg/src/lib/start.ts`:
- Around line 22-29: Add a top-level JSDoc block for the exported interface
SnapshotEggOptions describing its purpose as the options shape for snapshot
startup (used by the public snapshot API), and document each property: framework
(string; absolute path or npm package), baseDir (string; defaults to
process.cwd()), env (string), and plugins (EggPlugin). Place the JSDoc
immediately above the SnapshotEggOptions declaration so the comment is included
in generated docs and IDE tooltips.
In `@tools/scripts/test/fixtures/snapshot-app/rebuild.mjs`:
- Around line 7-35: The STATUS_CODES map in the HTTP_STUB proxy is missing
several common HTTP status entries (e.g., 101, 206, 422, 429) which forces
getReal()/node:http to be loaded when tests access them; update the STATUS_CODES
constant (inside the HTTP_STUB string) to include the missing codes such as
101:'Switching Protocols', 206:'Partial Content', 422:'Unprocessable Entity',
429:'Too Many Requests' (and any other commonly used codes you deem necessary)
so STATIC contains a more complete set and the Proxy can return them without
calling getReal() or loading node:http. Ensure you modify the STATUS_CODES
symbol inside the HTTP_STUB template string so the Proxy behavior (get, has,
getOwnPropertyDescriptor) continues to use STATIC values.
In `@tools/scripts/test/snapshot.test.ts`:
- Around line 223-323: Tests that exercise the "--single" startup path are
skipped (describe.skip for 'start with --single (no snapshot)' and 'snapshot
startup performance'), so start-single.(mjs|cjs) never runs in CI; remove the
.skip on those suites and add regression cases that explicitly run the
single-process start for both CommonJS and TypeScript example apps (use the
existing "it('should start in single process mode')" pattern and the snapshot
performance flow) — reuse detectPort(), request(), coffee.fork(...) with
'--single' and '--snapshot-blob' invocations, and ensure afterEach/afterAll
cleanup (coffee.fork(..., ['stop', ...]), cleanup(fixturePath), fs.rm for
blobs/logs) so the CommonJS (start-single.cjs) and TS (start-single.mjs or
compiled) variants are actually exercised in CI.
In `@tools/scripts/tsdown.config.ts`:
- Around line 3-9: The export currently calls defineConfig(...) inline; create a
typed intermediate variable (e.g., const tsdownConfig) annotated with a JSDoc
type for clarity and documentation, assign the object to that variable
(preserving unused.ignore entries and values), and then export it as the default
(export default tsdownConfig) so the code uses defineConfig only when typing the
variable and the exported symbol is clearly typed and documented.
🪄 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: 2bfb1bb6-5c4c-4c49-8dc0-19a19c8fddbb
⛔ Files ignored due to path filters (2)
packages/egg/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.github/workflows/ci.ymldocs/snapshot-design.mddocs/snapshot-test-plan.mddocs/utoo-bundling.mdpackages/core/src/egg.tspackages/core/src/lifecycle.tspackages/egg/src/index.tspackages/egg/src/lib/agent.tspackages/egg/src/lib/application.tspackages/egg/src/lib/egg.tspackages/egg/src/lib/snapshot.tspackages/egg/src/lib/start.tspackages/koa/src/application.tspackages/koa/test/application/snapshot.test.tspackages/utils/src/import.tstools/scripts/package.jsontools/scripts/scripts/generate-snapshot-entry.mjstools/scripts/scripts/snapshot-builder.mjstools/scripts/scripts/start-single.cjstools/scripts/scripts/start-single.mjstools/scripts/src/commands/snapshot-build.tstools/scripts/src/commands/start.tstools/scripts/src/commands/stop.tstools/scripts/src/helper.tstools/scripts/src/index.tstools/scripts/test/fixtures/snapshot-app/.gitignoretools/scripts/test/fixtures/snapshot-app/app/router.jstools/scripts/test/fixtures/snapshot-app/config/config.default.jstools/scripts/test/fixtures/snapshot-app/package.jsontools/scripts/test/fixtures/snapshot-app/rebuild.mjstools/scripts/test/snapshot.test.tstools/scripts/tsdown.config.ts
Comment on lines
+40
to
+54
| ``` | ||
| BUILD PHASE: | ||
| 1. node --snapshot-blob snap.blob --build-snapshot entry.js | ||
| 2. entry.js executes: loads modules, builds app state | ||
| 3. addSerializeCallback()s run: cleanup handles | ||
| 4. V8 serializes heap to snap.blob | ||
| 5. Process exits | ||
|
|
||
| RESTORE PHASE: | ||
| 1. node --snapshot-blob snap.blob [optional-entry.js] | ||
| 2. V8 deserializes heap from snap.blob | ||
| 3. process.env / process.argv refreshed to runtime values | ||
| 4. addDeserializeCallback()s run: restore resources | ||
| 5. setDeserializeMainFunction() callback runs: start app | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language ids to the fenced examples.
markdownlint is already flagging these blocks with MD040.
Also applies to: 108-165, 276-319, 325-349, 392-419, 423-447
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/snapshot-design.md` around lines 40 - 54, The fenced code blocks
containing the BUILD PHASE / RESTORE PHASE examples (the triple-backtick blocks
that start with "BUILD PHASE:" and "RESTORE PHASE:") are missing language
identifiers and are flagged by markdownlint (MD040); add an appropriate language
id (e.g., ```text or ```bash) to each of those fenced code blocks (and the other
similar blocks mentioned) so the linter stops flagging them, leaving the content
unchanged and only adding the language token to each opening ``` fence.
Comment on lines
+93
to
+105
| ### Bundling Decision | ||
|
|
||
| **Full single-file bundling of egg is NOT feasible.** Egg relies on runtime filesystem scanning (`globby.sync`) and dynamic `import()` for controllers, services, plugins, and config files. A bundler cannot follow these dynamic patterns. | ||
|
|
||
| **Approach: Run egg's full loader during snapshot build.** The `snapshot-build.mjs` entry script calls `startEgg()` which runs the entire loader (filesystem scanning, dynamic imports, plugin resolution, config merging, etc.). The resulting fully-initialized app state is captured in the V8 heap snapshot. On restore, the loader does NOT run again -- everything is already in memory. | ||
|
|
||
| This means: | ||
|
|
||
| - No utoo/esbuild bundle step needed | ||
| - The snapshot build machine needs the full app source and `node_modules` | ||
| - The snapshot blob captures all loaded modules in the V8 heap | ||
| - utoo bundling is an **optional optimization** to speed up module resolution during the build phase itself | ||
|
|
Contributor
There was a problem hiding this comment.
The design doc no longer matches the shipped pipeline.
These sections still say bundling is unnecessary and that startEgg() runs during the build phase. The actual CLI flow in tools/scripts/src/commands/snapshot-build.ts is generate-snapshot-entry.mjs → esbuild CJS bundle → node --build-snapshot, so readers following this design will be debugging the wrong architecture.
Also applies to: 390-419, 614-619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/snapshot-design.md` around lines 93 - 105, Summary: The doc incorrectly
states bundling is unnecessary and that startEgg() runs directly during snapshot
build; update it to match the shipped pipeline which bundles the snapshot entry
and then runs Node with --build-snapshot. Fix: Replace the paragraphs that claim
"Full single-file bundling of egg is NOT feasible" and the "Approach: Run egg's
full loader during snapshot build" flow with the actual pipeline description:
explain that generate-snapshot-entry.mjs is produced, esbuild produces a CJS
bundle of that entry, and node --build-snapshot is executed on the bundle (and
that startEgg() executes inside the bundled entry during snapshot runtime, not
as an unbundled loader step), and remove or revise the lines asserting no
bundling step and that the build machine needs full app source/unbundled module
resolution; also update the other similar sections that repeat this erroneous
claim so all references reflect the real flow (generate-snapshot-entry.mjs →
esbuild CJS bundle → node --build-snapshot and where startEgg() actually runs).
Comment on lines
+83
to
+86
| ### 1.4 Koa: Snapshot Serialize/Deserialize Round-Trip | ||
|
|
||
| **File:** `packages/koa/test/snapshot.test.ts` | ||
|
|
Contributor
There was a problem hiding this comment.
Fix the Koa test path in the plan.
The suite added in this PR lives under packages/koa/test/application/snapshot.test.ts, but the plan still points to packages/koa/test/snapshot.test.ts here and in the summary table. That mismatch will send follow-up work to a nonexistent file.
Also applies to: 351-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/snapshot-test-plan.md` around lines 83 - 86, Update the Koa test path
string referenced in the "1.4 Koa: Snapshot Serialize/Deserialize Round-Trip"
section and the summary table so it points to the actual test location under the
application subdirectory (i.e., change the referenced test file path that
currently omits the application folder to include it); locate the section header
"### 1.4 Koa: Snapshot Serialize/Deserialize Round-Trip" and the summary table
entries and replace the outdated test path with the correct one that includes
the application directory.
Comment on lines
+217
to
+223
| ```typescript | ||
| // snapshot-build.ts | ||
| const app = await startEgg({ baseDir: '/app' }); | ||
| // All files are loaded, all plugins resolved, all configs merged | ||
| // Now construct the V8 snapshot from this state | ||
| ``` | ||
|
|
Contributor
There was a problem hiding this comment.
Point this example at the snapshot bootstrap API.
This snippet uses startEgg(), which runs the normal startup path. For the flow introduced in this PR, the metadata-only bootstrap is startEggForSnapshot()/buildSnapshot(); otherwise the doc is telling readers to snapshot an already-booted app with ready hooks and other runtime side effects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/utoo-bundling.md` around lines 217 - 223, Replace the runtime startup
call in the example: instead of using startEgg() in snapshot-build.ts, point the
snippet at the metadata-only snapshot bootstrap by invoking
startEggForSnapshot() and then calling buildSnapshot() to construct the V8
snapshot; update references and surrounding comments to reflect that this flow
avoids running ready hooks and other runtime side effects.
Comment on lines
+72
to
+74
| if (!this.options.snapshot) { | ||
| this.#bindEvents(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if registerSnapshotCallbacks or deserialize hooks re-register these event handlers
rg -n -A 20 'registerSnapshotCallbacks|addDeserializeCallback' packages/egg/src/lib/application.ts packages/egg/src/lib/egg.ts
# Also check if bindEvents is called elsewhere after restore
rg -n 'bindEvents' packages/egg/src/Repository: eggjs/egg
Length of output: 2995
🏁 Script executed:
# Get the full deserialize callback context from egg.ts
sed -n '573,650p' packages/egg/src/lib/egg.tsRepository: eggjs/egg
Length of output: 2991
🏁 Script executed:
# Get the context around lines 72-74 in application.ts
sed -n '65,85p' packages/egg/src/lib/application.tsRepository: eggjs/egg
Length of output: 601
🏁 Script executed:
# Get the `#bindEvents`() method implementation
sed -n '275,320p' packages/egg/src/lib/application.tsRepository: eggjs/egg
Length of output: 1027
Re-register event handlers after snapshot deserialization to restore WebSocket and cookie error handling.
When snapshot mode is enabled, #bindEvents() is intentionally skipped during load() (line 74). However, the addDeserializeCallback hook in egg.ts does not re-register these handlers after snapshot restoration. This means:
- The
cookieLimitExceedevent handler (for logging cookie size errors) won't be available - The
serverevent listener for WebSocket support viaonServer()won't be registered
Add this.#bindEvents() to the deserialize callback in egg.ts to restore these handlers after snapshot restoration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/egg/src/lib/application.ts` around lines 72 - 74, The load() path
skips this.#bindEvents() when snapshot mode is enabled, so update the
deserialize callback registered via addDeserializeCallback in egg.ts to call
this.#bindEvents() after deserialization; specifically, in the callback ensure
you invoke the Application instance's private `#bindEvents`() (so
cookieLimitExceed handler and the server -> onServer() WebSocket listener are
re-registered) once the snapshot is restored.
Comment on lines
+508
to
+518
| lines.push(' server.listen(port, () => {'); | ||
| lines.push(' const addr = server.address();'); | ||
| lines.push(' const url = typeof addr === "string" ? addr : `http://127.0.0.1:${addr.port}`;'); | ||
| lines.push(' console.log("[snapshot] Server started on %s (from snapshot)", url);'); | ||
| lines.push(' if (process.send) {'); | ||
| lines.push(' process.send({ action: "egg-ready", data: { address: url, port: addr?.port ?? port } });'); | ||
| lines.push(' }'); | ||
| lines.push(' });'); | ||
| lines.push(''); | ||
| lines.push(' app.messenger.broadcast("egg-ready");'); | ||
| lines.push(''); |
Contributor
There was a problem hiding this comment.
Emit egg-ready only after listen() succeeds.
serverDidReady() hangs off this event, but the broadcast currently happens before the listen callback. That lets ready hooks run before the socket is actually bound and even on bind failures.
♻️ Possible fix
server.listen(port, () => {
const addr = server.address();
const url = typeof addr === "string" ? addr : `http://127.0.0.1:${addr.port}`;
console.log("[snapshot] Server started on %s (from snapshot)", url);
if (process.send) {
process.send({ action: "egg-ready", data: { address: url, port: addr?.port ?? port } });
}
+ app.messenger.broadcast("egg-ready");
});
-
- app.messenger.broadcast("egg-ready");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/scripts/scripts/generate-snapshot-entry.mjs` around lines 508 - 518,
The broadcast of the "egg-ready" event is happening before server.listen's
callback, allowing serverDidReady hooks to run before the socket is bound; move
the call to app.messenger.broadcast("egg-ready") into the server.listen callback
(after computing addr/url and after process.send) so the broadcast only occurs
once listen() has succeeded; remove the earlier pre-listen broadcast to avoid
duplicate/early emissions and ensure you reference server.listen, process.send
and app.messenger.broadcast("egg-ready") in the change.
Comment on lines
+21
to
+31
| // Initialize the egg app in single mode | ||
| const app = await startEgg({ | ||
| baseDir: options.baseDir, | ||
| framework: options.framework, | ||
| env: options.env, | ||
| mode: 'single', | ||
| }); | ||
|
|
||
| // Create HTTP server (but don't listen yet) | ||
| const server = http.createServer(app.callback()); | ||
| app.emit('server', server); |
Contributor
There was a problem hiding this comment.
Use the snapshot-aware build path here.
This script still calls the normal startEgg() flow and creates the HTTP server during build. That bypasses the new snapshot lifecycle gates and pulls runtime resources into the pre-serialize phase instead of the deserialize main.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/scripts/scripts/snapshot-builder.mjs` around lines 21 - 31, Replace the
normal startEgg() + server creation path with the snapshot-aware build path:
call the snapshot-specific start helper (e.g. startEggSnapshotBuild or the
project's startEgg variant for snapshot builds) instead of startEgg(), and
remove the runtime server creation (the http.createServer(app.callback()) and
app.emit('server', server)) from this build script so the HTTP server is created
later in the deserialize/main lifecycle gate; ensure the snapshot start helper
is used with the same options (baseDir, framework, env, mode: 'single') so
runtime resources are not pulled into pre-serialize.
Comment on lines
+103
to
+120
| server.listen(port, () => { | ||
| const address = server.address(); | ||
| const url = typeof address === 'string' ? address : `http://127.0.0.1:${address.port}`; | ||
|
|
||
| debug('server started on %s (from snapshot)', url); | ||
| console.log('[snapshot] Server started on %s', url); | ||
|
|
||
| // Notify parent process (daemon mode IPC) | ||
| if (process.send) { | ||
| process.send({ | ||
| action: 'egg-ready', | ||
| data: { address: url, port: address?.port ?? port }, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| // Broadcast egg-ready to app and agent messenger | ||
| app.messenger.broadcast('egg-ready'); |
Contributor
There was a problem hiding this comment.
Emit egg-ready after the server is actually listening.
Just like the generated entry, this broadcasts readiness before the listen() callback runs. That can trigger serverDidReady() while the bind is still in flight.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/scripts/scripts/snapshot-builder.mjs` around lines 103 - 120, The
snapshot builder currently broadcasts readiness before the server actually
binds; move the app.messenger.broadcast('egg-ready') (and any other readiness
notifications) into the server.listen callback after computing address (the same
place process.send is called) so the 'egg-ready' event is emitted only once the
listen callback runs and the server.address() (and address.port) are available;
update any related usage in the server.listen callback block
(server.listen(...), process.send, server.address()) to ensure broadcast happens
after successful binding.
Comment on lines
+489
to
+503
| const env: Record<string, string | undefined> = { | ||
| ...process.env, | ||
| NODE_ENV: 'production', | ||
| PATH: [ | ||
| path.join(baseDir, 'node_modules/.bin'), | ||
| path.join(baseDir, '.node/bin'), | ||
| process.env.PATH ?? process.env.Path, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(path.delimiter), | ||
| }; | ||
|
|
||
| if (flags.env) { | ||
| env.EGG_SERVER_ENV = flags.env; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't hard-code NODE_ENV to production when --env says otherwise.
Lines 491-503 bake NODE_ENV=production into every snapshot build, so --env local / --env unittest still execute user code under production semantics. Any config or plugin branching on NODE_ENV will end up serialized with the wrong state.
♻️ Possible fix
const env: Record<string, string | undefined> = {
...process.env,
- NODE_ENV: 'production',
+ NODE_ENV: flags.env === 'prod' ? 'production' : flags.env,
PATH: [
path.join(baseDir, 'node_modules/.bin'),
path.join(baseDir, '.node/bin'),
process.env.PATH ?? process.env.Path,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/scripts/src/commands/snapshot-build.ts` around lines 489 - 503, The
NODE_ENV value is being hard-coded to 'production' in the env object regardless
of --env; update the logic so NODE_ENV uses flags.env when provided (i.e., set
NODE_ENV = flags.env if flags.env is truthy, otherwise default to 'production'),
and keep the existing setting of EGG_SERVER_ENV when flags.env is present;
change the env construction around the env constant (and reference to flags.env)
to derive NODE_ENV dynamically rather than always 'production'.
Comment on lines
+109
to
+115
| beforeAll(async () => { | ||
| // Build the snapshot first | ||
| await coffee.fork(eggBin, ['snapshot-build', '--output', blobPath, fixturePath]).expect('code', 0).end(); | ||
|
|
||
| const stat = await fs.stat(blobPath); | ||
| assert(stat.size > 0, 'snapshot blob should be built before start tests'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Build the shared blob under the mocked HOME too.
This beforeAll runs before the outer beforeEach, so snapshot-build still resolves homedir() against the real environment. That leaks logs/cache outside fixtures/home-snapshot and weakens suite isolation.
♻️ Possible fix
beforeAll(async () => {
- // Build the snapshot first
- await coffee.fork(eggBin, ['snapshot-build', '--output', blobPath, fixturePath]).expect('code', 0).end();
-
- const stat = await fs.stat(blobPath);
- assert(stat.size > 0, 'snapshot blob should be built before start tests');
+ mm(process.env, 'MOCK_HOME_DIR', homePath);
+ try {
+ await coffee.fork(eggBin, ['snapshot-build', '--output', blobPath, fixturePath]).expect('code', 0).end();
+ const stat = await fs.stat(blobPath);
+ assert(stat.size > 0, 'snapshot blob should be built before start tests');
+ } finally {
+ restore();
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| beforeAll(async () => { | |
| // Build the snapshot first | |
| await coffee.fork(eggBin, ['snapshot-build', '--output', blobPath, fixturePath]).expect('code', 0).end(); | |
| const stat = await fs.stat(blobPath); | |
| assert(stat.size > 0, 'snapshot blob should be built before start tests'); | |
| }); | |
| beforeAll(async () => { | |
| mm(process.env, 'MOCK_HOME_DIR', homePath); | |
| try { | |
| await coffee.fork(eggBin, ['snapshot-build', '--output', blobPath, fixturePath]).expect('code', 0).end(); | |
| const stat = await fs.stat(blobPath); | |
| assert(stat.size > 0, 'snapshot blob should be built before start tests'); | |
| } finally { | |
| restore(); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/scripts/test/snapshot.test.ts` around lines 109 - 115, The
snapshot-build invocation in beforeAll uses the real homedir because the outer
beforeEach hasn't set the mocked HOME yet; update the beforeAll to run
coffee.fork for snapshot-build with the same mocked HOME environment used by the
tests (i.e. pass an env that spreads process.env and overrides HOME to the test
fixture home path), so snapshot-build resolves homedir() against the test
fixture (use the same fixture home value you use elsewhere) and keeps
blobPath/fixturePath outputs inside the mocked HOME.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add V8 startup snapshot support for eggjs to dramatically reduce application cold start time. The snapshot pre-loads all framework modules (295+) into a V8 heap snapshot blob that restores in ~236ms vs ~1632ms for a normal cluster cold start.
Key changes:
AsyncLocalStoragecreation during snapshot building viav8.startupSnapshot.isBuildingSnapshot()didLoadphase (skips willReady/didReady)buildSnapshot()/restoreSnapshot()APIs, serialize/deserialize hooksimportModule(), handle restrictedrequireForUserSnapshoteggctl snapshot-buildcommand,--snapshot-blobstart,--singlemode, Promise-based IPC readiness (replaces 1s polling sleep)test-snapshotjob (Node.js 22+24)Performance (21 plugins, 295 modules, Apple Silicon)
Snapshot build pipeline
Usage
Known Limitations
--singlemode with TS source + tegg decorators requiresemitDecoratorMetadata(not supported by tsx/esbuild). Use compileddist/in production.node --build-snapshotrequires CJS (Node.js limitation) — resolved by esbuild bundling step.Test plan
buildSnapshot/restoreSnapshotexported🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
snapshot-buildCLI command to generate optimized snapshots--snapshot-blobflag to start applications from pre-built snapshotsbuildSnapshot,restoreSnapshot) for framework integrationDocumentation