Skip to content

feat(tegg-loader): support async module importer override (__EGG_MODULE_IMPORTER__)#5989

Merged
killagu merged 3 commits into
eggjs:nextfrom
elrrrrrrr:feat/tegg-loader-module-importer
Jun 23, 2026
Merged

feat(tegg-loader): support async module importer override (__EGG_MODULE_IMPORTER__)#5989
killagu merged 3 commits into
eggjs:nextfrom
elrrrrrrr:feat/tegg-loader-module-importer

Conversation

@elrrrrrrr

@elrrrrrrr elrrrrrrr commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What

Add an optional __EGG_MODULE_IMPORTER__ global hook to LoaderUtil.loadFile, mirroring the existing __EGG_BUNDLE_MODULE_LOADER__ bundle hook. When set, the loader delegates module loading to it instead of the built-in await import().

// tegg/core/loader/src/LoaderUtil.ts — loadFile()
if (exports == null && typeof globalThis.__EGG_MODULE_IMPORTER__ === 'function') {
  exports = await globalThis.__EGG_MODULE_IMPORTER__(originalFilePath);
}
// ...existing native fallback
if (exports == null) {
  exports = await import(filePath);
}

Why

This is an extension point for bundler-based test runners (e.g. Vitest).

When an egg app is tested with the tegg packages installed as published packages (so the loader is externalized and uses native import()), while the test file imports the same fixture source through the runner's module graph (Vite), the same file resolves to two different module instances. The proto registry keys by the class object (Reflect.defineMetadata(CLAZZ_PROTO, proto, clazz)), so the class the loader registered is not the class the test holds, and:

await ctx.getEggObject(HelloService) // → Error: can not get proto for clazz HelloService

eggjs's own tests don't hit this because the tegg packages resolve to workspace source (inlined by Vite), so the loader already imports through Vite and there is a single instance.

