feat(bundler): add Bundler orchestrator#5885
feat(bundler): add Bundler orchestrator#5885killagu wants to merge 1 commit intosplit/13-bundler-pack-runnerfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the core Bundler class and the bundle function for @eggjs/egg-bundler, orchestrating manifest loading, externals resolution, entry generation, and the pack runner execution. It also adds documentation for the bundle output structure and integration tests with fixtures. The review feedback suggests minor optimizations to avoid redundant file writes when patching package.json and to remove an unnecessary sorting operation on the output file list.
| await wrapStep('patch output package.json', async () => { | ||
| const srcPkg = JSON.parse(await fs.readFile(path.join(absBaseDir, 'package.json'), 'utf8')) as { | ||
| name?: string; | ||
| }; | ||
| const outPkg = JSON.parse(await fs.readFile(outputPkgPath, 'utf8')) as Record<string, unknown>; | ||
| if (srcPkg.name) outPkg.name = srcPkg.name; | ||
| await fs.writeFile(outputPkgPath, JSON.stringify(outPkg, null, 2)); | ||
| }); |
There was a problem hiding this comment.
The patching of package.json performs a redundant write if the source project name is missing. Since PackRunner already writes the initial package.json with { "type": "commonjs" }, we should only read and re-write it if there is a name to actually patch.
| await wrapStep('patch output package.json', async () => { | |
| const srcPkg = JSON.parse(await fs.readFile(path.join(absBaseDir, 'package.json'), 'utf8')) as { | |
| name?: string; | |
| }; | |
| const outPkg = JSON.parse(await fs.readFile(outputPkgPath, 'utf8')) as Record<string, unknown>; | |
| if (srcPkg.name) outPkg.name = srcPkg.name; | |
| await fs.writeFile(outputPkgPath, JSON.stringify(outPkg, null, 2)); | |
| }); | |
| await wrapStep('patch output package.json', async () => { | |
| const srcPkg = JSON.parse(await fs.readFile(path.join(absBaseDir, 'package.json'), 'utf8')) as { | |
| name?: string; | |
| }; | |
| if (srcPkg.name) { | |
| const outPkg = JSON.parse(await fs.readFile(outputPkgPath, 'utf8')) as Record<string, unknown>; | |
| outPkg.name = srcPkg.name; | |
| await fs.writeFile(outputPkgPath, JSON.stringify(outPkg, null, 2)); | |
| } | |
| }); |
| framework, | ||
| entries: [{ name: 'worker', source: entries.workerEntry }], | ||
| externals: Object.keys(externalsMap).sort(), | ||
| chunks: [...packResult.files].sort(), |
There was a problem hiding this comment.
The packResult.files array is already sorted by PackRunner.#collectFiles. Sorting it again here is redundant.
| chunks: [...packResult.files].sort(), | |
| chunks: packResult.files, |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new Bundler orchestrator and wires the bundle() entry point to run the end-to-end bundling pipeline, producing a bundle-manifest.json and patching output package.json as needed. Includes initial documentation and integration tests with minimal/empty app fixtures.
Changes:
- Introduces
Bundler(ManifestLoader → ExternalsResolver → EntryGenerator → PackRunner) with step-based error wrapping andbundle-manifest.jsonemission. - Exposes the new public surface (
Bundler,bundle()) and expands package exports for subpath imports. - Adds docs for output structure plus Vitest integration tests and app fixtures.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/egg-bundler/test/integration.test.ts | Adds end-to-end integration tests for bundle() using a minimal fixture and mocked pack output. |
| tools/egg-bundler/test/index.test.ts | Updates public API test to assert Bundler + bundle() are exported. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/tsconfig.json | Adds TS config for the minimal app fixture. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/package.json | Adds package metadata + deps for the minimal app fixture. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/config/config.default.ts | Adds minimal Egg config fixture. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app/service/user.ts | Adds a simple service fixture used by controllers. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app/router.ts | Adds routes fixture for minimal app. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app/middleware/timing.ts | Adds a middleware fixture. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app/extend/context.ts | Adds context extension fixture used by controller response. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app/controller/home.ts | Adds controller fixture. |
| tools/egg-bundler/test/fixtures/apps/minimal-app/app.ts | Adds lifecycle boot fixture. |
| tools/egg-bundler/test/fixtures/apps/empty-app/tsconfig.json | Adds TS config for the empty app fixture. |
| tools/egg-bundler/test/fixtures/apps/empty-app/package.json | Adds package metadata + deps for the empty app fixture. |
| tools/egg-bundler/test/fixtures/apps/empty-app/config/config.default.ts | Adds minimal config fixture for empty app. |
| tools/egg-bundler/test/fixtures/apps/empty-app/app/router.ts | Adds a basic router fixture for empty app. |
| tools/egg-bundler/src/lib/Bundler.ts | Implements Bundler orchestration, error-wrapping, output package.json patching, and bundle-manifest writing. |
| tools/egg-bundler/src/index.ts | Exports Bundler and implements bundle() helper via new Bundler(config).run(). |
| tools/egg-bundler/package.json | Adds subpath exports for new/previously-unexported lib modules. |
| tools/egg-bundler/docs/output-structure.md | Documents expected bundle output layout and bundle-manifest.json schema. |
| import { bundle, type BuildFunc } from '../src/index.ts'; | ||
|
|
||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)); | ||
| const FIXTURE_BASE = path.join(__dirname, 'fixtures/apps/minimal-app'); |
| async function writeFixtureManifest(): Promise<string> { | ||
| const manifestDir = path.join(FIXTURE_BASE, '.egg'); | ||
| await fs.mkdir(manifestDir, { recursive: true }); | ||
| const manifestPath = path.join(manifestDir, 'manifest.json'); | ||
| await fs.writeFile(manifestPath, JSON.stringify(FIXTURE_MANIFEST, null, 2)); | ||
| return manifestPath; | ||
| } | ||
|
|
||
| async function removeFixtureManifest(): Promise<void> { | ||
| await fs.rm(path.join(FIXTURE_BASE, '.egg'), { recursive: true, force: true }); | ||
| } |
|
|
||
| export default (app: Application): void => { | ||
| app.get('/', async (ctx) => { |
| ESM-only packages, peer dependencies, `@eggjs/*`, and the user's | ||
| `externals.force` list) are **not** inlined. They must be installed alongside | ||
| the bundle — typically by copying the app's `package.json` next to | ||
| `worker.js` and running `npm ci --production`, or by deploying the bundle |
Wires ManifestLoader → ExternalsResolver → EntryGenerator → PackRunner with named error wrapping. Writes
bundle-manifest.json(version/mode/entries/externals/chunks). Patches outputpackage.jsonname from source project. Wiresbundle()entry point. Addsdocs/output-structure.md, integration tests with minimal-app + empty-app fixtures.Part of #5863 split. Tracking: #5871.
🤖 Generated with Claude Code