feat(utils): consume __EGG_MODULE_IMPORTER__ in importModule#5990
feat(utils): consume __EGG_MODULE_IMPORTER__ in importModule#5990elrrrrrrr wants to merge 1 commit into
__EGG_MODULE_IMPORTER__ in importModule#5990Conversation
Follow-up to eggjs#5989 (which added the `ModuleImporter` type/global and the @eggjs/tegg-loader consumer). Extend the same async module-importer override to egg-core's module/boot-file loading via `importModule`: when a test runner sets `globalThis.__EGG_MODULE_IMPORTER__`, app boot files (app.ts, app/extend/*.ts) and config load through the runner's module graph too, so enums/decorators in boot files transpile and the whole app shares one module realm with the test (matching how the tegg loader already routes tegg modules). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesModule Importer Hook
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![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 an async module importer override mechanism via globalThis.__EGG_MODULE_IMPORTER__ in packages/utils/src/import.ts to allow custom loaders (like Vitest) to intercept and resolve modules. It includes handling for unwrapping double-default shapes and respecting the importDefaultOnly option. Comprehensive unit tests have been added in packages/utils/test/module-importer.test.ts to verify these behaviors. No review comments were provided, and there is no additional feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5990 +/- ##
=======================================
Coverage 85.94% 85.95%
=======================================
Files 669 669
Lines 19929 19937 +8
Branches 3962 3965 +3
=======================================
+ Hits 17128 17136 +8
Misses 2423 2423
Partials 378 378 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Extends @eggjs/utils’s importModule() to optionally delegate module loading to the global __EGG_MODULE_IMPORTER__ hook (introduced in #5989) so Egg’s app boot/config/module loading can be routed through a bundler/test-runner module graph (e.g. Vitest), avoiding cross-realm module duplication.
Changes:
- Add
__EGG_MODULE_IMPORTER__interception path toimportModule()with the same default/__esModuleunwrapping behavior used by existing loaders. - Add a new Vitest test suite validating importer interception,
importDefaultOnly, and__esModule“double-default” unwrapping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/utils/src/import.ts | Adds __EGG_MODULE_IMPORTER__ hook support to importModule() before the native dynamic import/require fallback. |
| packages/utils/test/module-importer.test.ts | Introduces tests covering importer override behaviors for importModule(). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/utils/test/module-importer.test.ts (1)
50-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for
nullimporter return fallback.The suite validates precedence, but it does not lock the
null/undefinedfallback behavior that sibling loader flows already rely on.Suggested test addition
it('takes precedence over the native dynamic import', async () => { globalThis.__EGG_MODULE_IMPORTER__ = async () => ({ fromImporter: true }); const result = await importModule(getFilepath('esm')); assert.deepEqual(result, { fromImporter: true }); }); + + it('falls back to native import when importer returns null', async () => { + globalThis.__EGG_MODULE_IMPORTER__ = async () => null; + + const result = await importModule(getFilepath('esm')); + assert.ok(result); + assert.equal(typeof result, 'object'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/utils/test/module-importer.test.ts` around lines 50 - 56, Add a regression test case after the existing precedence test in the test suite to verify that when the globalThis.__EGG_MODULE_IMPORTER__ returns null or undefined, the importModule function falls back to the native dynamic import behavior. The new test should set the custom importer to return null/undefined, call importModule with the same esm module filepath, and assert that the result matches what the native import would return rather than a null/undefined value from the importer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/utils/src/import.ts`:
- Around line 504-517: The conditional check for `_moduleImporter` should verify
it is a function using `typeof _moduleImporter === 'function'` to prevent
crashes from non-function truthy values. The path passed to the
`_moduleImporter` function call should be normalized to POSIX format for
cross-platform compatibility instead of using the raw `moduleFilePath`.
Additionally, the logic should only short-circuit and return the importer result
when it returns a non-null value, allowing fallback to native import when the
custom importer returns null or undefined.
---
Nitpick comments:
In `@packages/utils/test/module-importer.test.ts`:
- Around line 50-56: Add a regression test case after the existing precedence
test in the test suite to verify that when the
globalThis.__EGG_MODULE_IMPORTER__ returns null or undefined, the importModule
function falls back to the native dynamic import behavior. The new test should
set the custom importer to return null/undefined, call importModule with the
same esm module filepath, and assert that the result matches what the native
import would return rather than a null/undefined value from the importer.
🪄 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: 877aa3c8-b17d-40bd-b6ee-8d93689d5261
📒 Files selected for processing (2)
packages/utils/src/import.tspackages/utils/test/module-importer.test.ts
|
Superseded by #5992 (same change, re-opened from a branch on eggjs/egg so CodeQL code scanning runs — this fork PR was permanently blocked waiting on CodeQL results). |
Pull request was closed
What
Follow-up to #5989. That PR added the
ModuleImportertype +__EGG_MODULE_IMPORTER__global declaration and made@eggjs/tegg-loaderconsume it. This PR extends the same hook to egg-core's module/boot-file loading in@eggjs/utilsimportModule():Why
importModule()is what egg-core uses to load plugin boot files (app.ts,app/extend/*.ts) and config. Under a bundler-based test runner (Vitest) the worker thread loads these via nativeimport(), which (a) can't transpile TSenum/decorators in boot files ("enum is not supported in strip-only mode"), and (b) puts them in a different module realm than the test file — so the tegg-loader hook alone (which covers tegg modules) isn't enough; the app's boot graph still loads natively.Routing
importModulethrough the same__EGG_MODULE_IMPORTER__(e.g.filePath => import(filePath)evaluated in the runner context) makes the whole app — boot files, config, and tegg modules — resolve through one module graph. This is what lets a downstream tegg distribution (published@scope/*packages, externalized by the runner) boot its app fixtures and runctx.getEggObject(ImportedClass)against a single class instance.Notes
__EGG_BUNDLE_MODULE_LOADER__/_snapshotModuleLoaderoverrides and the@eggjs/tegg-loaderconsumer from feat(tegg-loader): support async module importer override (__EGG_MODULE_IMPORTER__) #5989, including the default/__esModuleunwrapping.ModuleImportertype + global are already onnext(from feat(tegg-loader): support async module importer override (__EGG_MODULE_IMPORTER__) #5989).Test
Added
packages/utils/test/module-importer.test.ts(load through importer / importDefaultOnly / __esModule unwrap / precedence over native import).vitest run packages/utils/test/module-importer.test.ts→ green; existing bundle/snapshot import tests unaffected.🤖 Generated with Claude Code
Summary by CodeRabbit