fix(core): FileLoader support .mjs as first-class#6023
Conversation
The default FileLoader match glob only includes `.js`/`.ts` (and `.js` when TypeScript is off), so `.mjs` files are silently skipped. egg4 plugins built as ESM (e.g. via tsdown/tsc emitting `.mjs`) put their `app/middleware`, `app/extend`, `config` etc. in `.mjs`, which then never get loaded — e.g. `app.middlewares` ends up missing plugin middleware, and any lookup like `app.middlewares[name]` throws "Middleware xxx not found". Add `.mjs` to the default match so ESM plugins shipping `.mjs` are scanned the same as `.js`. In `type: module` packages `.js` is already ESM, but many build toolchains emit `.mjs` explicitly; treating `.mjs` as first-class avoids requiring every ESM plugin to re-emit as `.js`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe change extends Changesmjs/cjs loading support
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)Not applicable — the changes are focused loader logic and fixture/test additions without multi-component orchestration warranting a sequence diagram. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 updates the default file loader match pattern to include .mjs files alongside .js and .ts files. The reviewer identified a potential issue where adding .mjs support could lead to duplicate loading of both .ts and compiled .mjs files, as the existing deduplication logic only checks for .js files. It is recommended to update the deduplication check to also handle .mjs files to prevent application crashes.
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 #6023 +/- ##
=======================================
Coverage 81.96% 81.97%
=======================================
Files 678 678
Lines 20800 20800
Branches 4147 4147
=======================================
+ Hits 17048 17050 +2
+ Misses 3235 3233 -2
Partials 517 517 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…s/.cjs - Add `.cjs` to the default FileLoader match alongside `.mjs`. - Extend the source-wins de-dup (previously `.js`-only) to `.mjs`/`.cjs`: when a `.ts` source and its compiled `.js`/`.mjs`/`.cjs` sibling coexist in the same directory (e.g. tsc/tsdown output kept beside the source during development), the `.ts` source wins and the compiled sibling is ignored — otherwise both are attached to the same property and `can't overwrite property` is thrown. (per @gemini-code-assist review) Priority when several extensions coexist for one basename: `.ts` (source) > compiled `.js`/`.mjs`/`.cjs`. Adds fixtures + a test covering standalone .mjs/.cjs loading and the .ts-over-.mjs / .ts-over-.cjs de-dup.
|
Thanks @gemini-code-assist — good catch, fixed in 4a2c566. Extended the "source wins" de-dup (previously if (filepath.endsWith('.js') || filepath.endsWith('.mjs') || filepath.endsWith('.cjs')) {
const filepathTs = filepath.replace(/\.(?:js|mjs|cjs)$/, '.ts');
if (filepaths.includes(filepathTs)) continue;
}Priority when several extensions coexist for one basename: Also added |
|
Thanks for the update, @elrrrrrrr. The logic for handling the source file priority and the addition of |
There was a problem hiding this comment.
Pull request overview
This PR updates Egg core’s FileLoader default glob matching so that .mjs files (and, in the current implementation, .cjs as well) are treated as first-class load targets alongside .js/.ts, and adds regression fixtures/tests to ensure .ts sources take precedence over adjacent compiled outputs.
Changes:
- Expand
FileLoaderdefault match patterns to include.mjs/.cjs, and ignore compiled siblings when a.tssource exists. - Add fixture services for standalone
.mjs/.cjsand “dual”.ts+ compiled output coexistence. - Add a test asserting
.mjs/.cjsloading and.ts-over-compiled precedence.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/loader/file_loader.ts | Extend default glob match to include .mjs/.cjs and skip compiled siblings when .ts exists. |
| packages/core/test/egg-ts.test.ts | Add a regression test around .mjs/.cjs loading and precedence rules. |
| packages/core/test/fixtures/egg-ts-js/app/service/pureMjs.mjs | New standalone .mjs service fixture. |
| packages/core/test/fixtures/egg-ts-js/app/service/pureCjs.cjs | New standalone .cjs service fixture. |
| packages/core/test/fixtures/egg-ts-js/app/service/dual.ts | New .ts source fixture intended to win over compiled sibling. |
| packages/core/test/fixtures/egg-ts-js/app/service/dual.mjs | New compiled .mjs sibling fixture intended to be ignored when .ts exists. |
| packages/core/test/fixtures/egg-ts-js/app/service/dualc.ts | New .ts source fixture intended to win over compiled sibling. |
| packages/core/test/fixtures/egg-ts-js/app/service/dualc.cjs | New compiled .cjs sibling fixture intended to be ignored when .ts exists. |
| function getDefaultFileLoaderMatch(): string[] { | ||
| return isSupportTypeScript() ? ['**/*.(js|ts)', '!**/*.d.ts'] : ['**/*.js']; | ||
| return isSupportTypeScript() ? ['**/*.(js|ts|mjs|cjs)', '!**/*.d.ts'] : ['**/*.{js,mjs,cjs}']; | ||
| } |
| it('should load mjs/cjs and prefer the ts source over its compiled mjs/cjs', async () => { | ||
| mm(process.env, 'EGG_TYPESCRIPT', 'true'); | ||
| app = createApp('egg-ts-js'); | ||
|
|
||
| // loadService must not throw "can't overwrite property" even though | ||
| // `dual.ts`+`dual.mjs` and `dualc.ts`+`dualc.cjs` coexist in the directory. | ||
| await app.loader.loadService(); | ||
|
|
||
| // standalone `.mjs` / `.cjs` are scanned and loaded as first-class files | ||
| assert(app.serviceClasses.pureMjs); | ||
| assert.equal(app.serviceClasses.pureMjs.loadedFrom, 'mjs'); | ||
| assert(app.serviceClasses.pureCjs); | ||
| assert.equal(app.serviceClasses.pureCjs.loadedFrom, 'cjs'); | ||
|
|
||
| // priority: when a `.ts` source and its compiled `.mjs`/`.cjs` coexist, | ||
| // the `.ts` source wins and the compiled sibling is ignored. | ||
| assert(app.serviceClasses.dual); | ||
| assert.equal(app.serviceClasses.dual.loadedFrom, 'ts'); | ||
| assert(app.serviceClasses.dualc); | ||
| assert.equal(app.serviceClasses.dualc.loadedFrom, 'ts'); | ||
| }); |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What
Add
.mjstoFileLoader's default match glob so.mjsfiles are scanned the same as.js/.ts.Why
The default match only includes
.js/.ts, so.mjsfiles are silently skipped. egg4 plugins built as ESM (e.g. tsdown/tsc emitting.mjs) put theirapp/middleware,app/extend,configetc. in.mjs. Those never get loaded →app.middlewaresis missing the plugin's middleware, and a lookup likeapp.middlewares[name]throwsMiddleware xxx not found.Concretely, an ESM plugin shipping
app/middleware/xxx.mjsand pushingxxxintoconfig.tr.middleware(orcoreMiddleware) currently fails to boot because the middleware file is never scanned.In
type: modulepackages.jsis already ESM, but many build toolchains emit.mjsexplicitly. Treating.mjsas first-class avoids forcing every ESM plugin to re-emit as.js.Status
Draft — opening for discussion / direction. Happy to add tests + cover the
.mjsload path if this direction looks right.Summary by CodeRabbit
New Features
.mjsand.cjsfiles in addition to.js, with TypeScript projects also recognizing.tssources.Bug Fixes
.js,.mjs, and.cjsfiles from being loaded alongside matching TypeScript files, avoiding duplicate attachments.