Skip to content

feat(bundler): lazy-external network stack for V8 snapshots #5999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
killagu merged 1 commit into from
Jun 27, 2026
Merged

feat(bundler): lazy-external network stack for V8 snapshots #5999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion tools/egg-bundler/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@ export { Bundler } from './lib/Bundler.ts';
export { EntryGenerator, type EntryGeneratorOptions, type GeneratedEntries } from './lib/EntryGenerator.ts';
export { ExternalsResolver, type ExternalsConfig, type ExternalsResolverOptions } from './lib/ExternalsResolver.ts';
export { ManifestLoader, type ManifestLoaderOptions } from './lib/ManifestLoader.ts';
export { renderSnapshotPrelude, prependSnapshotPrelude, SNAPSHOT_PRELUDE_MARKER } from './lib/prelude.ts';
export {
renderSnapshotPrelude,
prependSnapshotPrelude,
injectExternalRequireLazyHook,
resolveSnapshotLazyModules,
SNAPSHOT_PRELUDE_MARKER,
DEFAULT_SNAPSHOT_LAZY_MODULES,
type ExternalRequireInjectionResult,
} from './lib/prelude.ts';
export {
PackRunner,
type BuildFunc,
Expand Down
137 changes: 107 additions & 30 deletions tools/egg-bundler/src/lib/Bundler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ExternalsResolver } from './ExternalsResolver.ts';
import { assertFrameworkPackageSpecifier } from './frameworkSpecifier.ts';
import { ManifestLoader } from './ManifestLoader.ts';
import { PackRunner } from './PackRunner.ts';
import { prependSnapshotPrelude } from './prelude.ts';
import { injectExternalRequireLazyHook, prependSnapshotPrelude, resolveSnapshotLazyModules } from './prelude.ts';

const debug = debuglog('egg/bundler/bundler');

