fix(bundler): skip app close after manifest generation#5902
Closed
killagu wants to merge 17 commits into
Closed
Conversation
Add `setBundleModuleLoader()` which intercepts `importModule()` BEFORE `importResolve()` so bundled apps can redirect module lookups to an in-bundle map without the source files existing on disk. Reuses the existing `__esModule` double-default and `importDefaultOnly` handling. State is stored on `globalThis.__EGG_BUNDLE_MODULE_LOADER__` so that bundled and external copies of @eggjs/utils share the same loader — required because a bundled entry and the external egg framework may resolve to different module instances. Required by the upcoming @eggjs/egg-bundler to bootstrap a bundled Egg app without filesystem discovery.
Add `ManifestStore.fromBundle(data, baseDir)` that creates a store from pre-validated bundled manifest data, skipping the lockfile / config fingerprint checks that would fail for a frozen artifact shipped across environments. Only the manifest `version` field is validated — the caller (bundler) is responsible for guaranteeing the data matches the artifact. Required by the upcoming @eggjs/egg-bundler to bootstrap a bundled Egg app without re-running filesystem discovery.
Add `ManifestStore.setBundleStore(store)` / `getBundleStore()` that register a pre-built store. `ManifestStore.load()` now returns the registered store unconditionally when present, bypassing the disk read and invalidation checks. The bundler-generated entry calls this at startup (before `new Application()`) so the loader uses the frozen manifest inlined into the bundle instead of looking on disk — bundled apps don't ship `.egg/manifest.json` as a separate file. State is stored on `globalThis.__EGG_BUNDLE_STORE__` so that bundled and external copies of @eggjs/core share the same store instance — required because a bundled entry and the external egg framework may resolve to different module instances. Pairs with `ManifestStore.fromBundle()` to complete the runtime side of the bundler's manifest plumbing.
Introduce the @eggjs/egg-bundler workspace package skeleton under tools/egg-bundler. Subsequent commits add ExternalsResolver, ManifestLoader, the generate-manifest subprocess, and the public BundlerConfig API on top of this scaffold. Wiring: - Added tools/egg-bundler to root tsconfig.json references. - Added @utoo/pack to the workspace catalog (tsx already present). - Root tsdown.config.ts: externalize @utoo/pack and any .node binary so rolldown does not attempt to analyse the NAPI-RS prebuilt binaries shipped by @utoo/pack and its transitive domparser-rs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add ExternalsResolver that identifies packages that must not be inlined by the bundler: - Native addons: install/postinstall/preinstall scripts matching node-gyp/prebuild-install/napi-rs/node-pre-gyp/electron-rebuild, binding.gyp, prebuilds/, *.node files. - ESM-only packages (type=module without a require export condition). - Hard-coded always-external entries (egg, @eggjs/*, @swc/helpers). - All peerDependencies of the application. User force/inline overrides are supported: force always wins over inline when both reference the same package name. Fixtures under test/fixtures/externals/basic-app cover each tier plus the override cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add ManifestLoader that loads `.egg/manifest.json`, normalizes fileDiscovery/resolveCache/extensions.tegg keys from realpath form (e.g. `../../plugins/static` under workspace links, or `.pnpm/...` under real pnpm installs) into stable `node_modules/<pkg>/<sub>` form via longest-prefix matching against the app + framework dependency map. The #generate() subprocess wiring is intentionally minimal in this commit (plain fork() into scripts/generate-manifest.mjs); the tsx loader injection and spawn/frameworkEntry handling are added in the next commit alongside the subprocess script itself. The ManifestStore.fromBundle wiring is left as a TODO until the core runtime split exposes that factory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tion Add scripts/generate-manifest.mjs which boots the egg framework to produce a manifest via app.loader.generateManifest() and writes it through ManifestStore.write(). The script accepts both: - `framework` (absolute package dir) — forwarded to framework.start() so egg's resolveFrameworkClasses() keeps working via importResolve. - `frameworkEntry` (file:// URL) — used for the subprocess's initial dynamic import(), which is the only way to load a workspace-linked framework whose `exports` map points at a TypeScript source. ManifestLoader changes to drive the script: - Switch from fork() to spawn(): tsx's ESM loader has resolver issues inside Node 22's IPC hooks-worker, causing workspace-linked packages with `exports: "./src/*.ts"` to fall back to directory resolution. - #buildExecArgv() injects `--import=file://.../tsx/dist/esm/index.mjs` unless a tsx loader is already present. The detection regex accepts bare specifiers and absolute file:// URLs to prevent recursive duplication. - #resolveFrameworkEntryUrl() reads the framework package.json's exports['.'] (string or conditional) and falls back to module/main, returning an absolute file:// URL. tsdown.config.ts: land the script at dist/scripts/generate-manifest.mjs via a copy rule so the subprocess resolves correctly in both dev (workspace link) and publish (tgz install) forms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce the BundlerConfig type surface that subsequent commits (Bundler orchestration + egg-bin CLI wrapper) will consume. bundle() still throws until the orchestration lands. Also re-export ExternalsResolver and ManifestLoader from the package entry so consumers can reach them directly from '@eggjs/egg-bundler'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…5890) Adds `Bundler.#patchImportMetaUrl()` post-process step that walks output JS files and rewrites turbopack's throwing `import.meta.url` / `import.meta.dirname` getters into working forms. Wired into `Bundler.run()` after PackRunner with named error wrapping. This is the final PR in the #5863 split. With all 19 PRs merged, the full bundler functionality from #5863 is available. Part of #5863 split. Tracking: #5871. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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: |
89be545
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fbaa8a38.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-6c808162-manif.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
89be545
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://679d7993.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-6c808162-manif.egg-v3.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 modifies the manifest generation script to prevent the execution of application lifecycle hooks, such as beforeClose, which can cause issues during bundle metadata collection. It introduces a helper to flush standard output and error streams before exiting the process directly, bypassing app.close(). Additionally, a new test suite has been added to verify that the manifest is correctly generated without triggering these side effects. I have no feedback to provide.
killagu
force-pushed
the
split/18-bundler-remove-eggjs-externals
branch
5 times, most recently
from
9065d0c to
07f9715
Compare
Contributor
Author
|
Superseded by #5904. Rebuilt this as a smaller core-level fix on top of current next so the metadataOnly close behavior is addressed in one place. |
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
1 participant
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
Tests
Refs: EGG-18