fix(bundler): resolve runtime app baseDir mappings#5920
fix(bundler): resolve runtime app baseDir mappings#5920
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:
📝 WalkthroughWalkthroughGenerated worker entries now compute __outputDir from process.argv[1], synchronously read bundle-manifest.json at startup to obtain __appBaseDir and __framework, root runtime createRequire at __appBaseDir, normalize and alias bundle-map keys for multiple path forms, update the global module loader lookup, and remove compile-time framework-specifier helpers. Tests and docs updated. ChangesWorker Entry Runtime & Bundle Map
Sequence DiagramsequenceDiagram
participant Worker as Generated Worker Entry
participant FS as File System
participant ManifestStore as ManifestStore
participant Loader as Bundle Module Loader
participant Egg as startEgg()
Worker->>FS: read "bundle-manifest.json"
FS-->>Worker: manifest (baseDir, framework, resolveCache...)
Worker->>Worker: set __outputDir from process.argv[1]\ncompute __appBaseDir and __framework
Worker->>Loader: populate __BUNDLE_MAP\n__setBundleMapAlias(rel, app-resolved, output-resolved)
Worker->>ManifestStore: ManifestStore.fromBundle(__appBaseDir)
Worker->>Loader: register module loader (use __toMapKey lookups)
Worker->>Egg: startEgg({ baseDir: __appBaseDir, framework: __framework, mode: 'single' })
Egg-->>Worker: initialized
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: |
9dbf101
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9e06f080.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-4aefd865.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5920 +/- ##
=======================================
Coverage 85.04% 85.04%
=======================================
Files 667 667
Lines 19123 19123
Branches 3723 3723
=======================================
Hits 16263 16263
Misses 2467 2467
Partials 393 393 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request refactors the EntryGenerator to support runtime configuration via a bundle-manifest.json file, enabling dynamic resolution of the application base directory and framework. It also enhances the bundle mapping logic with path normalization and resolveCache alias support. Feedback focuses on improving the robustness of path resolution for __appBaseDir and __outputDir in the generated entry file to prevent issues in varied execution environments.
Deploying egg-v3 with
|
| Latest commit: |
9dbf101
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://15ac5804.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-4aefd865.egg-v3.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR updates @eggjs/egg-bundler’s generated worker entry to correctly distinguish the bundle output directory from the original app baseDir/framework at runtime by reading bundle-manifest.json, and expands runtime module aliasing to cover more path forms used during manifest-backed resolution.
Changes:
- Read
bundle-manifest.jsonat worker runtime to derive__appBaseDirand__framework(while still importingstartEggfromegg). - Expand bundle module map aliasing to include relKey, app absolute paths, output absolute paths, and
manifest.resolveCacherequest aliases. - Update snapshots/tests to reflect the new generated worker output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/egg-bundler/src/lib/EntryGenerator.ts | Generate worker entry that reads bundle-manifest.json, uses __appBaseDir/__framework, and registers additional bundle map aliases. |
| tools/egg-bundler/test/EntryGenerator.test.ts | Adjust assertions and test cases to match the new generated worker runtime behavior and aliasing strategy. |
| tools/egg-bundler/test/snapshots/EntryGenerator.worker.canonical.snap | Update canonical snapshot for the worker entry output. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 227-238: The current __readBundleManifest function swallows all
errors reading/parsing bundle-manifest.json causing __bundleManifest,
__appBaseDir, and __framework to silently fall back and produce misleading
runtime failures; change __readBundleManifest to let failures surface (either
rethrow the error or log and abort startup) when fs.readFileSync or JSON.parse
fails, so that reading bundle-manifest.json throws on invalid/unreadable
content; ensure __bundleManifest is only assigned from this function when it
succeeds, and do not fallback to __outputDir or default framework values
silently.
🪄 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: e5d0a1d4-8cc4-421c-9ffb-d3193b0c32b3
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
tools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/egg-bundler/test/EntryGenerator.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/egg-bundler/docs/output-structure.md (1)
45-47: ⚡ Quick winDocument fallback behavior when
bundle-manifest.jsonis missing/invalid.This section currently reads as a hard requirement, but runtime code falls back to defaults (
baseDir = __outputDir,framework = compiled default) when manifest read/parse fails or fields are absent. Please add one sentence clarifying fallback + likely consequence (app may start with wrong baseDir mappings). Cross-file evidence:tools/egg-bundler/src/lib/EntryGenerator.tsLine 227-246 and Line 282-288.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/docs/output-structure.md` around lines 45 - 47, Update the docs to note that reading/parsing bundle-manifest.json is not fatal: when Bundler/worker fails to read or parse the manifest or misses fields, the runtime falls back to defaults (baseDir is set to __outputDir and framework is set to the compiled default) — add one sentence stating this fallback behavior and the likely consequence that the app may start with incorrect baseDir mappings or an unexpected framework value; reference the relevant symbols bundle-manifest.json, baseDir, framework, __outputDir and EntryGenerator.ts to locate the code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/egg-bundler/docs/output-structure.md`:
- Around line 45-47: Update the docs to note that reading/parsing
bundle-manifest.json is not fatal: when Bundler/worker fails to read or parse
the manifest or misses fields, the runtime falls back to defaults (baseDir is
set to __outputDir and framework is set to the compiled default) — add one
sentence stating this fallback behavior and the likely consequence that the app
may start with incorrect baseDir mappings or an unexpected framework value;
reference the relevant symbols bundle-manifest.json, baseDir, framework,
__outputDir and EntryGenerator.ts to locate the code path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f3a3322-09d9-4ce2-ba9e-142e05dce580
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/egg-bundler/src/lib/EntryGenerator.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tools/egg-bundler/docs/output-structure.md:56
- The
bundle-manifest.jsonexample shows an absolutebaseDir(e.g./abs/path/to/app), but the worker now uses this value to compute__appBaseDirand pass it intostartEgg. If the bundle is meant to be relocatable/self-contained under<outputDir>/, the docs should clarify whetherbaseDiris expected to be relative to<outputDir>(recommended) vs an absolute build-time path (in which case relocations require rewriting the manifest or keeping the original path layout).
A runtime metadata file produced by `Bundler`. The worker reads `baseDir` and
`framework` from this file during startup, so deployment must keep it next to
`worker.js`. The remaining fields are reference / debug metadata. Shape:
```json
{
"version": 1,
"generatedAt": "2026-04-11T00:00:00.000Z",
"mode": "production",
"baseDir": "/abs/path/to/app",
"framework": "egg",
"entries": [{ "name": "worker", "source": "/abs/path/to/app/.egg-bundle/entries/worker.entry.ts" }],
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/egg-bundler/src/lib/EntryGenerator.ts (1)
215-216: 💤 Low valueMinor: Inconsistent quote style in generated imports.
Line 215 uses single quotes while line 216 uses double quotes. This is cosmetic but could be unified for consistency.
import { ManifestStore } from '@eggjs/core'; -import { startEgg } from "egg"; +import { startEgg } from 'egg';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/src/lib/EntryGenerator.ts` around lines 215 - 216, The two import statements use inconsistent quote styles; update the import for startEgg to match the project's single-quote style used by ManifestStore (i.e., change the double quotes around "egg" to single quotes) so both imports use single quotes; look for the import lines referencing ManifestStore and startEgg in EntryGenerator (symbols: ManifestStore, startEgg) and make the quote style consistent across them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 215-216: The two import statements use inconsistent quote styles;
update the import for startEgg to match the project's single-quote style used by
ManifestStore (i.e., change the double quotes around "egg" to single quotes) so
both imports use single quotes; look for the import lines referencing
ManifestStore and startEgg in EntryGenerator (symbols: ManifestStore, startEgg)
and make the quote style consistent across them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d49d52b-618f-4aed-a9e6-cc0b94bfec5d
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.ts
✅ Files skipped from review due to trivial changes (1)
- tools/egg-bundler/docs/output-structure.md
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tools/egg-bundler/test/EntryGenerator.test.ts:290
- This test passes
frameworkintonew EntryGenerator(...), butEntryGeneratorno longer uses that option; the assertions only check that the worker reads__frameworkfrombundle-manifest.jsonat runtime. Keeping the unused option in the unit test setup is misleading. Prefer removingframeworkhere (and validatingbundle-manifest.json.frameworkvia Bundler/integration tests instead).
it('reads a custom framework specifier from bundle-manifest.json at runtime', async () => {
const gen = new EntryGenerator({
baseDir: tmpDir,
framework: '@my-org/framework',
manifestLoader: createFakeLoader(makeManifest()),
});
Summary
Tests
Refs EGG-65.
Summary by CodeRabbit
Refactor
Tests
Documentation