fix: support load plugin from typescript dir #287
Conversation
WalkthroughThis pull request introduces enhancements to the Egg.js framework's plugin loading mechanism, with a focus on improving TypeScript support. The changes include updating the Changes
Sequence DiagramsequenceDiagram
participant Loader as EggLoader
participant Plugin as Plugin Package
participant TSConfig as TypeScript Config
Loader->>Plugin: Request plugin path
Plugin-->>Loader: Return package.json
Loader->>TSConfig: Check TypeScript export
alt TypeScript path exists
Loader->>Loader: Use TypeScript source path
else
Loader->>Loader: Fallback to default paths
end
Loader->>Loader: Load plugin module
Possibly Related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #287 +/- ##
==========================================
+ Coverage 97.73% 97.76% +0.03%
==========================================
Files 10 10
Lines 3351 3363 +12
Branches 596 599 +3
==========================================
+ Hits 3275 3288 +13
+ Misses 76 75 -1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/loader/egg_loader.ts (1)
101-102: Consider guarding against registration failures
While the logic for checkingtsconfig.jsonis good, you might consider wrappingtsconfigPathsRegisterin a try/catch to handle unforeseen registration errors, thereby strengthening fault tolerance.Also applies to: 104-105
test/loader/mixin/load_plugin.test.ts (1)
225-231: Consider adding non-ESM coverage
This new test effectively verifies the TypeScript path scenario but does not cover theexports.requirebranch for CommonJS fallback. Adding an additional test would help ensure complete coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
package.json(1 hunks)src/loader/egg_loader.ts(4 hunks)test/fixtures/plugin-ts-src/config/config.js(1 hunks)test/fixtures/plugin-ts-src/config/plugin.js(1 hunks)test/fixtures/plugin-ts-src/package.json(1 hunks)test/fixtures/plugin-ts-src/plugins/g/package.json(1 hunks)test/fixtures/plugin-ts-src/plugins/g/src/index.ts(1 hunks)test/loader/mixin/load_plugin.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- test/fixtures/plugin-ts-src/package.json
- test/fixtures/plugin-ts-src/plugins/g/package.json
- test/fixtures/plugin-ts-src/config/plugin.js
- test/fixtures/plugin-ts-src/plugins/g/src/index.ts
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/loader/egg_loader.ts
[warning] 775-778: src/loader/egg_loader.ts#L775-L778
Added lines #L775 - L778 were not covered by tests
🔇 Additional comments (6)
src/loader/egg_loader.ts (4)
8-10: Ensure new import is fully utilized
The addition ofexistsfrom'utility'is consistent with its usage later in the file (e.g., line 780). No immediate issues found.
13-14: Importing tsconfigPathsRegister
No issues found with this import statement. It successfully integrates the TypeScript paths registration logic.
604-604: Path resolution for plugin
Invokingawait this.#formatPluginPathFromPackageJSON(...)is a sensible approach to ensure that asynchronous checks for the plugin's path complete before further usage.
758-787: Expand test coverage for require path
Lines 775–778 (in the non-ESM branch) are not covered by tests, as flagged by static analysis. Consider adding a test scenario that forces execution of theexports.requirebranch to increase coverage and validate the fallback logic.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 775-778: src/loader/egg_loader.ts#L775-L778
Added lines #L775 - L778 were not covered by teststest/fixtures/plugin-ts-src/config/config.js (1)
1-3: Configuration overrides
Declaring thepluginandmiddlewareproperties is straightforward. No major issues or conflicts identified.package.json (1)
43-43: Verify updated dependency
The bump of@eggjs/utilsto^4.2.4looks safe. However, please confirm there are no breaking changes by running the full test suite.
[skip ci] ## [6.2.13](v6.2.12...v6.2.13) (2025-01-04) ### Bug Fixes * support load plugin from typescript dir ([#287](#287)) ([7adabd5](7adabd5))
Summary by CodeRabbit
Release Notes
Dependencies
@eggjs/utilsfrom version 4.1.5 to 4.2.4Improvements
Testing
These updates improve the framework's compatibility and robustness when working with TypeScript plugins.