Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Deploying egg with
|
| Latest commit: |
0d1f1eb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3fe164e0.egg-cci.pages.dev |
| Branch Preview URL: | https://egg-15-typings-bundle-loader.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5895 +/- ##
==========================================
- Coverage 85.52% 85.50% -0.02%
==========================================
Files 662 662
Lines 18863 18875 +12
Branches 3658 3662 +4
==========================================
+ Hits 16132 16140 +8
- Misses 2360 2362 +2
- Partials 371 373 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Deploying egg-v3 with
|
| Latest commit: |
0d1f1eb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dac41da1.egg-v3.pages.dev |
| Branch Preview URL: | https://egg-15-typings-bundle-loader.egg-v3.pages.dev |
![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 the @eggjs/typings package and implements a global bundle module loader within @eggjs/utils to support bundled applications. The importModule function is updated to check for a registered loader before proceeding with standard imports, including logic for path normalization and module unwrapping. Review feedback suggests extending the BundleModuleLoader type to accept options for better manifest verification in multi-application environments and refactoring the module unwrapping logic to reduce duplication and improve maintainability.
| * serve modules that no longer exist on disk. Return `undefined` to fall | ||
| * through to the standard import path. | ||
| */ | ||
| export type BundleModuleLoader = (filepath: string) => unknown; |
There was a problem hiding this comment.
The BundleModuleLoader currently only receives the raw filepath. In multi-application environments, it is important to verify that the global bundle store's baseDir matches the requested baseDir to prevent using an incorrect manifest. Consider passing options (including baseDir) to the loader to facilitate this check.
| export type BundleModuleLoader = (filepath: string) => unknown; | |
| export type BundleModuleLoader = (filepath: string, options?: { baseDir?: string; [key: string]: any }) => unknown; |
References
- When using a global bundle store, verify that its baseDir matches the requested baseDir to prevent using an incorrect manifest in multi-application environments.
|
|
||
| export async function importModule(filepath: string, options?: ImportModuleOptions): Promise<any> { | ||
| const _bundleModuleLoader = globalThis.__EGG_BUNDLE_MODULE_LOADER__; | ||
| if (_bundleModuleLoader) { | ||
| const hit = _bundleModuleLoader(normalizeBundleModulePath(filepath)); | ||
| 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; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for unwrapping __esModule "double default" and handling importDefaultOnly is duplicated multiple times. Introducing a private helper function improves maintainability and ensures consistency. Additionally, passing options to the bundle loader allows it to verify the baseDir against the global bundle store, ensuring the correct manifest is used in multi-application environments.
function unwrapModule(obj: any, options?: ImportModuleOptions): 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;
}
export async function importModule(filepath: string, options?: ImportModuleOptions): Promise<any> {
const _bundleModuleLoader = globalThis.__EGG_BUNDLE_MODULE_LOADER__;
if (_bundleModuleLoader) {
const hit = _bundleModuleLoader(normalizeBundleModulePath(filepath), options);
if (hit !== undefined) {
return unwrapModule(hit, options);
}
}References
- When using a global bundle store, verify that its baseDir matches the requested baseDir to prevent using an incorrect manifest in multi-application environments.
1 participant
Summary
@eggjs/typingsfor shared BundleModuleLoader/global typing@eggjs/utilsconsume BundleModuleLoader from@eggjs/typingsand keep the bundle loader registry onglobalThis@eggjs/coreto the shared typings package and add bundle import coverageCloses EGG-15.
Local validation
pnpm -r --if-present run typecheckpassedpnpm exec oxlint --type-aware --type-check --quietpassed (274 existing warnings, 0 errors)pnpm exec oxfmt --check .passedpnpm exec vitest run packages/utils/test/bundle-import.test.ts --bail 1 --retry 2 --testTimeout 20000 --hookTimeout 20000passedpnpm exec vitest run packages/utils/test/import.test.ts packages/utils/test/index.test.ts --bail 1 --retry 2 --testTimeout 20000 --hookTimeout 20000passedpnpm --filter @eggjs/typings exec tsdownpassedpnpm --filter @eggjs/utils exec tsdownpassedpnpm --filter @eggjs/core exec tsdownpassedNotes
pnpm run typecheckandpnpm run cistill fail before validation starts becauseut run clean --workspaces --if-presentreportsERROR Script 'clean' not found in package.json, same local repo blocker observed in PR feat(utils): add setBundleModuleLoader runtime hook #5867.