Skip to content

feat(utils): consume __EGG_MODULE_IMPORTER__ in importModule#5992

Merged
elrrrrrrr merged 2 commits into
nextfrom
feat/utils-module-importer
Jun 24, 2026
Merged

feat(utils): consume __EGG_MODULE_IMPORTER__ in importModule#5992
elrrrrrrr merged 2 commits into
nextfrom
feat/utils-module-importer

Conversation

@elrrrrrrr

@elrrrrrrr elrrrrrrr commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Re-opened from a same-repo branch so CodeQL code scanning runs (the fork-based #5990 was permanently BLOCKED waiting on CodeQL results that never arrive for fork PRs). Supersedes #5990.

What

Follow-up to #5989. That PR added the ModuleImporter type + __EGG_MODULE_IMPORTER__ global and made @eggjs/tegg-loader consume it. This extends the same hook to egg-core's module/boot-file loading in @eggjs/utils importModule():

const _moduleImporter = globalThis.__EGG_MODULE_IMPORTER__;
if (_moduleImporter) {
  let obj = await _moduleImporter(moduleFilePath);
  // …same default/__esModule unwrapping as the other loaders
  return obj;
}
// …existing native `await import(fileUrl)` fallback

Why

importModule() loads plugin boot files (app.ts, app/extend/*.ts) and config. Under a bundler-based test runner (Vitest) the worker thread loads these via native import(), which can't transpile TS enum/decorators in boot files and puts them in a different module realm than the test. Routing importModule through the same __EGG_MODULE_IMPORTER__ makes boot files, config, and tegg modules resolve through one module graph — which is what lets a downstream tegg distribution (externalized published packages) boot its app fixtures and run ctx.getEggObject(ImportedClass) against a single class instance.

Test

Adds packages/utils/test/module-importer.test.ts (load through importer / importDefaultOnly / __esModule unwrap / precedence over native import).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for a global async module importer hook, allowing module loading to be intercepted before the standard import flow.
    • Module results are now normalized consistently, including default-only handling and nested default export shapes.
  • Tests

    • Added coverage for standard loading, importer interception, default-only behavior, export normalization, and precedence over native import behavior.

Follow-up to #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>
@elrrrrrrr elrrrrrrr requested review from Copilot and killagu June 24, 2026 06:08
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

importModule() in packages/utils/src/import.ts gains an early async override: after resolving moduleFilePath, if globalThis.__EGG_MODULE_IMPORTER__ is registered, it is called with moduleFilePath and its result is normalized (default.__esModule unwrapping, importDefaultOnly) and returned without entering any ESM/CJS branch. A new Vitest suite covers all interception scenarios.

Changes

Global EGG_MODULE_IMPORTER hook in importModule

Layer / File(s) Summary
EGG_MODULE_IMPORTER hook implementation and tests
packages/utils/src/import.ts, packages/utils/test/module-importer.test.ts
Inserts an early-exit block in importModule() that invokes globalThis.__EGG_MODULE_IMPORTER__ with the resolved file path when registered, applies the same default.__esModule double-default unwrapping used elsewhere in the file, and respects importDefaultOnly. New tests verify passthrough (no importer), interception with path assertion, importDefaultOnly extraction, __esModule unwrapping, and importer precedence over native dynamic import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • eggjs/egg#5853: Modifies importModule() in the same file with overlapping importDefaultOnly and __esModule unwrapping logic in the dynamic import path.
  • eggjs/egg#5867: Also adds an early global hook short-circuit to importModule() (__EGG_BUNDLE_MODULE_LOADER__) with the same interop/unwrapping and importDefaultOnly handling pattern.
  • eggjs/egg#5989: Directly consumes the same globalThis.__EGG_MODULE_IMPORTER__ hook in tegg/core/loader/src/LoaderUtil.ts, making it a direct functional counterpart to this PR.

Suggested reviewers

  • killagu
  • jerryliang64

Poem

🐇 A rabbit dug a hook into the ground,
Where modules once were fetched without a sound.
Now __EGG_MODULE_IMPORTER__ leaps ahead,
Unwraps the defaults, returns what's right instead.
No CJS, no ESM fuss — just one tidy hop,
From resolve to result, with never a stop! 🥚

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: importModule now consumes __EGG_MODULE_IMPORTER__.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/utils-module-importer

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.

@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 async module importer override via globalThis.__EGG_MODULE_IMPORTER__ to intercept module loading, which is useful for test runners like Vitest. It also adds comprehensive unit tests to verify this behavior. The review feedback suggests simplifying the module unwrapping logic in import.ts to reduce redundant checks and improve readability.

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 packages/utils/src/import.ts

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

Extends @eggjs/utils’s importModule() to optionally delegate module loading to a global async importer hook (globalThis.__EGG_MODULE_IMPORTER__), enabling test runners/bundlers (e.g. Vitest) to load boot/config files through a single module graph/realm. Adds a dedicated utils test to validate the new interception and export-normalization behavior.

Changes:

  • Add __EGG_MODULE_IMPORTER__ override path to importModule() with export normalization (__esModule double-default + importDefaultOnly).
  • Add Vitest coverage validating importer interception, normalization, and precedence over native dynamic import.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/utils/src/import.ts Adds importer-hook interception to importModule() before native import()/require().
packages/utils/test/module-importer.test.ts Adds tests covering importer interception, normalization, and precedence.

Comment thread packages/utils/src/import.ts
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploying egg with  Cloudflare Pages  Cloudflare Pages

Latest commit: e06638c
Status: ✅  Deploy successful!
Preview URL: https://b8999730.egg-cci.pages.dev
Branch Preview URL: https://feat-utils-module-importer.egg-cci.pages.dev

View logs

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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 507-516: Normalize the module path before calling the global
importer hook in importModule so Windows callers don’t pass a different key than
tegg modules. Update the _moduleImporter(moduleFilePath) path in
packages/utils/src/import.ts to convert the path to a POSIX-style module
identifier first, keeping the existing default-unwrapping logic in place. Use
the importModule and _moduleImporter symbols to locate the change.
🪄 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: 1d3323fe-9228-4b09-95ab-382b56c25d31

📥 Commits

Reviewing files that changed from the base of the PR and between 0d2a357 and b9ba1a2.

📒 Files selected for processing (2)
  • packages/utils/src/import.ts
  • packages/utils/test/module-importer.test.ts

Comment thread packages/utils/src/import.ts

@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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploying egg-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: e06638c
Status: ✅  Deploy successful!
Preview URL: https://fa864473.egg-v3.pages.dev
Branch Preview URL: https://feat-utils-module-importer.egg-v3.pages.dev

View logs

@elrrrrrrr elrrrrrrr enabled auto-merge June 24, 2026 06:16
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.95%. Comparing base (12403e0) to head (e06638c).
⚠️ Report is 2 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #5992   +/-   ##
=======================================
  Coverage   85.94%   85.95%           
=======================================
  Files         669      669           
  Lines       19930    19938    +8     
  Branches     3962     3965    +3     
=======================================
+ Hits        17129    17137    +8     
  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.

@elrrrrrrr elrrrrrrr added this pull request to the merge queue Jun 24, 2026
Merged via the queue into next with commit 96376f8 Jun 24, 2026
26 checks passed
@elrrrrrrr elrrrrrrr deleted the feat/utils-module-importer branch June 24, 2026 08:15
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