Expand Down Expand Up @@ -346,6 +346,13 @@ export class Bundler {
const singleFile = snapshot ? true : mergedPack?.singleFile;
debug('snapshot=%s singleFile=%o', snapshot, singleFile);

// The Node network-stack ids (plus the app's egg.snapshot.lazyModules) that must
// stay external so @utoo/pack emits an externalRequire call the prelude can
// intercept, and that the prelude's __LAZY_EXT set names.
const snapshotLazyModules = snapshot
? await wrapStep('resolve snapshot lazy modules', () => resolveSnapshotLazyModules(absBaseDir))
: [];

const manifestLoader = new ManifestLoader({
baseDir: absBaseDir,
manifestPath,
Expand All @@ -359,8 +366,15 @@ export class Bundler {
force: externals?.force,
inline: externals?.inline,
});
const externalsMap = await wrapStep('externals resolve', () => externalsResolver.resolve());
debug('externals resolved: %d packages', Object.keys(externalsMap).length);
const resolvedExternals = await wrapStep('externals resolve', () => externalsResolver.resolve());
debug('externals resolved: %d packages', Object.keys(resolvedExternals).length);

// Keep the lazy network-stack ids external so @utoo/pack does not inline them
// (an inlined http/tls/dns would load — and fail to serialize — at snapshot
// build time). A builtin id maps to itself for the runtime require().
const externalsMap: Record<string, string> = snapshot
? { ...resolvedExternals, ...Object.fromEntries(snapshotLazyModules.map((id) => [id, id])) }
: resolvedExternals;

const entryGen = new EntryGenerator({
baseDir: absBaseDir,
Expand Down Expand Up @@ -402,13 +416,18 @@ export class Bundler {
patchResult.deletedMapCount,
);

// In snapshot mode prepend the snapshot prelude to each entry's worker.js so it
// runs before the bundle IIFE (and therefore before any bundled module loads).
// In snapshot mode inject the lazy-external dispatch into @utoo/pack's
// externalRequire and prepend the prelude to each entry's worker.js so it runs
// before the bundle IIFE (and therefore before any bundled module loads).
if (snapshot) {
const prependedEntries = await wrapStep('prepend snapshot prelude', () =>
this.#prependSnapshotPrelude(absOutputDir, ['worker']),
const applied = await wrapStep('apply snapshot prelude', () =>
this.#applySnapshotPrelude(absOutputDir, ['worker'], snapshotLazyModules, patchResult.outputFiles),
);
debug(
'snapshot: prepended prelude to %d entry file(s), injected lazy hook into %d externalRequire(s)',
applied.prependedEntries.length,
applied.injectedCount,
);
debug('prepended snapshot prelude to %d entry file(s)', prependedEntries.length);
}

const copiedRuntimeAssets = await wrapStep('runtime asset copy', () =>
Expand Down Expand Up @@ -447,32 +466,90 @@ export class Bundler {
};
}

async #prependSnapshotPrelude(outputDir: string, entryNames: readonly string[]): Promise<readonly string[]> {
const prepended: string[] = [];
for (const name of entryNames) {
const rel = this.#sanitizeOutputRelativePath(`${name}.js`);
/**
* Make the bundled output V8-snapshot-eligible:
* 1. Inject the lazy dispatch at the start of every `externalRequire` body (in any
* emitted .js) so a require of a lazy module id routes to the prelude's
* `__makeLazyExt` instead of loading the real (non-serializable) module.
* 2. Prepend the prelude (carrying the resolved lazy id set) to each entry's
* worker.js so it runs before the bundle IIFE.
*/
async #applySnapshotPrelude(
outputDir: string,
entryNames: readonly string[],
lazyModules: readonly string[],
outputFiles: readonly string[],
): Promise<{ prependedEntries: readonly string[]; injectedCount: number }> {
const entrySet = new Set(entryNames.map((name) => this.#sanitizeOutputRelativePath(`${name}.js`)));
const seenEntries = new Set<string>();
const prependedEntries: string[] = [];
let injectedCount = 0;
let sawExternalRequire = false;
let sawInjectedLazyHook = false;

// Single pass over the emitted .js files: inject the lazy hook into every file
// carrying externalRequire (single-file mode keeps it in worker.js; the loop
// stays robust if it ever moves) and, in the same read/write, prepend the
// prelude to entry files so worker.js is only touched once.
for (const rawRel of outputFiles) {
const rel = this.#sanitizeOutputRelativePath(rawRel);
if (!rel.endsWith('.js')) continue;
const isEntry = entrySet.has(rel);

const filepath = path.join(outputDir, rel);
// The entry file is required output; a missing worker.js means the pack
// build did not emit what we expect. Fail fast with a clear error instead
// of silently skipping the prelude (which would surface later as an
// obscure snapshot-build failure).
const content = await fs.readFile(filepath, 'utf8').catch((err: NodeJS.ErrnoException) => {
// Only a missing file means "broken pack output"; preserve the original
// error for permission / other I/O failures so they stay diagnosable.
if (err.code === 'ENOENT') {
throw new Error(`snapshot prelude: expected bundle entry "${rel}" was not found at ${filepath}`, {
cause: err,
});
}
throw err;
});
const next = prependSnapshotPrelude(content);
if (next !== content) {
const original = await fs.readFile(filepath, 'utf8');
// Detect the @utoo/pack helper *definition* independently of our injection
// regex, so a helper that is emitted but not patched (e.g. a codegen format
// our injection regex no longer matches) becomes a loud build error below
// rather than a silent snapshot that loads the network stack at build time.
if (/function\s+externalRequire\b/.test(original)) sawExternalRequire = true;
// A file already carrying the dispatch was patched on a previous run; its
// injectedCount is 0 (idempotent) but it must NOT count as "unpatched".
if (original.includes('globalThis.__makeLazyExt(')) sawInjectedLazyHook = true;
Comment on lines +505 to +508

@coderabbitai coderabbitai Bot Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
import re
detect = re.compile(r'function\s+externalRequire\b')
samples = [
    'const s = "function externalRequire(id) { return id }";',
    '// function externalRequire(id, thunk) {}',
]
for sample in samples:
    print(bool(detect.search(sample)), sample)
PY

Repository: eggjs/egg

Length of output: 255

False positives in externalRequire detection risking snapshot build failures.

The regex /function\s+externalRequire\b/ matches raw bundle text, causing false positives when the phrase appears inside string literals or comments. Execution confirms inputs like 'const s = "function externalRequire(id) { return id }";' and // function externalRequire(id, thunk) {} trigger sawExternalRequire = true.

When combined with the strict failure check at lines 535-544, this causes snapshot builds to crash incorrectly on valid code containing such text. Replace the raw-text regex with an AST-based check or refine the pattern to exclude string/comment contexts.

Code snippet 
if (/function\s+externalRequire\b/.test(original)) sawExternalRequire = true;
🤖 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/Bundler.ts` around lines 505 - 508, The
`Bundler.ts` `externalRequire` detection is too broad because
`/function\s+externalRequire\b/` matches text inside strings and comments, which
can incorrectly trip the later snapshot-build failure path. Update the
`sawExternalRequire` check to use a syntax-aware approach in `Bundler` (or
otherwise restrict the match to real function declarations only) so `original`
is not flagged by literal/comment content, while preserving the existing
`sawInjectedLazyHook` logic.

@killagu killagu Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the string-literal/comment collision is a real theoretical edge, but I'd like to keep the regex here rather than pull in an AST parser, for consistency and proportionality:

  1. Established pattern in this same file. Bundler#patchImportMetaOutput already post-processes the @utoo/pack worker output with regex replacement (TURBOPACK_IMPORT_META_OBJECT, THROWING_IMPORT_META_URL). This PR follows that accepted approach; switching only the snapshot path to AST would be inconsistent.
  2. Cost vs. risk. The single-file worker is multi-MB; running acorn/@babel over it on every snapshot build (plus a new parser dependency) is heavy for a collision that needs the Turbopack-internal token externalRequire to appear in function-signature shape inside a string literal in a bundled dep — vanishingly unlikely for that specific identifier.
  3. Bounded failure. The injection is idempotent (skips when __makeLazyExt( is present), and the fail-loud check only triggers on a function externalRequire definition that was emitted but left unpatched. The snapshot-lazy.realbuild.test.ts exercises the real @utoo/pack output end-to-end.

Happy to switch to an AST-scoped injection in a follow-up if you'd prefer the stronger guarantee — just let me know.

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@killagu, that makes sense — thanks for the detailed rationale. I’m okay leaving this as-is for this PR given the consistency with the existing regex-based post-processing in tools/egg-bundler/src/lib/Bundler.ts, the low practical risk, and the real-build coverage you added. If it ever shows up in the wild, an AST-scoped follow-up would be a reasonable escalation.

🐇💭

✏️ Learnings added 
Learnt from: killagu
Repo: eggjs/egg PR: 5999
File: tools/egg-bundler/src/lib/Bundler.ts:505-508
Timestamp: 2026-06-26T18:36:56.614Z
Learning: In `tools/egg-bundler/src/lib/Bundler.ts`, maintainers prefer regex-based post-processing of emitted `utoo/pack` worker output (including snapshot-prelude handling) over adding an AST parser, for consistency with `Bundler#patchImportMetaOutput` and to avoid the cost of parsing multi-megabyte single-file bundles unless a real issue justifies the stronger approach.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const hooked = injectExternalRequireLazyHook(original);
injectedCount += hooked.injected;

let next = hooked.content;
if (isEntry) {
next = prependSnapshotPrelude(next, lazyModules);
seenEntries.add(rel);
prependedEntries.push(rel);
}
if (next !== original) {
await fs.writeFile(filepath, next);
prepended.push(rel);
}
}
return prepended;

// The entry file is required output; a missing worker.js (absent from the pack
// output enumeration) means the build did not emit what we expect. Fail fast
// with a clear error instead of silently skipping the prelude (which would
// surface later as an obscure snapshot-build failure).
for (const rel of entrySet) {
if (!seenEntries.has(rel)) {
throw new Error(
`snapshot prelude: expected bundle entry "${rel}" was not found at ${path.join(outputDir, rel)}`,
);
}
}

if (injectedCount === 0) {
if (sawExternalRequire && !sawInjectedLazyHook) {
// The helper was emitted but our injection regex matched nothing: the lazy
// dispatch is absent, so the snapshot build would load the network stack and
// fail to serialize. Fail loud instead of producing a broken blob.
throw new Error(
'snapshot prelude: an externalRequire helper was emitted but the lazy hook could not be ' +
'injected (its signature did not match). The bundle would load the network stack at ' +
'snapshot-build time.',
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
// Otherwise benign: a bundle that never requires an external has nothing to
// defer. A real app that touches the network stack always produces an
// externalRequire, so this only fires for trivial/synthetic bundles.
debug('snapshot: no externalRequire helper emitted — lazy hook injected nowhere');
}

return { prependedEntries, injectedCount };
}

async #copyRuntimeAssets(
Expand Down
Loading
Loading