Downstream consumers (e.g. tegg distributed as published @scope/* packages) can route the loader's import through the runner by setting the hook in a Vitest setupFile (which is Vite-processed, so its import is the runner's):

// vitest setupFile
globalThis.__EGG_MODULE_IMPORTER__ = (filePath) => import(filePath);

This keeps one module instance → the loader-registered class === the test's class → getEggObject(ClassRef) works. (A follow-up can wire this provision into @eggjs/tegg-vitest / @eggjs/mock's setup_vitest so it is out-of-box.)

Notes

  • No behavior change when the hook is unset (default path unchanged).
  • Errors from the importer are wrapped the same way as the bundle loader and the native import.
  • Type added as ModuleImporter in @eggjs/typings and declared on globalThis, mirroring BundleModuleLoader.

Test

Added 3 cases to tegg/core/loader/test/Loader.test.ts mirroring the bundle-loader tests (load through importer / fall through on null / wrap errors). vitest run tegg/core/loader/test/Loader.test.ts → 10 passed.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added support for a custom async module importer via a new global hook, allowing an injected importer to override module resolution before the default loader fallback.
  • Tests
    • Expanded loader tests to cover the new async importer behavior, including correct importer invocation, fallback when it returns null, and wrapped error reporting when it throws.
  • Documentation
    • Updated the egg-bin bundle command parameter table formatting (no functional changes).

Add an optional `__EGG_MODULE_IMPORTER__` global hook to `LoaderUtil.loadFile`,
mirroring the existing `__EGG_BUNDLE_MODULE_LOADER__` bundle hook. When set, the
loader delegates module loading to it instead of the built-in `await import()`.

This is an extension point for bundler-based test runners (e.g. Vitest). When an
app is tested as published packages (the loader externalized to native `import()`)
while the test file imports the same fixture source through the runner's module
graph, the two resolve to different module instances — so a class registered as an
egg proto by the loader is not the same class the test references, and
`ctx.getEggObject(ClassRef)` throws "can not get proto for clazz". Routing the
loader's import through the runner (e.g. `filePath => import(filePath)` evaluated
in the runner context) keeps a single module instance.

eggjs's own tests don't hit this because tegg packages resolve to workspace source
(inlined by Vite), so the loader already imports through Vite.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 23, 2026 13:06
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b512b493-ac49-45d2-be6d-b64b7d2d1a25

📥 Commits

Reviewing files that changed from the base of the PR and between 3cd661b and dc5c25e.

📒 Files selected for processing (1)
  • site/docs/zh-CN/core/bundle.md
✅ Files skipped from review due to trivial changes (1)
  • site/docs/zh-CN/core/bundle.md

📝 Walkthrough

Walkthrough

Adds a new ModuleImporter type (async function returning Promise<unknown>) to the typings package and declares __EGG_MODULE_IMPORTER__ as an optional global. LoaderUtil.loadFile is extended to invoke this importer as a fallback when the bundle loader returns nothing, before falling back to dynamic import(). Three new tests cover the importer success, null-return, and error cases.

Changes

ModuleImporter global fallback

Layer / File(s) Summary
ModuleImporter type declaration and global binding
packages/typings/src/index.ts, packages/typings/src/global.ts
Exports the ModuleImporter async function type and declares __EGG_MODULE_IMPORTER__: ModuleImporter | undefined in the global scope alongside __EGG_BUNDLE_MODULE_LOADER__.
LoaderUtil async importer fallback and tests
tegg/core/loader/src/LoaderUtil.ts, tegg/core/loader/test/Loader.test.ts
Adds a conditional branch in loadFile that awaits globalThis.__EGG_MODULE_IMPORTER__ when the bundle loader misses; errors are wrapped via createLoadError. Tests cover module-map return, null-return fallback to dynamic import, and error wrapping with cause chaining.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • eggjs/egg#5932: Also modifies LoaderUtil.loadFile's control flow and Loader.test.ts to route module loading through a global loader before dynamic import().
  • eggjs/egg#5924: Changes the module-loading fallback path when the bundle module loader returns nothing, working within the same loader chain this PR extends.

Poem

🐇 A new importer hops into the chain,
__EGG_MODULE_IMPORTER__ whispers its name.
When the bundle loader comes up empty and bare,
this async little fallback leaps through the air! 🥚
Errors wrapped neatly, cause pinned with care—
the loader now tests all three paths with flair! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main feature addition: support for an async module importer override called __EGG_MODULE_IMPORTER__ in the tegg loader, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an async module import override hook (globalThis.__EGG_MODULE_IMPORTER__) to LoaderUtil.loadFile so bundler-based runners (e.g. Vitest) can force loader imports through the runner’s module graph and avoid duplicate module instances.

Changes:

  • Add __EGG_MODULE_IMPORTER__ hook handling (with error wrapping) to LoaderUtil.loadFile.
  • Add ModuleImporter type + global declaration in @eggjs/typings.
  • Add loader tests covering importer usage, fallback behavior, and error wrapping.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
tegg/core/loader/src/LoaderUtil.ts Adds the async importer override hook before native dynamic import.
tegg/core/loader/test/Loader.test.ts Adds tests for importer override, null fallback, and error wrapping.
packages/typings/src/index.ts Introduces ModuleImporter type and docs for the new hook.
packages/typings/src/global.ts Declares globalThis.__EGG_MODULE_IMPORTER__ for consumers.

Comment thread packages/typings/src/index.ts
Comment thread tegg/core/loader/src/LoaderUtil.ts
Comment thread tegg/core/loader/test/Loader.test.ts
Comment thread tegg/core/loader/test/Loader.test.ts
Comment thread tegg/core/loader/test/Loader.test.ts
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.94%. Comparing base (b2040cf) to head (dc5c25e).
⚠️ Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #5989   +/-   ##
=======================================
  Coverage   85.94%   85.94%           
=======================================
  Files         669      669           
  Lines       19925    19929    +4     
  Branches     3961     3962    +1     
=======================================
+ Hits        17124    17128    +4     
  Misses       2423     2423           
  Partials      378      378           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an asynchronous module importer override (__EGG_MODULE_IMPORTER__) to delegate module loading to custom runners (like Vitest), preventing duplicate module instances during testing. It updates global typings, implements the loader fallback in LoaderUtil.ts, and adds corresponding unit tests. The feedback suggests normalizing the file path passed to the importer to use forward slashes, ensuring cross-platform consistency on Windows.

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.

Comment thread tegg/core/loader/src/LoaderUtil.ts Outdated
// same instance the test file imports. See `ModuleImporter` in @eggjs/typings.
if (exports == null && typeof globalThis.__EGG_MODULE_IMPORTER__ === 'function') {
try {
exports = await globalThis.__EGG_MODULE_IMPORTER__(originalFilePath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure cross-platform consistency and prevent backslash escaping or path matching issues on Windows, the file path passed to __EGG_MODULE_IMPORTER__ should be normalized to use forward slashes (/). This matches the normalization pattern already used for __EGG_BUNDLE_MODULE_LOADER__ on line 93, maintaining symmetry between path resolution and module loading.

Suggested change
exports = await globalThis.__EGG_MODULE_IMPORTER__(originalFilePath);
exports = await globalThis.__EGG_MODULE_IMPORTER__(originalFilePath.split('\\').join('/'));
References
  1. Maintain symmetry between path resolution and module loading for bundle-only modules by using the same raw, unresolved path as the canonical key.

Address review: pass the importer a forward-slash path, mirroring
__EGG_BUNDLE_MODULE_LOADER__, for cross-platform consistency on Windows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@elrrrrrrr

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

  • Addressed @gemini-code-assist's suggestion: the importer now receives a POSIX-normalized path (originalFilePath.split('\\').join('/')), mirroring __EGG_BUNDLE_MODULE_LOADER__.
  • Heads-up on the red typecheck check: tsgo --noEmit passes for all packages — the failure is from fmtcheck (oxfmt --check .) flagging site/docs/zh-CN/core/bundle.md, which is unrelated to this PR (introduced by docs(site): add Bundle Deployment guide #5988 and present on next). This PR touches only 4 TS files, all of which pass typecheck + format. The codecov/project red is a coverage-delta artifact from a partial report upload, not a regression (patch coverage is green).

Drive-by: `oxfmt --check .` (the repo-wide fmtcheck) was failing on
site/docs/zh-CN/core/bundle.md (table alignment) on `next`, turning the
check red for every PR. Reformat so fmtcheck passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 23, 2026 13:25
@elrrrrrrr

Copy link
Copy Markdown
Contributor Author

Also pushed a drive-by commit reformatting site/docs/zh-CN/core/bundle.md with oxfmt — that was the only file failing the repo-wide fmtcheck on next (table alignment), so this should turn the check green. Happy to drop it into a separate PR if you'd prefer to keep this one focused.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment on lines +100 to +104
if (exports == null && typeof globalThis.__EGG_MODULE_IMPORTER__ === 'function') {
try {
// Pass a POSIX-normalized path, mirroring __EGG_BUNDLE_MODULE_LOADER__,
// so importers behave consistently across platforms (e.g. on Windows).
exports = await globalThis.__EGG_MODULE_IMPORTER__(originalFilePath.split('\\').join('/'));
Comment on lines +11 to +22
* When set, the loader delegates module loading to this importer instead of the
* built-in `await import(filePath)`. Its main use is testing with a bundler-based
* test runner (e.g. Vitest): when an app's egg modules are loaded by the loader
* via the native `import()` while the test file imports the same source through
* the runner's module graph, the two resolve to *different* module instances —
* so a class decorated as an egg proto by the loader is not the same class the
* test references, and `ctx.getEggObject(ClassRef)` fails with "can not get proto".
*
* A test runner can inject an importer that routes loading through its own module
* graph (e.g. `filePath => import(filePath)` evaluated inside the runner context),
* keeping a single module instance. Return value mirrors `await import()`.
*/
@elrrrrrrr elrrrrrrr requested review from fengmk2 and killagu June 23, 2026 13:30

@killagu killagu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elrrrrrrr elrrrrrrr requested review from fengmk2 and removed request for fengmk2 June 23, 2026 14:01
@elrrrrrrr elrrrrrrr enabled auto-merge June 23, 2026 14:01
@killagu killagu disabled auto-merge June 23, 2026 14:02
@killagu killagu merged commit 4a3a638 into eggjs:next Jun 23, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants