feat(utils): add setBundleModuleLoader runtime hook#5867
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:
📝 WalkthroughWalkthroughAdds a global bundle module loader and makes importModule consult it first (posix-normalizing paths), returning loader-provided values with interop/default-unwrapping and honoring importDefaultOnly; falls back to normal resolution when the loader returns undefined. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ImportModule as importModule
participant BundleLoader as BundleModuleLoader
participant Resolver
Client->>ImportModule: importModule(filepath, options)
ImportModule->>ImportModule: normalize filepath (\\ -> /)
alt loader registered
ImportModule->>BundleLoader: BundleModuleLoader(normalized_filepath)
BundleLoader-->>ImportModule: module | undefined
alt module returned
ImportModule->>ImportModule: unwrap defaults / apply importDefaultOnly
ImportModule-->>Client: return processed module
else undefined
ImportModule->>Resolver: importResolve / existing flows
Resolver-->>Client: return resolved module
end
else no loader
ImportModule->>Resolver: importResolve / existing flows
Resolver-->>Client: return resolved module
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5867 +/- ##
==========================================
- Coverage 85.51% 85.51% -0.01%
==========================================
Files 662 662
Lines 18863 18876 +13
Branches 3658 3662 +4
==========================================
+ Hits 16131 16142 +11
Misses 2361 2361
- Partials 371 373 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 BundleModuleLoader to allow bundled applications to serve modules that may not exist on disk. It adds a global registration mechanism via setBundleModuleLoader and updates importModule to intercept requests using the registered loader. Feedback highlights potential consistency issues with the isESM flag when multiple package instances exist and the need to restore the original isESM state when the loader is unset to ensure test isolation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/utils/src/import.ts`:
- Around line 410-415: The code uses (globalThis as
any).__EGG_BUNDLE_MODULE_LOADER__ which weakens typing; declare a typed global
interface for __EGG_BUNDLE_MODULE_LOADER__ (e.g. interface Global {
__EGG_BUNDLE_MODULE_LOADER__?: BundleModuleLoader }) via declare global so you
can replace both (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ occurrences
with properly typed accesses, and keep the existing logic in importModule and
the loader assignment (references: __EGG_BUNDLE_MODULE_LOADER__,
BundleModuleLoader, importModule, loader, isESM).
- Around line 409-412: setBundleModuleLoader currently sets (globalThis as
any).__EGG_BUNDLE_MODULE_LOADER__ and flips isESM = false on registration but
never restores isESM when loader is unset, causing importModule to incorrectly
use require(); update setBundleModuleLoader(loader) to set
__EGG_BUNDLE_MODULE_LOADER__ and set isESM = loader ? false : true (or restore
previous ESM state) so unregistering (loader === undefined) resets isESM; also
remove the (globalThis as any) casts by adding a globalThis augmentation that
declares __EGG_BUNDLE_MODULE_LOADER__?: BundleModuleLoader and use that typed
property instead of any to access the global.
🪄 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: a64667b7-dc8d-4ad0-9fcc-ba23be486dc5
⛔ Files ignored due to path filters (1)
packages/utils/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/utils/src/import.tspackages/utils/test/bundle-import.test.ts
There was a problem hiding this comment.
Pull request overview
Adds a bundle-aware runtime hook to @eggjs/utils so a bundled Egg application can intercept importModule() calls and serve modules from an in-bundle map (avoiding disk-based resolution/scanning).
Changes:
- Add
BundleModuleLoader+setBundleModuleLoader()and consult the loader at the start ofimportModule(). - Add Vitest coverage for bundle-loader behavior (hit, miss/fallthrough, default unwrapping, virtual paths).
- Update the “export all” snapshot to include the new API.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces setBundleModuleLoader() (stored on globalThis) and adds a pre-importResolve interception path in importModule(). |
| packages/utils/test/bundle-import.test.ts | New tests validating bundle interception, default handling, fallthrough behavior, and virtual module paths. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
killagu
force-pushed
the
split/01-utils-bundle-module-loader
branch
from
acb9a67 to
f97af39
Compare
|
已按 review comments 调整完这轮修改：去掉了 |
|
Fixed the review comments in a0fc95b: setBundleModuleLoader no longer mutates isESM, globalThis.EGG_BUNDLE_MODULE_LOADER is typed, and bundle path normalization now uses node:path constants. Added regression coverage for ESM fallback on loader miss and Windows-style bundle paths. Local checks: pnpm exec vitest run packages/utils/test/bundle-import.test.ts packages/utils/test/index.test.ts; pnpm --filter @eggjs/utils run typecheck; pnpm exec oxfmt --check packages/utils/src/import.ts packages/utils/test/bundle-import.test.ts; pnpm exec oxlint --type-aware packages/utils/src/import.ts packages/utils/test/bundle-import.test.ts. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/utils/src/import.ts (1)
421-446: Optional: dedupe the__esModuledouble-default +importDefaultOnlyinterop logic.The bundle branch (lines 426-429), the snapshot branch (lines 436-445), and the ESM branch (lines 461-493) all implement the same unwrap pattern with subtly different formulations (e.g. the snapshot/ESM branches include an extra
typeof obj === 'object'guard around the outer check, while the bundle branch uses optional chaining only). The three are equivalent today, but as the interop rules evolve they will drift. Consider extracting a single helper, e.g.:♻️ Proposed refactor
function unwrapInterop(obj: any, importDefaultOnly?: boolean): any { if ( obj && typeof obj === 'object' && obj.default?.__esModule === true && obj.default && 'default' in obj.default ) { obj = obj.default; } if (importDefaultOnly && obj && typeof obj === 'object' && 'default' in obj) { obj = obj.default; } return obj; }…and call it from all three sites. Non-blocking, can be deferred to a follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/import.ts` around lines 421 - 446, The importModule function duplicates the same interop/unwrap logic across the bundle, snapshot, and ESM branches; extract a single helper (e.g., unwrapInterop(obj, importDefaultOnly)) that implements the two-step unwrapping (first check for obj.default.__esModule === true with nested default, then apply importDefaultOnly to pick obj.default if present), and call this helper from the _bundleModuleLoader hit handling, the _snapshotModuleLoader handling, and the ESM branch to centralize behavior and remove the repeated conditional patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/utils/src/import.ts`:
- Around line 421-446: The importModule function duplicates the same
interop/unwrap logic across the bundle, snapshot, and ESM branches; extract a
single helper (e.g., unwrapInterop(obj, importDefaultOnly)) that implements the
two-step unwrapping (first check for obj.default.__esModule === true with nested
default, then apply importDefaultOnly to pick obj.default if present), and call
this helper from the _bundleModuleLoader hit handling, the _snapshotModuleLoader
handling, and the ESM branch to centralize behavior and remove the repeated
conditional patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb8dcf44-c470-47a6-9107-590ae80b74b8
📒 Files selected for processing (2)
packages/utils/src/import.tspackages/utils/test/bundle-import.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/utils/test/bundle-import.test.ts
There was a problem hiding this comment.
Pull request overview
Adds a bundle-aware runtime hook to @eggjs/utils so bundled Egg applications can intercept importModule() and return modules from an in-bundle map (shared across module instances via globalThis), while preserving existing default-export unwrapping behavior.
Changes:
- Add
setBundleModuleLoader()andBundleModuleLoaderto register a global bundle module loader hook. - Update
importModule()to consult the bundle loader (with Windows-path normalization) beforeimportResolve(). - Add Vitest coverage for interception, fallback behavior, default-only behavior, double-default unwrapping, and virtual specifiers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces the global bundle module loader hook and integrates it into importModule() before disk-based resolution. |
| packages/utils/test/bundle-import.test.ts | Adds test coverage for the new bundle loader behavior and path normalization. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates public export snapshot to include setBundleModuleLoader. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/egg/test/cluster1/app_worker.test.ts (1)
11-17:⚠️ Potential issue | 🟡 MinorBody comparison is sensitive to source-file line endings.
DEFAULT_BAD_REQUEST_HTMLis a template literal whose embedded newlines are whatever bytes the source file uses on disk. If a contributor checks the repo out on Windows withoutcore.autocrlf=input/ a.gitattributestext eol=lfrule for.ts, this literal becomes CRLF while the server-emitted body remains LF, andresponse.includes(DEFAULT_BAD_REQUEST_HTML)(Line 37) silently fails. Consider normalizing on comparison, e.g. matching against a regex or stripping\rfromresponsebefore theincludescheck, to make the test robust to checkout settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/egg/test/cluster1/app_worker.test.ts` around lines 11 - 17, The test's body comparison uses the template literal DEFAULT_BAD_REQUEST_HTML which may contain CRLF depending on checkout, making response.includes(DEFAULT_BAD_REQUEST_HTML) brittle; update the test to normalize line endings before comparison (e.g., strip '\r' from the server response or compare using a regex that ignores CRLF) so the includes check on DEFAULT_BAD_REQUEST_HTML succeeds regardless of source-file EOLs; locate DEFAULT_BAD_REQUEST_HTML and the response.includes usage in app_worker.test.ts to apply the normalization.
🧹 Nitpick comments (1)
packages/egg/test/cluster1/app_worker.test.ts (1)
136-149: Add a socket timeout to avoid indefinite hangs.
requestRawPathonly resolves onendand only rejects onerror. If the server keeps the socket half-open (e.g. due to a regression in 400 handling or a stuck worker), the promise never settles and the suite relies entirely on Vitest's outer test timeout, producing a less actionable failure than a targeted timeout here. Since this helper is specifically used to probe broken-HTTP behavior, defending against the hang case is worthwhile.♻️ Suggested hardening
function requestRawPath(port: number, path: string) { return new Promise<string>((resolve, reject) => { let response = ''; const socket = net.createConnection(port, '127.0.0.1', () => { socket.write(`GET ${path} HTTP/1.1\r\nHost: 127.0.0.1:${port}\r\nConnection: close\r\n\r\n`); }); + socket.setTimeout(5000, () => { + socket.destroy(new Error(`requestRawPath timeout after 5s, partial response: ${response}`)); + }); socket.setEncoding('utf8'); socket.on('data', (chunk) => { response += chunk; }); socket.on('error', reject); socket.on('end', () => resolve(response)); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/egg/test/cluster1/app_worker.test.ts` around lines 136 - 149, The requestRawPath helper can hang if the socket never closes; add a socket timeout to fail fast: set a reasonable timeout (e.g. 5s) on the created socket (socket.setTimeout) and listen for the 'timeout' event to destroy the socket and reject the promise with a clear error; ensure the 'data'/'end'/'error' handlers still clear any timeout and that you remove the timeout listener once the promise settles so no listeners leak. Target the requestRawPath function and its socket variable to implement this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/egg/test/cluster1/app_worker.test.ts`:
- Around line 11-17: The test's body comparison uses the template literal
DEFAULT_BAD_REQUEST_HTML which may contain CRLF depending on checkout, making
response.includes(DEFAULT_BAD_REQUEST_HTML) brittle; update the test to
normalize line endings before comparison (e.g., strip '\r' from the server
response or compare using a regex that ignores CRLF) so the includes check on
DEFAULT_BAD_REQUEST_HTML succeeds regardless of source-file EOLs; locate
DEFAULT_BAD_REQUEST_HTML and the response.includes usage in app_worker.test.ts
to apply the normalization.
---
Nitpick comments:
In `@packages/egg/test/cluster1/app_worker.test.ts`:
- Around line 136-149: The requestRawPath helper can hang if the socket never
closes; add a socket timeout to fail fast: set a reasonable timeout (e.g. 5s) on
the created socket (socket.setTimeout) and listen for the 'timeout' event to
destroy the socket and reject the promise with a clear error; ensure the
'data'/'end'/'error' handlers still clear any timeout and that you remove the
timeout listener once the promise settles so no listeners leak. Target the
requestRawPath function and its socket variable to implement this.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c493e0b3-33f4-41af-81fe-0e5924eb0419
📒 Files selected for processing (1)
packages/egg/test/cluster1/app_worker.test.ts
Adds static `ManifestStore.fromBundle(data, baseDir)` that creates a store from pre-validated bundled manifest data, skipping lockfile/config fingerprint checks. Part of #5863 split. Tracking: #5871. Stacked on: #5867 (A1) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/utils/test/bundle-import.test.ts (2)
9-12: Optional: also clear the loader inbeforeEachfor defensive symmetry.The first test (lines 14–18) assumes
globalThis.__EGG_BUNDLE_MODULE_LOADER__is unset on entry. Vitest isolates files by default so this is fine in practice, but adding abeforeEach(() => setBundleModuleLoader(undefined))would make the suite robust against any leakage from prior runs in the same worker context.♻️ Suggested addition
- afterEach(() => { - setBundleModuleLoader(undefined); - }); + beforeEach(() => { + setBundleModuleLoader(undefined); + }); + afterEach(() => { + setBundleModuleLoader(undefined); + });(Remember to add
beforeEachto thevitestimport.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/test/bundle-import.test.ts` around lines 9 - 12, Add a defensive beforeEach that calls setBundleModuleLoader(undefined) to mirror the existing afterEach cleanup so tests cannot rely on prior worker state; update the vitest import to include beforeEach (alongside afterEach) and ensure setBundleModuleLoader is referenced in the new beforeEach, which will also reset globalThis.__EGG_BUNDLE_MODULE_LOADER__ before each test.
81-89: Windows-path test is functional but slightly fragile across OSes.
getFilepath('esm').split(path.posix.sep).join(path.win32.sep)only converts forward slashes to backslashes. On a Windows runnergetFilepathmay already return backslashes, so the split becomes a no-op andfilepathends up unchanged — meaning the test then exercises the “already-normalized” path rather than the Windows→POSIX normalization innormalizeBundleModulePath. It still passes (loader matches either way), but it doesn't actually prove cross-OS normalization in that environment.If you want the assertion to be meaningful regardless of host OS, build the input from a known POSIX-style path:
♻️ Suggested change
- const fakeModule = { windows: true }; - const filepath = getFilepath('esm').split(path.posix.sep).join(path.win32.sep); - - setBundleModuleLoader((p) => (p.endsWith('/fixtures/esm') ? fakeModule : undefined)); - - const result = await importModule(filepath); - assert.deepEqual(result, fakeModule); + const fakeModule = { windows: true }; + // Force a Windows-style input regardless of host OS. + const filepath = getFilepath('esm').split(/[\\/]/).join(path.win32.sep); + + setBundleModuleLoader((p) => (p.endsWith('/fixtures/esm') ? fakeModule : undefined)); + + const result = await importModule(filepath); + assert.deepEqual(result, fakeModule);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/test/bundle-import.test.ts` around lines 81 - 89, The test currently builds a "Windows-style" filepath by swapping posix separators which is a no-op on Windows hosts, so instead force a POSIX input first and then convert it to Windows form so normalizeBundleModulePath is actually exercised; specifically, derive a guaranteed POSIX path from getFilepath('esm') (e.g. replace any win32 separators with posix ones) and then create filepath by replacing posix separators with path.win32.sep, keeping the rest of the test (setBundleModuleLoader, importModule, fakeModule) unchanged so the loader lookup uses the normalized path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/utils/test/bundle-import.test.ts`:
- Around line 73-79: Rename the test title to accurately reflect what's being
verified: that the bundle loader can serve specifiers that don't exist on disk
rather than claiming it "short-circuits importResolve". Update the description
in the test containing setBundleModuleLoader, importModule and fakeModule to
something like "loader serves specifiers that need not exist on disk" (or
similar) so it references setBundleModuleLoader and importModule instead of
importResolve.
---
Nitpick comments:
In `@packages/utils/test/bundle-import.test.ts`:
- Around line 9-12: Add a defensive beforeEach that calls
setBundleModuleLoader(undefined) to mirror the existing afterEach cleanup so
tests cannot rely on prior worker state; update the vitest import to include
beforeEach (alongside afterEach) and ensure setBundleModuleLoader is referenced
in the new beforeEach, which will also reset
globalThis.__EGG_BUNDLE_MODULE_LOADER__ before each test.
- Around line 81-89: The test currently builds a "Windows-style" filepath by
swapping posix separators which is a no-op on Windows hosts, so instead force a
POSIX input first and then convert it to Windows form so
normalizeBundleModulePath is actually exercised; specifically, derive a
guaranteed POSIX path from getFilepath('esm') (e.g. replace any win32 separators
with posix ones) and then create filepath by replacing posix separators with
path.win32.sep, keeping the rest of the test (setBundleModuleLoader,
importModule, fakeModule) unchanged so the loader lookup uses the normalized
path.
🪄 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: 042e76fe-6676-4036-a3a2-5f4a1da5adc5
📒 Files selected for processing (1)
packages/utils/test/bundle-import.test.ts
There was a problem hiding this comment.
Pull request overview
Adds a runtime hook in @eggjs/utils to let bundled Egg applications intercept importModule() calls via a shared globalThis loader, enabling in-bundle module maps to bypass filesystem resolution.
Changes:
- Introduces
BundleModuleLoaderandsetBundleModuleLoader()and wires the hook intoimportModule()beforeimportResolve(). - Adds comprehensive unit tests for bundle interception, default-unwrapping behavior, fallback behavior, and Windows path normalization.
- Updates an Egg cluster test to use raw HTTP requests to exercise invalid-path parsing behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/utils/src/import.ts | Adds globalThis-backed bundle module loader hook and integrates it into importModule() resolution flow. |
| packages/utils/test/bundle-import.test.ts | New tests covering bundle loader interception, fallthrough, importDefaultOnly, __esModule unwrapping, and path normalization. |
| packages/utils/test/snapshots/index.test.ts.snap | Snapshot update to reflect new exported API. |
| packages/egg/test/cluster1/app_worker.test.ts | Switches broken-HTTP-packet test to send raw requests to preserve unencoded paths. |
|
Addressed the latest review feedback:
Local checks run:
Note: root |
There was a problem hiding this comment.
Pull request overview
Adds a runtime hook to @eggjs/utils to let bundled Egg apps intercept importModule() and serve modules from an in-bundle map (via a globalThis-stored loader), which is a prerequisite for the upcoming bundler work.
Changes:
- Add
setBundleModuleLoader()and aBundleModuleLoadertype, storing the loader onglobalThis.__EGG_BUNDLE_MODULE_LOADER__and consulting it insideimportModule()beforeimportResolve(). - Add Vitest coverage for bundle interception, fallthrough behavior, default-unwrapping, virtual specifiers, and Windows path normalization.
- Stabilize an Egg cluster test by normalizing CRLF differences and improving raw socket request timeout handling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces the bundle module loader hook and integrates it into importModule() with existing default-unwrapping behavior. |
| packages/utils/test/bundle-import.test.ts | Adds dedicated tests covering bundle loader behavior and edge cases. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
| packages/egg/test/cluster1/app_worker.test.ts | Normalizes newline differences in assertions and refines raw socket request settling/timeout behavior to reduce flakiness. |
There was a problem hiding this comment.
Pull request overview
Adds a runtime hook to @eggjs/utils so bundled Egg applications can intercept importModule() resolution via a shared globalThis loader, enabling in-bundle module maps and avoiding filesystem lookups during boot.
Changes:
- Add
setBundleModuleLoader()and consultglobalThis.__EGG_BUNDLE_MODULE_LOADER__insideimportModule()(with default-unwrapping +importDefaultOnlyhandling). - Add Vitest coverage for bundle-loader interception, fallback behavior, and Windows path normalization; update export snapshot.
- Adjust an Egg cluster test helper to make raw HTTP request assertions and socket settling behavior more robust across platforms.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/import.ts | Adds the bundle module loader hook and early interception path in importModule(). |
| packages/utils/test/bundle-import.test.ts | New tests validating loader interception, fallback, default-unwrapping, and path normalization. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
| packages/utils/tsconfig.json | Includes cross-package global typing file for __EGG_BUNDLE_MODULE_LOADER__. |
| packages/core/src/global.d.ts | Introduces global type for __EGG_BUNDLE_MODULE_LOADER__. |
| packages/egg/test/cluster1/app_worker.test.ts | Normalizes CRLF in assertions and refactors raw socket request settling. |
There was a problem hiding this comment.
Pull request overview
Adds a runtime hook to @eggjs/utils so bundled Egg applications can intercept importModule() and serve modules from an in-bundle map via a shared globalThis loader.
Changes:
- Add
setBundleModuleLoader()API andglobalThis.__EGG_BUNDLE_MODULE_LOADER__fast-path inimportModule(). - Add Vitest coverage for bundle-loader interception, fallthrough behavior, default-unwrapping, and Windows path normalization.
- Update utils export snapshot and add
@eggjs/typingsdependency to provide the global typing.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces setBundleModuleLoader() and checks the global loader before resolving/importing modules. |
| packages/utils/test/bundle-import.test.ts | Adds new tests validating bundle-loader behavior and edge cases. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
| packages/utils/package.json | Adds @eggjs/typings workspace dependency (required for @eggjs/typings/global import). |
| packages/egg/test/cluster1/app_worker.test.ts | Refactors rawRequest() settling/timeout handling to be single-settle and include partial response on timeout. |
killagu
force-pushed
the
split/01-utils-bundle-module-loader
branch
from
dea4f1b to
d2912aa
Compare
killagu
force-pushed
the
split/01-utils-bundle-module-loader
branch
from
d2912aa to
e028bea
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new runtime hook in @eggjs/utils to let bundled Egg runtimes intercept importModule() via a shared globalThis loader, enabling bundled module maps to short-circuit filesystem/module resolution.
Changes:
- Add
setBundleModuleLoader()andglobalThis.__EGG_BUNDLE_MODULE_LOADER__lookup path (with Windows→POSIX separator normalization) toimportModule(). - Add Vitest coverage for bundle-loader interception, fallthrough, default-unwrapping, and virtual specifiers; update export snapshot.
- Add
@eggjs/typingsdependency and adjust an Egg cluster test helper (rawRequest) for more robust timeout/error settling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces the global bundle module loader hook and wires it into importModule() before normal resolution. |
| packages/utils/package.json | Adds @eggjs/typings as a dependency to source/export the BundleModuleLoader type. |
| packages/utils/test/bundle-import.test.ts | New tests validating bundle loader behavior, fallthrough, and normalization. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
| packages/egg/test/cluster1/app_worker.test.ts | Improves test stability: longer beforeAll timeout and safer raw socket request settling/timeout handling. |
killagu
force-pushed
the
split/01-utils-bundle-module-loader
branch
from
e028bea to
87414b5
Compare
killagu
force-pushed
the
split/01-utils-bundle-module-loader
branch
from
87414b5 to
4800381
Compare
There was a problem hiding this comment.
Pull request overview
Adds a runtime hook to @eggjs/utils so bundled Egg apps can intercept importModule() resolution via a shared globalThis loader, enabling in-bundle module maps and reducing filesystem-based lookups.
Changes:
- Add
setBundleModuleLoader(loader)and invoke the registered loader before normalimportResolve()inimportModule(). - Add new Vitest coverage for bundle-loader interception, fall-through behavior, default-unwrapping semantics, and path normalization.
- Add a small test hardening change in
packages/eggaround raw socket requests/timeouts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/import.ts | Introduces setBundleModuleLoader (globalThis-backed) and integrates it into importModule() with existing default-unwrapping behavior. |
| packages/utils/package.json | Adds @eggjs/typings dependency for the exported BundleModuleLoader type. |
| packages/utils/test/bundle-import.test.ts | New tests validating interception, fall-through, importDefaultOnly, virtual specifiers, and Windows-path normalization. |
| packages/utils/test/import.test.ts | Extends existing importModule() tests to cover __esModule double-default + importDefaultOnly behavior. |
| packages/utils/test/fixtures/esm/es-module-default.js | New fixture exercising __esModule double-default export shape. |
| packages/utils/test/snapshots/index.test.ts.snap | Updates export snapshot to include setBundleModuleLoader. |
| packages/egg/test/cluster1/app_worker.test.ts | Adjusts test timeout and makes raw socket request helper settle deterministically with clearer timeout rejection. |
Adds static `ManifestStore.fromBundle(data, baseDir)` that creates a store from pre-validated bundled manifest data, skipping lockfile/config fingerprint checks. Part of #5863 split. Tracking: #5871. Stacked on: #5867 (A1) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
Adds
setBundleModuleLoader(loader)to@eggjs/utilsso a bundled Egg application can interceptimportModule()and short-circuit module lookups to an in-bundle map. Reuses the existing__esModuledouble-default andimportDefaultOnlyhandling.State is stored on
globalThis.__EGG_BUNDLE_MODULE_LOADER__so the bundled entry and an externaleggframework instance — which may be different module instances under turbopack — share the same hook.Why
This is batch 1, part of a 19-PR split of #5863 (a 4812-line monolith adding
@eggjs/egg-bundler). #5863 is kept open as a tracking reference. This PR is independent (base =next) and can land in any order with the other batch-1 PRs.The runtime hook is a prerequisite for the upcoming
@eggjs/egg-bundlerpackage: the generated worker entry calls it before `new Application()` to redirect every loader `importModule()` to the bundled namespace, eliminating filesystem scanning at runtime.Test plan
Stack context
Other batch-1 PRs (independent, can land in any order):
Subsequent batches (will be opened once batch-1 merges):
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests