feat: support @eggjs/core next version #21
Conversation
BREAKING CHANGE: drop Node.js < 18.19.0 support eggjs/egg#5257
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent updates focus on improving module management, testing workflows, and modernizing JavaScript features. Key changes include transitioning to ES modules, refining GitHub Actions workflows, updating dependencies, and refactoring import paths for consistency. Additionally, async/await has been introduced to several functions for enhanced readability and performance. The Changes
Sequence Diagram(s)Import and Module Resolution FlowsequenceDiagram
participant Importer
participant Resolver as importResolve()
participant ModuleManager as importModule()
Importer->>Resolver: Request module resolution
Resolver-->>Importer: Return resolved path
Importer->>ModuleManager: Import the resolved module
ModuleManager-->>Importer: Return imported module
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/eslint-config-egg@12.3.1, npm/npm@9.9.3 |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a typosquat?Package name is similar to other popular packages and may not be the package you want. Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
src/deprecated.ts (1)
Line range hint
27-27: Consider using optional chaining to simplify access to nested properties, enhancing code safety and readability.- if (pkg.egg && pkg.egg.framework) { + if (pkg.egg?.framework) {test/plugin.test.ts (1)
Line range hint
21-21: DuplicateafterEachhooks detected. Consider consolidating or removing redundant hooks to enhance clarity and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- .github/workflows/nodejs.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .gitignore (1 hunks)
- package.json (2 hunks)
- src/deprecated.ts (1 hunks)
- src/framework.ts (1 hunks)
- src/import.ts (1 hunks)
- src/index.ts (1 hunks)
- src/plugin.ts (3 hunks)
- test/fixtures/cjs/es-module-default.js (1 hunks)
- test/fixtures/cjs/exports.cjs (1 hunks)
- test/fixtures/cjs/exports.js (1 hunks)
- test/fixtures/cjs/index.js (1 hunks)
- test/fixtures/cjs/package.json (1 hunks)
- test/fixtures/egg-app/get_config.js (1 hunks)
- test/fixtures/egg-app/get_loadunit.js (1 hunks)
- test/fixtures/egg-app/get_plugin.js (1 hunks)
- test/fixtures/egg-app/package.json (1 hunks)
- test/fixtures/esm/exports.js (1 hunks)
- test/fixtures/esm/exports.mjs (1 hunks)
- test/fixtures/esm/index.js (1 hunks)
- test/fixtures/esm/package.json (1 hunks)
- test/framework.test.ts (9 hunks)
- test/getFrameworkOrEggPath.test.ts (1 hunks)
- test/helper.ts (1 hunks)
- test/import.test.ts (1 hunks)
- test/plugin.test.ts (8 hunks)
- tsconfig.json (1 hunks)
Files skipped from review due to trivial changes (15)
- .github/workflows/release.yml
- .gitignore
- src/framework.ts
- test/fixtures/cjs/es-module-default.js
- test/fixtures/cjs/exports.cjs
- test/fixtures/cjs/exports.js
- test/fixtures/cjs/index.js
- test/fixtures/cjs/package.json
- test/fixtures/egg-app/package.json
- test/fixtures/esm/exports.js
- test/fixtures/esm/exports.mjs
- test/fixtures/esm/index.js
- test/fixtures/esm/package.json
- test/helper.ts
- test/import.test.ts
Additional context used
Biome
src/deprecated.ts
[error] 27-27: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
test/plugin.test.ts
[error] 21-21: Disallow duplicate setup and teardown hooks. (lint/suspicious/noDuplicateTestHooks)
Disallow afterEach duplicacy inside the describe function.
Additional comments not posted (30)
test/fixtures/egg-app/get_plugin.js (1)
1-6: The changes in this script are well-implemented and consistent with the intended functionality.tsconfig.json (1)
4-9: The updates to the TypeScript configuration align with modern best practices and enhance the project's type safety and compatibility with recent ECMAScript standards.test/fixtures/egg-app/get_config.js (1)
1-7: The updates in this script are correctly implemented, enhancing the script's utility for debugging and configuration management..github/workflows/nodejs.yml (1)
12-16: The changes to the GitHub Actions workflow are well-aligned with the project's updated requirements and best practices for continuous integration.test/fixtures/egg-app/get_loadunit.js (1)
1-9: This script effectively logs the count of different types of units loaded. Ensure that theprocess.argv[2]input is properly formatted JSON to avoid runtime errors.src/index.ts (1)
1-9: The updates to the export statements align with the new ESM format. Ensure that all referenced files correctly implement the exported functionalities.src/deprecated.ts (1)
3-3: The update to include the.jsextension in the import statement is correct and necessary for ESM compatibility.test/getFrameworkOrEggPath.test.ts (2)
2-3: The updated imports correctly reference the new file locations and utilities, ensuring that the tests have access to necessary functionalities.
5-37: The test cases are well-structured and effectively test various scenarios for thegetFrameworkOrEggPathfunction. The use ofgetFilepathenhances consistency in path handling.package.json (7)
4-6: Updated Node.js engine requirement to >=18.19.0 aligns with the new support policy.
13-19: Added several new scripts to enhance the build and test processes.
33-46: Significant updates to dependencies to support new features and ensure compatibility with the latest standards.
48-51: Includingdistandsrcin thefilesarray ensures that only necessary files are packaged, enhancing the cleanliness of the package.
52-52: Switching to ES Modules withtype: moduleis a significant shift that aligns with modern JavaScript practices.
59-73: Theexportsfield configuration supports both CommonJS and ESM, which is crucial for compatibility across different module systems.
74-75: Setting main entry point and types definition for CommonJS explicitly.src/import.ts (3)
1-3: Imports from Node.js built-in modules to support utility functions.
18-29:importResolvefunction correctly utilizes custom or native require to resolve module paths, enhancing flexibility in module resolution.
31-82:importModulefunction supports both CommonJS and ESM, handling different module formats effectively. Good use of async/await for ESM imports.test/framework.test.ts (3)
5-6: Utilizes helper functions for path handling to maintain consistency and reduce redundancy.
Line range hint
13-109: Updated tests to use the new path handling functions, ensuring that the tests are robust and maintainable.
Line range hint
125-139: Tests for default and monorepo configurations are well-handled, ensuring the framework path is correctly resolved in complex setups.src/plugin.ts (4)
1-7: Imports and setup for plugin utilities are correctly implemented, ensuring foundational tools are available for plugin handling.
Line range hint
42-65: FunctionsgetPlugins,getLoadUnits, andgetConfigare well implemented with async/await, ensuring plugins are loaded and configured asynchronously.
95-120:getLoaderfunction is robust, handling various scenarios with assertions and dynamic path handling, ensuring the loader is correctly set up.
122-161:findEggCorefunction effectively searches for core dependencies, handling errors and paths intelligently.test/plugin.test.ts (4)
8-9: Updated import paths to include.jsextensions align with the new module system. Ensure all related files are correctly updated to ES modules as well.
12-13: Correct usage of helper functions for path handling, enhancing maintainability and adaptability of the test environment setup.
34-34: Ensure that the expected output in tests (/get all plugins \["onerror",/) matches the actual output from the application, especially after changes in the plugin management.Also applies to: 48-48, 63-63
66-66: The async functionsgetPluginsandgetLoadUnitsare correctly implemented and used. Ensure that they handle errors gracefully and that the tests cover possible edge cases.Also applies to: 108-108
| .expect('code', 0) | ||
| .end(); | ||
| const config = utils.getConfig({ | ||
| const config = await utils.getConfig({ |
There was a problem hiding this comment.
Ensure the getConfig function correctly fetches configuration from the specified framework path. Consider adding more robust error handling and logging for better diagnostics.
- const config = await utils.getConfig({
+ try {
+ const config = await utils.getConfig({
baseDir: tmp,
framework: path.join(tmp, 'node_modules/framework-demo'),
});
+ assert(config);
+ assert.equal(config.name, 'egg-app');
+ } catch (error) {
+ console.error('Error fetching config:', error);
+ throw error;
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const config = await utils.getConfig({ | |
| try { | |
| const config = await utils.getConfig({ | |
| baseDir: tmp, | |
| framework: path.join(tmp, 'node_modules/framework-demo'), | |
| }); | |
| assert(config); | |
| assert.equal(config.name, 'egg-app'); | |
| } catch (error) { | |
| console.error('Error fetching config:', error); | |
| throw error; | |
| } |
| @@ -162,8 +160,7 @@ describe('test/plugin.test.ts', () => { | |||
| }); | |||
| await coffee.fork(bin, [ args ], { cwd: tmp }) | |||
| .debug() | |||
| // .expect('stdout', /get app configs session,security,helper,jsonp,onerror,i18n,watcher,multipart,logrotator,static,view,env,name,keys,proxy,protocolHeaders,ipHeaders,hostHeaders,pkg,baseDir,HOME,rundir,dump,notfound,siteFile,bodyParser,logger,httpclient,coreMiddleware,workerStartTimeout,coreMiddlewares,appMiddlewares,appMiddleware/) | |||
| .expect('stdout', /get app configs/) | |||
| .expect('stdout', /get app configs \["session"/) | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that references to session are present in the test/plugin.test.ts file, specifically within the test cases. However, there are no references to session in the main codebase, particularly within the getConfig method.
- Lines 163, 177, and 191 in
test/plugin.test.tscontain expectations related tosession.
Given this, it appears that the test cases might be outdated or misaligned with the current implementation of getConfig, which does not directly reference session.
Analysis chain
Updated test cases for the getConfig method to ensure they reflect the new configurations and dependencies. Verify that the configurations being tested are up-to-date with the latest application settings.
Also applies to: 177-177, 191-191
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the configurations being tested are up-to-date with the latest application settings.
# Test: Search for the configuration settings in the application code.
rg --type config $'/get app configs \["session"/'
Length of output: 199
Script:
#!/bin/bash
# Search for the `getConfig` method in the codebase.
ast-grep --lang typescript --pattern 'function getConfig($_) { $$$ }'
Length of output: 370
Script:
#!/bin/bash
# Search for references to `session` in the codebase.
rg 'session'
Length of output: 316
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21 +/- ##
==========================================
- Coverage 88.85% 87.65% -1.21%
==========================================
Files 5 6 +1
Lines 296 413 +117
Branches 40 53 +13
==========================================
+ Hits 263 362 +99
- Misses 33 51 +18 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [4.0.0](v3.0.1...v4.0.0) (2024-06-17) ### ⚠ BREAKING CHANGES * drop Node.js < 18.19.0 support eggjs/egg#5257 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new utility functions for resolving and importing modules with support for CommonJS and ESM formats. - Added new test fixtures for CommonJS and ESM modules to validate module import functionality. - **Refactor** - Updated import statements to include file extensions (`.js`) for consistency and compatibility. - Refactored code to use async/await for asynchronous operations. - Improved path handling in tests with helper functions. - **Documentation** - Updated `package.json` with new scripts, dependencies, and module management configurations. - **Chores** - Enhanced `.gitignore` to exclude `.tshy*` files and `dist/` directory. - Modified GitHub Actions workflows for Node.js and release processes. - **Tests** - Added tests for new module import functions. - Updated existing tests to reflect new import paths and async changes. - **Configuration** - Updated `tsconfig.json` for stricter TypeScript settings and modern module resolution. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Features * support @eggjs/core next version ([#21](#21)) ([a37968c](a37968c))
BREAKING CHANGE: drop Node.js < 18.19.0 support
eggjs/egg#5257
Summary by CodeRabbit
New Features
Refactor
.js) for consistency and compatibility.Documentation
package.jsonwith new scripts, dependencies, and module management configurations.Chores
.gitignoreto exclude.tshy*files anddist/directory.Tests
Configuration
tsconfig.jsonfor stricter TypeScript settings and modern module resolution.