feat(bundler): support app-supplied pack aliases#5914
Conversation
📝 WalkthroughWalkthroughAdds optional pack resolve alias support: new ChangesPack resolve alias wiring
Sequence DiagramsequenceDiagram
participant CLI as egg-bin CLI
participant Bundler as Bundler
participant PackRunner as PackRunner
participant Pack as "@utoo/pack" (buildFunc)
CLI->>Bundler: parse `--pack-alias` → alias map (resolve relative targets)
Bundler->>PackRunner: construct with options (rootPath, mode, buildFunc, resolve.alias)
PackRunner->>Pack: call buildFunc({ ..., resolve: { alias } }) when non-empty
Pack->>PackRunner: return build result
PackRunner->>Bundler: return build output
Bundler->>CLI: complete bundle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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: |
54a8592
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d08344dc.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-0fb70fdf.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
54a8592
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://80f33471.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-0fb70fdf.egg-v3.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #5914 +/- ##
==========================================
+ Coverage 85.03% 85.04% +0.01%
==========================================
Files 667 667
Lines 19110 19123 +13
Branches 3719 3723 +4
==========================================
+ Hits 16250 16263 +13
Misses 2467 2467
Partials 393 393 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tools/egg-bundler/test/PackRunner.test.ts (1)
112-141: ⚡ Quick winCover the hoisted
node_moduleslayout too.This fixture only proves the direct
supports-hyperlinks/node_modules/supports-colorcase. The new resolver’s easy-to-break branch is the upward walk to a parentnode_modules, which is the layout the pnpm/hoisted reproduction is more likely to hit.Fixture tweak that exercises the upward walk
const supportsHyperlinksDir = path.join(tmpDir, 'node_modules', 'supports-hyperlinks'); - const supportsColorDir = path.join(supportsHyperlinksDir, 'node_modules', 'supports-color'); + const supportsColorDir = path.join(tmpDir, 'node_modules', 'supports-color');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/test/PackRunner.test.ts` around lines 112 - 141, The test 'aliases supports-color to its node export when bundled through supports-hyperlinks' only creates supports-color under supports-hyperlinks/node_modules; to cover the hoisted layout also create a hoisted package in the project-level node_modules and its files so the resolver's upward-walk code is exercised. Concretely: in the test add a supportsColorHoistedDir = path.join(tmpDir, 'node_modules', 'supports-color'), write a package.json and index.js/index.d.ts (matching the exports shape used) into that hoisted dir (and ensure supports-hyperlinks package.json still exists), then run makeRunner({ buildFunc }).run() and assert the alias resolves to the hoisted path (path.join(tmpDir, 'node_modules', 'supports-color', 'index.js')) so the upward-walk branch is exercised; reference the existing symbols buildFunc, makeRunner, supportsHyperlinksDir, supportsColorDir, and the expect on config.resolve.alias to locate where to change the fixture.tools/egg-bundler/src/lib/PackRunner.ts (1)
185-199: Handle nested condition objects withinexports.node.The
nodecondition can itself contain a conditional object withrequire,import, ordefaultproperties. With shapes like{ node: { require: './index.cjs', default: './index.js' } }, the current implementation returnsundefinedand silently falls back to non-node resolution.Suggested fix
- `#resolvePackageTarget`(target: unknown): string | undefined { + `#resolvePackageTarget`(target: unknown, inNodeCondition = false): string | undefined { if (typeof target === 'string') return target; if (Array.isArray(target)) { for (const item of target) { - const resolved = this.#resolvePackageTarget(item); + const resolved = this.#resolvePackageTarget(item, inNodeCondition); if (resolved) return resolved; } return undefined; } if (!target || typeof target !== 'object') return undefined; const map = target as Record<string, unknown>; - if (!Object.hasOwn(map, 'node')) return undefined; - return this.#resolvePackageTarget(map.node); + if (Object.hasOwn(map, 'node')) { + return this.#resolvePackageTarget(map.node, true); + } + if (inNodeCondition) { + return this.#resolvePackageTarget(map.require ?? map.default ?? map.import, true); + } + return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/src/lib/PackRunner.ts` around lines 185 - 199, The `#resolvePackageTarget` currently treats map.node as a scalar or a nested conditional array but doesn't handle when map.node is itself a conditional object with keys like require/import/default; update `#resolvePackageTarget` to detect when map.node is an object containing any of 'require', 'import', or 'default' and recursively resolve in a preferred order (e.g., prefer 'require', then 'import', then 'default') by calling this.#resolvePackageTarget on the chosen property value, falling back to other keys if the chosen one returns undefined; ensure you still handle arrays and other nested condition shapes the same way so that inputs like { node: { require: './index.cjs', default: './index.js' } } return './index.cjs'.
🤖 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/PackRunner.ts`:
- Around line 185-199: The `#resolvePackageTarget` currently treats map.node as a
scalar or a nested conditional array but doesn't handle when map.node is itself
a conditional object with keys like require/import/default; update
`#resolvePackageTarget` to detect when map.node is an object containing any of
'require', 'import', or 'default' and recursively resolve in a preferred order
(e.g., prefer 'require', then 'import', then 'default') by calling
this.#resolvePackageTarget on the chosen property value, falling back to other
keys if the chosen one returns undefined; ensure you still handle arrays and
other nested condition shapes the same way so that inputs like { node: {
require: './index.cjs', default: './index.js' } } return './index.cjs'.
In `@tools/egg-bundler/test/PackRunner.test.ts`:
- Around line 112-141: The test 'aliases supports-color to its node export when
bundled through supports-hyperlinks' only creates supports-color under
supports-hyperlinks/node_modules; to cover the hoisted layout also create a
hoisted package in the project-level node_modules and its files so the
resolver's upward-walk code is exercised. Concretely: in the test add a
supportsColorHoistedDir = path.join(tmpDir, 'node_modules', 'supports-color'),
write a package.json and index.js/index.d.ts (matching the exports shape used)
into that hoisted dir (and ensure supports-hyperlinks package.json still
exists), then run makeRunner({ buildFunc }).run() and assert the alias resolves
to the hoisted path (path.join(tmpDir, 'node_modules', 'supports-color',
'index.js')) so the upward-walk branch is exercised; reference the existing
symbols buildFunc, makeRunner, supportsHyperlinksDir, supportsColorDir, and the
expect on config.resolve.alias to locate where to change the fixture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 164fbbb7-4027-47b7-80b3-545724dfaff1
📒 Files selected for processing (2)
tools/egg-bundler/src/lib/PackRunner.tstools/egg-bundler/test/PackRunner.test.ts
![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 introduces a mechanism to resolve and alias specific dependencies to their Node.js entry points, addressing an issue where the bundler incorrectly selects browser exports. It includes logic for traversing the file system to find package directories and parsing the exports field in package.json. Feedback was provided to improve the robustness of the export resolution logic by supporting additional Node.js conditions such as require, import, and default as fallbacks.
There was a problem hiding this comment.
Pull request overview
Fixes a Node bundling runtime issue where @utoo/pack selects supports-color’s browser/default export (via the supports-hyperlinks -> supports-color chain) instead of the Node export.
Changes:
- Add conditional
resolve.aliasinjection inPackRunnerto pointsupports-colorat itsexports.nodeentry when applicable. - Add a unit test covering the alias behavior and assert
resolveis otherwise omitted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/egg-bundler/src/lib/PackRunner.ts | Detects supports-hyperlinks + supports-color and conditionally injects resolve.alias to force the Node export path. |
| tools/egg-bundler/test/PackRunner.test.ts | Adds coverage validating the alias is emitted only when the dependency chain exists. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/egg-bundler/src/lib/PackRunner.ts (1)
175-207: 💤 Low value
#resolveExportsNodeEntry: top-level array exports are silently dropped while nested arrays are handled.Line 177 rejects array-form root exports early and returns
undefined:if (!exportsField || typeof exportsField !== 'object' || Array.isArray(exportsField)) return undefined;
#resolvePackageTargethandles arrays recursively (lines 187-193), so the gap only affects top-level fallback arrays like"exports": ["./index.cjs", "./index.mjs"]. These are rare and the Node.js spec discourages them at root level, but if a futureNODE_CONDITION_ALIAS_DEPSentry targets such a package the alias would be silently skipped rather than resolved.Consider routing top-level arrays through
#resolvePackageTargetfor consistency:♻️ Optional fix
`#resolveExportsNodeEntry`(exportsField: unknown): string | undefined { if (typeof exportsField === 'string') return exportsField; - if (!exportsField || typeof exportsField !== 'object' || Array.isArray(exportsField)) return undefined; + if (!exportsField || typeof exportsField !== 'object') return undefined; + if (Array.isArray(exportsField)) return this.#resolvePackageTarget(exportsField); const map = exportsField as Record<string, unknown>;For
#resolvePackageTargetline 197: thenodecondition exits early even when it resolves toundefined(e.g."node": nullor a types-only node block). This is correct per the Node.js exports spec — a matched-but-unresolved condition does not fall through todefault. In the current workaround this is fine sincesupports-color'snodecondition always yields a valid path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/src/lib/PackRunner.ts` around lines 175 - 207, The top-level array branch in `#resolveExportsNodeEntry` silently returns undefined for array-form exports; instead allow arrays to be processed by the existing recursive resolver: change the early-return so that only null/non-objects are rejected (keep the typeof check) but if Array.isArray(exportsField) (or generally when it's an object/array) call and return this.#resolvePackageTarget(exportsField) so root-level arrays like ["./index.cjs","./index.mjs"] are handled consistently by `#resolvePackageTarget`; leave the current `#resolvePackageTarget` node/condition behavior unchanged.
🤖 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/PackRunner.ts`:
- Around line 175-207: The top-level array branch in `#resolveExportsNodeEntry`
silently returns undefined for array-form exports; instead allow arrays to be
processed by the existing recursive resolver: change the early-return so that
only null/non-objects are rejected (keep the typeof check) but if
Array.isArray(exportsField) (or generally when it's an object/array) call and
return this.#resolvePackageTarget(exportsField) so root-level arrays like
["./index.cjs","./index.mjs"] are handled consistently by `#resolvePackageTarget`;
leave the current `#resolvePackageTarget` node/condition behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24e0dbbf-a759-4629-b777-3e12ffb42b0f
📒 Files selected for processing (2)
tools/egg-bundler/src/lib/PackRunner.tstools/egg-bundler/test/PackRunner.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/egg-bundler/test/PackRunner.test.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tools/egg-bundler/src/lib/PackRunner.ts:106
- The PR title/description mention forcing the Node export for the supports-hyperlinks -> supports-color chain, but this PackRunner change only adds pass-through support for application-supplied
resolve.aliasand does not implement any automatic aliasing for supports-color (nor is it covered by tests in this PR). Either add the intended auto-alias behavior here, or update the PR title/description to reflect the actual change (exposingpack.resolve.alias).
const resolveConfig = this.#buildResolveConfig(resolve);
const config = {
entry: entries.map((e) => ({ name: e.name, import: e.filepath })),
target: 'node 22',
platform: 'node',
mode,
output: {
path: outputDir,
type: 'standalone',
},
externals: umdExternals,
...(resolveConfig ? { resolve: resolveConfig } : {}),
optimization: {
treeShaking: false,
minify: false,
},
};
killagu
changed the title
Summary
pack.resolve.aliassupport.egg-bin bundle --pack-alias <specifier>=<target>wiring so applications can provide dependency-specific pack aliases at the app layer.Verification
pnpm vitest run test/Bundler.test.ts test/PackRunner.test.tsfromtools/egg-bundlerpnpm vitest run test/commands/bundle.test.tsfromtools/egg-binpnpm --filter @eggjs/egg-bundler typecheckpnpm --filter @eggjs/bin typecheckpnpm --filter @eggjs/egg-bundler lintpnpm --filter @eggjs/egg-bundler testNotes
--pack-aliaskeeps package-style targets as-is; only dot-relative targets such as./target.jsand../target.jsare resolved from the application base directory.