feat: support pkg.eggPlugin.exports property
#274
Conversation
e.g.:
```json
"eggPlugin": {
"name": "schedule",
"exports": {
"import": "./dist/esm",
"require": "./dist/commonjs"
}
}
```
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces enhancements to the Egg.js core library, focusing on improving the loader's asynchronous capabilities, error handling, and utility functions. The changes include adding a Node.js version badge to the README, modifying the Changes
Sequence DiagramsequenceDiagram
participant Loader as EggLoader
participant Utils as Utils
participant PluginConfig as Plugin Config
Loader->>PluginConfig: loadPlugin()
Loader->>Loader: #mergePluginConfig (async)
Loader->>Utils: existsPath()
Utils-->>Loader: File existence result
Loader->>Loader: #formatPluginPathFromPackageJSON()
Loader-->>PluginConfig: Resolved Plugin Configuration
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 (
|
![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 (1)
src/utils/index.ts (1)
57-57: Consider the impact of removingconsole.traceon deprecation warningsBy commenting out
console.trace, deprecation warnings will no longer include stack traces. Evaluate whether this change might make it harder for developers to locate the source of deprecated usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)src/loader/egg_loader.ts(7 hunks)src/utils/index.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🧰 Additional context used
🪛 Biome (1.9.4)
src/utils/index.ts
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (11)
src/loader/egg_loader.ts (8)
8-8: Imported readJSON for asynchronous JSON reading
The addition of readJSON facilitates asynchronous reading of JSON files, enhancing non-blocking operations.
465-465: Properly updated to await the asynchronous #mergePluginConfig method
The call to #mergePluginConfig has been correctly updated to use await, reflecting its new asynchronous nature.
541-541: Added debug logging for custom plugins
The debug statement improves visibility into the loading of custom plugins.
626-636: Transitioned #mergePluginConfig to asynchronous execution
The #mergePluginConfig method is now asynchronous, which allows non-blocking file I/O operations for plugin configurations.
630-637: Asynchronously reading plugin package configuration
Using await readJSON(pluginPackage) ensures that the plugin's package.json is read without blocking the event loop.
717-718: Logging implicitly enabled plugins for transparency
The code logs plugins that are enabled implicitly, which improves transparency and aids in debugging plugin dependencies.
791-810: Added #formatPluginPathFromPackageJSON method to handle plugin exports
The new method correctly resolves plugin paths based on the exports field in package.json, accommodating both CommonJS and ESM formats.
785-787: Verify Node.js version compatibility for Error constructor with cause property
The use of the cause option in the Error constructor is supported in Node.js v16 and above. Ensure that the project's minimum Node.js version requirement reflects this to prevent compatibility issues.
Run the following script to verify the Node.js engine version requirement:
✅ Verification successful
Node.js version requirement is compatible with Error cause property
The project requires Node.js version >= 18.19.0, which fully supports the Error constructor's cause property (introduced in Node.js 16.9.0). The code usage is safe and compatible with the project's minimum Node.js version requirement.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the Node.js version requirement specified in package.json
# Test: Display the Node.js version requirement
jq '.engines.node' package.json
Length of output: 44
src/utils/index.ts (3)
4-4: Imported stat from node:fs/promises for asynchronous file operations
The import allows the use of stat in an asynchronous manner, improving non-blocking file system checks.
22-53: Refactored getCalleeFromStack as a standalone function
The getCalleeFromStack function has been redefined as a standalone function, enhancing clarity and modularity of the code.
🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
64-71: Added existsPath method for asynchronous file existence checks
The new existsPath method provides an asynchronous way to check for file existence using stat, promoting non-blocking code execution.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/utils/index.ts (1)
57-62: Consider using structured logging.The current implementation could benefit from structured logging to make it easier to parse and analyze logs in production.
- if (debug.enabled) { - console.trace('[@eggjs/core:deprecated] %s', message); - } else { - console.warn('[@eggjs/core:deprecated] %s', message); - console.warn('[@eggjs/core:deprecated] set NODE_DEBUG=@eggjs/core:utils can show call stack'); - } + const logData = { + type: 'deprecation', + message, + stack: debug.enabled ? new Error().stack : undefined, + }; + if (debug.enabled) { + console.trace('[@eggjs/core:deprecated]', logData); + } else { + console.warn('[@eggjs/core:deprecated]', logData); + console.warn('[@eggjs/core:deprecated] set NODE_DEBUG=@eggjs/core:utils can show call stack'); + }test/loader/mixin/load_plugin.test.ts (1)
27-35: Consider adding edge cases for plugin exports.While the basic functionality is tested, consider adding test cases for:
- Invalid exports configuration
- Missing export paths
- Conflicting export formats
Example test case:
it('should handle invalid plugin exports configuration', async () => { app = createApp('plugin-pkg-exports-invalid'); const loader = app.loader; await assert.rejects(async () => { await loader.loadPlugin(); }, /Invalid exports configuration/); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
test/fixtures/plugin-pkg-exports/node_modules/a/foo_dist/commonjs/config/config.default.jsis excluded by!**/node_modules/**test/fixtures/plugin-pkg-exports/node_modules/a/foo_dist/esm/config/config.default.jsis excluded by!**/node_modules/**test/fixtures/plugin-pkg-exports/node_modules/a/package.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (6)
.github/workflows/nodejs.yml(1 hunks)src/utils/index.ts(3 hunks)test/fixtures/plugin-egg-plugin/config/plugin.js(0 hunks)test/fixtures/plugin-pkg-exports/config/plugin.js(1 hunks)test/fixtures/plugin-pkg-exports/package.json(1 hunks)test/loader/mixin/load_plugin.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- test/fixtures/plugin-egg-plugin/config/plugin.js
✅ Files skipped from review due to trivial changes (2)
- test/fixtures/plugin-pkg-exports/config/plugin.js
- test/fixtures/plugin-pkg-exports/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
src/utils/index.ts
[error] 40-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
.github/workflows/nodejs.yml (2)
14-14: LGTM! Adding Node.js v23 testing is beneficial.
Testing against Node.js v23 is particularly relevant for this PR as it enhances support for package exports, which aligns with the PR's objective of supporting the pkg.eggPlugin.exports property.
14-14: Verify Node.js version compatibility in package.json
Since we're adding Node.js v23 to the test matrix, we should ensure the package.json's engines field reflects the supported Node.js versions correctly.
✅ Verification successful
Node.js version compatibility is correctly configured
The package.json's engines field specifies ">= 18.19.0" which aligns perfectly with the Node.js versions being tested (18.19.0, 18, 20, 22, 23) in the workflow. No explicit version checks were found in the codebase that might conflict with these versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Node.js version compatibility settings
# Check if package.json exists and contains engines field
echo "Checking package.json for Node.js version constraints..."
if [ -f package.json ]; then
jq '.engines.node' package.json
fi
# Look for any explicit version checks in the codebase
echo "Checking for explicit Node.js version checks in the code..."
rg -l "process\.version"
Length of output: 349
src/utils/index.ts (1)
68-75: LGTM! Clean async implementation.
The implementation correctly uses async/await with proper error handling.
test/loader/mixin/load_plugin.test.ts (1)
17-25: LGTM! Good test coverage for plugin exports.
The test case properly verifies the presence of essential plugin properties.
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #274 +/- ##
==========================================
- Coverage 92.98% 92.88% -0.10%
==========================================
Files 10 10
Lines 3238 3291 +53
Branches 508 528 +20
==========================================
+ Hits 3011 3057 +46
- Misses 227 234 +7 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [6.1.0](v6.0.3...v6.1.0) (2024-12-17) ### Features * support `pkg.eggPlugin.exports` property ([#274](#274)) ([df9efed](df9efed))
e.g.:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation