feat(utils): add setBundleModuleLoader runtime hook#5867
feat(utils): add setBundleModuleLoader runtime hook#5867killagu wants to merge 1 commit intoeggjs:nextfrom
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.
📝 WalkthroughWalkthroughAdded a bundle module loader mechanism to the import system. New Changes
Sequence DiagramsequenceDiagram
participant Client
participant importModule
participant BundleLoader
participant StandardResolver
Client->>importModule: importModule(filepath)
importModule->>importModule: normalize filepath (backslash→forward slash)
importModule->>BundleLoader: check if loader registered?
alt Loader Registered
BundleLoader->>BundleLoader: loader(normalized_filepath)
BundleLoader-->>importModule: module object (or undefined)
alt Loader returned value
importModule->>importModule: apply interop rules (unwrap defaults)
importModule-->>Client: processed module
else Loader returned undefined
importModule->>StandardResolver: fall through to normal resolution
StandardResolver-->>Client: resolved module
end
else No Loader Registered
importModule->>StandardResolver: proceed to normal resolution
StandardResolver-->>Client: resolved module
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 86.00% 86.02% +0.01%
==========================================
Files 667 667
Lines 18945 18956 +11
Branches 3652 3657 +5
==========================================
+ Hits 16294 16306 +12
+ Misses 2297 2295 -2
- Partials 354 355 +1 ☔ 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.
| const _bundleModuleLoader: BundleModuleLoader | undefined = (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__; | ||
| if (_bundleModuleLoader) { | ||
| const hit = _bundleModuleLoader(filepath.replaceAll('\\', '/')); |
There was a problem hiding this comment.
There is a consistency issue when multiple instances of @eggjs/utils are present in the same process (e.g., under Turbopack). While the bundle loader is shared via globalThis, the isESM = false override in setBundleModuleLoader only affects the current module instance. Other instances will still have isESM = true, leading to inconsistent behavior in importModule and importResolve if the loader falls through. Consider synchronizing this state globally or checking the global loader presence dynamically when evaluating ESM vs CJS logic.
const _bundleModuleLoader: BundleModuleLoader | undefined = (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__;
if (_bundleModuleLoader) {
isESM = false;
const hit = _bundleModuleLoader(filepath.replaceAll('\\', '/'));| export function setBundleModuleLoader(loader: BundleModuleLoader | undefined): void { | ||
| (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader; | ||
| if (loader) isESM = false; | ||
| } |
There was a problem hiding this comment.
The isESM variable is set to false when a loader is provided, but it is not restored to its original state when setBundleModuleLoader(undefined) is called. This can lead to persistent side effects in the current process, which is particularly problematic for test isolation (as seen in the afterEach cleanup in the new tests). Consider capturing the initial detected value of isESM and restoring it when the loader is unset.
![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
| export function setBundleModuleLoader(loader: BundleModuleLoader | undefined): void { | ||
| (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader; | ||
| if (loader) isESM = false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there is no unregister branch restoring isESM today.
rg -n -A12 -B2 'export function setBundleModuleLoader' packages/utils/src/import.ts
rg -n 'isESM\s*=' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 1118
🏁 Script executed:
# Find all calls to setBundleModuleLoader to understand usage pattern
rg -n 'setBundleModuleLoader' --type ts --type jsRepository: eggjs/egg
Length of output: 876
🏁 Script executed:
# Check what _snapshotModuleLoader is and where it's defined
rg -n '_snapshotModuleLoader' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 247
🏁 Script executed:
# Look at more context around the isESM initialization and snapshot logic
sed -n '1,100p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 4089
🏁 Script executed:
# Examine the test file to understand the impact of isESM state
cat -n packages/utils/test/bundle-import.test.tsRepository: eggjs/egg
Length of output: 2702
🏁 Script executed:
# Check if isESM is used in importModule logic to understand the actual impact
sed -n '414,450p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1728
🏁 Script executed:
# Check if there are any test failures related to isESM state between tests
cd packages/utils && npm test 2>&1 | head -100Repository: eggjs/egg
Length of output: 273
🏁 Script executed:
# Look at how snapshot loader is set to understand the state management pattern
sed -n '375,405p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1465
🏁 Script executed:
# Check if there's any cleanup/reset of isESM in test setup/teardown
rg -n 'isESM|beforeEach|afterEach|before|after' packages/utils/test/bundle-import.test.tsRepository: eggjs/egg
Length of output: 126
🏁 Script executed:
# Check setSnapshotModuleLoader for similar isESM restoration issue
sed -n '388,402p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 705
🏁 Script executed:
# Verify all `any` casts in this file to understand full scope
rg -n 'as any' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 265
🏁 Script executed:
# Verify that _snapshotModuleLoader is in scope and accessible where proposed fix would apply
sed -n '375,430p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 2817
🏁 Script executed:
# Check if there are any type definitions for globalThis to use instead of 'as any'
rg -n 'globalThis|declare' packages/utils/src/import.ts | head -20Repository: eggjs/egg
Length of output: 309
Restore isESM when unregistering the bundle loader.
Line 411 only sets isESM = false on registration. When setBundleModuleLoader(undefined) is called (as in the afterEach in tests), isESM stays false, causing subsequent importModule() calls to incorrectly route through require() instead of import(), breaking imports of ESM-only modules.
Additionally, replace as any casts on globalThis.
Lines 410 and 415 use (globalThis as any) which violates the TypeScript guideline against any types. Define a proper type or augmentation for the globalThis properties instead.
🛠️ Proposed fix for state restoration
+const detectedIsESM = isESM;
+
export function setBundleModuleLoader(loader: BundleModuleLoader | undefined): void {
(globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader;
- if (loader) isESM = false;
+ if (loader) {
+ isESM = false;
+ } else if (!_snapshotModuleLoader) {
+ isESM = detectedIsESM;
+ }
}🤖 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 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.
| (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader; | ||
| if (loader) isESM = false; | ||
| } | ||
|
|
||
| export async function importModule(filepath: string, options?: ImportModuleOptions): Promise<any> { | ||
| const _bundleModuleLoader: BundleModuleLoader | undefined = (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current any-casts on bundle loader global access.
rg -n '\(globalThis as any\)\.__EGG_BUNDLE_MODULE_LOADER__' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Find all occurrences of __EGG_BUNDLE_MODULE_LOADER__ in the codebase
rg -n '__EGG_BUNDLE_MODULE_LOADER__' --type ts --type tsxRepository: eggjs/egg
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Show the context around the current usage in import.ts (lines 400-420)
sed -n '400,420p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1158
🏁 Script executed:
#!/bin/bash
# Check the beginning of import.ts to see current exports and type definitions
head -50 packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1968
🏁 Script executed:
#!/bin/bash
# Search all files for __EGG_BUNDLE_MODULE_LOADER__ without file type filter
rg -n '__EGG_BUNDLE_MODULE_LOADER__'Repository: eggjs/egg
Length of output: 291
🏁 Script executed:
#!/bin/bash
# Get the exact line numbers from the import.ts file (use wc to check file length first)
wc -l packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Get lines around 410-415 from import.ts
sed -n '405,420p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Get context around the BundleModuleLoader type definition (should be earlier in file)
sed -n '390,425p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1762
🏁 Script executed:
#!/bin/bash
# Search for any existing declare global blocks in the file
rg -n 'declare global' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 35
🏁 Script executed:
#!/bin/bash
# Get the section with BundleModuleLoader export (likely around line 400 or earlier)
rg -B 5 -A 2 'export type BundleModuleLoader' packages/utils/src/import.tsRepository: eggjs/egg
Length of output: 390
🏁 Script executed:
#!/bin/bash
# Double-check for any usages in plugins directory
rg -n '__EGG_BUNDLE_MODULE_LOADER__' plugins/ 2>/dev/null || echo "No matches in plugins/"Repository: eggjs/egg
Length of output: 75
🏁 Script executed:
#!/bin/bash
# Verify the exact structure - show where BundleModuleLoader export appears
sed -n '400,420p' packages/utils/src/import.ts | cat -nRepository: eggjs/egg
Length of output: 1158
Replace globalThis as any with a typed global declaration.
Lines 410 and 415 use as any casts, which weakens strict typing. The __EGG_BUNDLE_MODULE_LOADER__ global is only used within this file and can be safely typed via declare global.
♻️ Proposed refactor
export type BundleModuleLoader = (filepath: string) => unknown;
+declare global {
+ // shared runtime hook across duplicated package instances
+ var __EGG_BUNDLE_MODULE_LOADER__: BundleModuleLoader | undefined;
+}
+
/**
* Register a bundle module loader. Uses globalThis so that bundled and
* external copies of `@eggjs/utils` share the same loader.
*/
export function setBundleModuleLoader(loader: BundleModuleLoader | undefined): void {
- (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader;
+ globalThis.__EGG_BUNDLE_MODULE_LOADER__ = loader;
if (loader) isESM = false;
}
export async function importModule(filepath: string, options?: ImportModuleOptions): Promise<any> {
- const _bundleModuleLoader: BundleModuleLoader | undefined = (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__;
+ const _bundleModuleLoader = globalThis.__EGG_BUNDLE_MODULE_LOADER__;🤖 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 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).
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. |
| export function setBundleModuleLoader(loader: BundleModuleLoader | undefined): void { | ||
| (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__ = loader; | ||
| if (loader) isESM = false; | ||
| } | ||
|
|
||
| export async function importModule(filepath: string, options?: ImportModuleOptions): Promise<any> { | ||
| const _bundleModuleLoader: BundleModuleLoader | undefined = (globalThis as any).__EGG_BUNDLE_MODULE_LOADER__; | ||
| if (_bundleModuleLoader) { | ||
| const hit = _bundleModuleLoader(filepath.replaceAll('\\', '/')); | ||
| if (hit !== undefined) { | ||
| let obj = hit as any; | ||
| if (obj?.default?.__esModule === true && 'default' in obj.default) obj = obj.default; | ||
| if (options?.importDefaultOnly && obj && typeof obj === 'object' && 'default' in obj) obj = obj.default; | ||
| return obj; |
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