feat: support cjs and esm both by tshy #14
Conversation
BREAKING CHANGE: drop Node.js < 18.19.0 support part of eggjs/egg#3644 eggjs/egg#5257
|
Warning Rate limit exceeded@fengmk2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 4 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 (4)
WalkthroughThe pull request represents a comprehensive modernization of the Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant I18n as I18n Plugin
participant Locale as Locale Files
participant Context as Request Context
App->>I18n: Initialize
I18n->>Locale: Load Translation Files
Context->>I18n: Request Locale
I18n->>Context: Provide Translation Method
Context->>Locale: Retrieve Translated String
Locale-->>Context: Return Localized Text
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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/config/config.default.ts (1)
3-12: Consider using type declaration instead of type assertionWhile the type assertion works, consider declaring the type directly for better type safety:
-export default { +export default <I18nConfig>{ i18n: { defaultLocale: 'en_US', dirs: [], queryField: 'locale', cookieField: 'locale', cookieDomain: '', cookieMaxAge: '1y', - } as I18nConfig, + }, };test/fixtures/custom_egg/index.js (1)
24-28: Consider simplifying the startCluster functionThe function could be simplified by using object spread and default parameters:
-export function startCluster(options, callback) { - options = options || {}; - options.customEgg = EGG_PATH; - _startCluster(options, callback); +export function startCluster(options = {}, callback) { + _startCluster({ ...options, customEgg: EGG_PATH }, callback); }src/types.ts (1)
66-69: Track technical debt for koa-locales refactorThere's a TODO comment about removing methods after koa-locales refactor. Consider creating an issue to track this technical debt.
Would you like me to create a GitHub issue to track the koa-locales refactor work?
.github/workflows/pkg.pr.new.yml (1)
1-2: Optimize workflow trigger conditions.The workflow currently runs on both push and pull request events without any conditions. Consider:
- Limiting to specific branches for push events
- Adding conditions to check if package version has changed
name: Publish Any Commit -on: [push, pull_request] +on: + push: + branches: [ master ] + paths: + - 'package.json' + pull_request: + branches: [ master ] + paths: + - 'package.json'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.autod.conf.js(0 hunks).eslintignore(1 hunks).eslintrc(1 hunks).github/workflows/nodejs.yml(1 hunks).github/workflows/pkg.pr.new.yml(1 hunks).github/workflows/release.yml(1 hunks).gitignore(1 hunks).travis.yml(0 hunks)README.md(4 hunks)app.js(0 hunks)app/middleware/i18n.js(0 hunks)appveyor.yml(0 hunks)config/config.default.js(0 hunks)package.json(2 hunks)src/app.ts(1 hunks)src/app/extend/context.ts(1 hunks)src/config/config.default.ts(1 hunks)src/index.ts(1 hunks)src/types.ts(1 hunks)src/typings/index.d.ts(1 hunks)test/fixtures/apps/i18n/app/controller/message.js(0 hunks)test/fixtures/apps/i18n/config/locales/zh-CN.js(1 hunks)test/fixtures/custom_egg/config/locales/zh-CN.js(1 hunks)test/fixtures/custom_egg/index.js(1 hunks)test/fixtures/custom_egg/package.json(1 hunks)test/i18n.test.ts(6 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (7)
- appveyor.yml
- .travis.yml
- app/middleware/i18n.js
- config/config.default.js
- test/fixtures/apps/i18n/app/controller/message.js
- .autod.conf.js
- app.js
✅ Files skipped from review due to trivial changes (5)
- src/typings/index.d.ts
- .eslintignore
- src/index.ts
- .eslintrc
- tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Node.js / Test (windows-latest, 22)
- GitHub Check: Node.js / Test (windows-latest, 20)
- GitHub Check: Node.js / Test (windows-latest, 18.19.0)
🔇 Additional comments (19)
src/app.ts (1)
6-6: Verify thatexistsis an asynchronous functionEnsure that the
existsfunction imported from'utility'returns a Promise since you're usingawait exists(localePath).If
existsis synchronous, usingawaitwon't have the intended effect. Please verify the implementation ofexists.test/fixtures/custom_egg/config/locales/zh-CN.js (1)
1-3: Ensure ES module syntax is supported in your environmentChanging the export from
module.exportstoexport defaultrequires that your Node.js environment and module loader support ES modules for this file.Ensure that your project is configured to handle ES module syntax for
.jsfiles, or consider renaming the file to.mjsor adding"type": "module"in yourpackage.json.test/fixtures/apps/i18n/config/locales/zh-CN.js (1)
6-6: Translation key added correctlyThe new translation for
"Hello %s, how are you today? How was your %s."has been added properly.src/config/config.default.ts (1)
1-12: LGTM! Configuration structure is well-definedThe default configuration provides sensible defaults and follows ESM best practices with the
.jsextension in the import path.src/app/extend/context.ts (1)
1-16: LGTM! Well-structured TypeScript implementationThe context extension is properly typed and documented. The getter/setter pattern is well implemented with proper type safety.
test/fixtures/custom_egg/index.js (1)
1-28: LGTM! Proper ESM migrationThe migration to ESM follows best practices:
- Uses
node:protocol for built-in modules- Properly handles ESM paths with
fileURLToPath- Clean export syntax
src/types.ts (1)
1-70: LGTM! Well-documented type definitionsThe type definitions are comprehensive and well-documented:
- Clear interface definitions with detailed JSDoc comments
- Proper module augmentation for @eggjs/core
- Type-safe method signatures
test/i18n.test.ts (4)
1-7: Well-structured ESM imports and module setup!The code follows Node.js best practices for ES modules by:
- Using the
node:protocol for core modules- Properly handling
__filenameand__dirnamein ESM context
9-16: Good use of TypeScript and modern async patterns!The test suite setup demonstrates good practices with:
- Explicit typing of the
appvariable- Use of async/await in hooks
58-74: Well-structured test for cookie-based locale!The new test case thoroughly verifies locale retrieval from cookies with comprehensive assertions.
24-24: Verify cookie expiration behavior with max-ageThe change from using only
expiresto includingmax-ageis good practice. However, please verify that:
- The
max-age=31557600(1 year) aligns with your session management requirements- Both
max-ageandexpiresare necessary for your browser compatibility needsAlso applies to: 43-43, 92-92, 111-111
✅ Verification successful
Cookie expiration configuration is correctly implemented
The cookie settings align with the codebase configuration:
cookieMaxAge: '1y'in default config correctly translates tomax-age=31557600- Both
max-ageandexpiresare intentionally used for broad browser compatibility- The implementation allows flexible configuration via
cookieMaxAgesetting🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent cookie expiration across the codebase rg -l "max-age=31557600" | while read -r file; do echo "=== $file ===" rg "max-age=31557600|expires=" "$file" doneLength of output: 1398
Script:
#!/bin/bash # Find cookie-related implementation files and cookie setting code echo "=== Searching for cookie/locale related files ===" fd -e ts -e js -E '*.test.*' -E '*.spec.*' | xargs rg -l "setCookie|cookie|locale" || true echo -e "\n=== Searching for cookie setting implementation ===" ast-grep -p 'ctx.cookies.set($$$)' || trueLength of output: 689
Script:
#!/bin/bash echo "=== Checking context extension implementation ===" cat src/app/extend/context.ts echo -e "\n=== Checking default configuration ===" cat src/config/config.default.ts echo -e "\n=== Searching for alternative cookie patterns ===" rg "cookie" -A 5 -B 5 src/Length of output: 3494
test/fixtures/custom_egg/package.json (1)
2-3: Verify ESM compatibility across the test fixturesThe addition of
"type": "module"is necessary for ESM support. However, please verify that:
- All JavaScript files in the custom_egg fixture are ESM-compatible
- No CommonJS-specific features are used that might break
✅ Verification successful
ESM compatibility verified in custom_egg fixture ✅
The
"type": "module"declaration is correct and necessary as the fixture uses ESM features:
- Native ESM imports from
node:modules- Multiple export statements including classes and functions
- No CommonJS patterns that could cause issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential CommonJS usage in the test fixture echo "=== Checking for require() calls ===" rg "require\(" "test/fixtures/custom_egg" echo "=== Checking for module.exports ===" rg "module\.exports" "test/fixtures/custom_egg" echo "=== Checking for exports. assignments ===" rg "exports\." "test/fixtures/custom_egg"Length of output: 388
Script:
#!/bin/bash # List all JavaScript files echo "=== JavaScript files in custom_egg ===" fd -e js . "test/fixtures/custom_egg" echo -e "\n=== Checking for import statements ===" rg "^import\s+.*from\s+" "test/fixtures/custom_egg" -n echo -e "\n=== Checking for export statements ===" rg "^export\s+" "test/fixtures/custom_egg" -nLength of output: 951
.gitignore (1)
1-11: Comprehensive .gitignore patterns!The updated .gitignore includes all necessary patterns for:
- TypeScript build artifacts
- Development tools
- Dependency management
- Test coverage
.github/workflows/release.yml (1)
1-13: Verify release workflow configurationThe workflow is well-structured but please verify:
- The required secrets (NPM_TOKEN, GIT_TOKEN) are configured in the repository
- The shared workflow from eggjs/github-actions is compatible with the new ESM setup
.github/workflows/nodejs.yml (1)
13-14: LGTM! Node.js version range aligns with project requirements.The Node.js versions (18.19.0, 20, 22) align well with the PR objective of dropping support for older versions. The range covers both LTS and latest versions.
package.json (3)
9-13: LGTM! Well-structured exports configuration.The eggPlugin exports configuration correctly defines paths for both ESM and CommonJS modules, with TypeScript source support.
71-83: Verify dual package hazards.The exports field correctly defines both ESM and CommonJS entry points. However, we should verify for dual package hazards.
✅ Verification successful
Dual package hazard verification completed successfully
The package correctly implements dual-package support following Node.js best practices, with clean separation between ESM and CommonJS modules, consistent file extensions, and proper type definitions for both formats. No hazardous patterns were detected in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for dual package hazards using @arethetypeswrong/cli # This tool is already in devDependencies npx attw --packLength of output: 111
Script:
#!/bin/bash # Check for mixed import/require usage and re-export patterns echo "=== Checking for mixed import/require usage ===" rg "require\(" -A 2 -B 2 echo -e "\n=== Checking for import statements ===" rg "import .* from" -A 2 -B 2 echo -e "\n=== Checking for re-export patterns ===" rg "export \* from|export \{.*\} from" -A 2 -B 2 echo -e "\n=== Checking index files for export patterns ===" fd "index\.(js|ts|mjs|cjs)$" --exec cat {}Length of output: 3154
34-36: Verify dependency compatibility.Need to verify if the dependencies support both ESM and CommonJS formats.
✅ Verification successful
Dependencies are compatible with project's ESM setup
- @eggjs/core and utility packages fully support both ESM and CommonJS
- koa-locales is CommonJS-only but works in ESM context through Node.js module interop
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if dependencies support both ESM and CJS # Check @eggjs/core npx attw --dependency @eggjs/core # Check koa-locales npx attw --dependency koa-locales # Check utility npx attw --dependency utilityLength of output: 386
Script:
#!/bin/bash # Search for import/require patterns for these dependencies echo "=== Import/Require Patterns ===" rg -A 2 "require\(['\"]@eggjs/core['\"]\)|import .* from ['\"]@eggjs/core['\"]" || true rg -A 2 "require\(['\"]koa-locales['\"]\)|import .* from ['\"]koa-locales['\"]" || true rg -A 2 "require\(['\"]utility['\"]\)|import .* from ['\"]utility['\"]" || true # Check if there are any .mjs or .cjs files echo -e "\n=== Module File Extensions ===" fd "\.(mjs|cjs)$" || true # Look for type:module in package.json echo -e "\n=== Package Type ===" rg "\"type\":\s*\"module\"" package.json || trueLength of output: 1131
Script:
#!/bin/bash # Fetch package info from npm registry echo "=== @eggjs/core Package Info ===" curl -s https://registry.npmjs.org/@eggjs/core/6.2.13 | jq '.exports, .type, .main, .module' echo -e "\n=== koa-locales Package Info ===" curl -s https://registry.npmjs.org/koa-locales/1.12.0 | jq '.exports, .type, .main, .module' echo -e "\n=== utility Package Info ===" curl -s https://registry.npmjs.org/utility/2.4.0 | jq '.exports, .type, .main, .module'Length of output: 1188
README.md (1)
141-143: LGTM! Clear priority documentation.The documentation clearly explains the priority order for language selection through query parameters, cookies, and headers.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/app.ts (2)
80-84:⚠️ Potential issueAdd only existing directories to
i18nConfig.dirsThe code adds
localePathtoi18nConfig.dirswithout confirming its existence.
92-94:⚠️ Potential issueHandle potential null values without using non-null assertion operator
The use of non-null assertion operator (
!) should be avoided.
🧹 Nitpick comments (14)
src/locales.ts (4)
18-20: Consider Prependingoptions.dirtolocaleDirsCurrently, if
options.diris specified and not already included inlocaleDirs, it is added to the end of the array. If you wantoptions.dirto take precedence over other directories, consider adding it to the beginning of the array.Apply this diff to prepend
options.dir:- localeDirs.push(options.dir); + localeDirs.unshift(options.dir);
22-25: Add Error Handling for Directory Existence CheckWhile checking if each directory exists using
await exists(dir), there is no error handling in case of exceptions (e.g., permission issues). Consider wrapping this in a try-catch block to handle potential errors gracefully.Apply this diff to add error handling:
- if (!(await exists(dir))) { - continue; - } + try { + if (!(await exists(dir))) { + continue; + } + } catch (err) { + console.error(`Error accessing directory ${dir}:`, err); + continue; + }
34-44: Handle Unsupported File TypesCurrently, files with unsupported extensions are silently ignored. It might be beneficial to log a warning or handle such cases explicitly, so that any unexpected files are accounted for, and potential misconfigurations are easier to debug.
Apply this diff to log a warning for unsupported file types:
} else if (name.endsWith('.yml') || name.endsWith('.yaml')) { resource = flattening(yaml.load(await fs.readFile(filepath, 'utf8'))); } else { + console.warn(`Unsupported file type for locale: ${filepath}`); }
55-73: Specify Types inflatteningFunction for Type SafetyThe
flatteningfunction currently usesanytypes, which reduces type safety. Specify the parameter and return types to improve type checking and maintainability.Apply this diff to add explicit types:
- function flattening(data: any) { + function flattening(data: Record<string, any>): Record<string, string> {And update the
deepFlatfunction accordingly:function deepFlat(data: any, prefix: string) {src/app/extend/application.ts (1)
69-91: Handle Missing Values informatWithObjectFunctionIn
formatWithObject, if a placeholder doesn't have a corresponding value in thevaluesobject, it returns the original placeholder. Consider providing a default value or throwing an error to handle missing values more gracefully.Apply this diff to provide a default value:
const value = values[matched]; - if (value) { + if (value !== undefined) { return value; }Or consider logging a warning:
if (value !== undefined) { return value; } + console.warn(`No value provided for placeholder: ${matched}`);src/app/extend/context.ts (1)
88-157: Refactor Locale Determination Logic for ReadabilityThe
__getLocalemethod contains complex logic with nested conditions. Breaking it down into smaller helper methods can improve readability and maintainability.Consider extracting parts of the logic into separate functions:
getLocaleFromQuery()getLocaleFromCookie()getLocaleFromHeader()normalizeLocale(locale: string): stringThis refactoring will make each method easier to understand and test.
test/i18n.test.ts (2)
81-81: Avoid redeclaring theappvariable in nesteddescribeblocksThe
appvariable is already declared in the outer scope at line 10. Redeclaring it within nesteddescribeblocks at lines 81 and 130 can lead to confusion or unintended side effects. Consider reusing the existingappvariable or using a different variable name within these blocks.Also applies to: 130-130
290-303: Improve test readability by avoiding hardcoded assertionsIn the test starting at line 290, multiple assertions check for specific cookie header patterns. Using regular expressions directly in assertions can make the test less readable and maintainable. Consider using helper functions or constants to improve clarity.
src/utils.ts (1)
1-1: Consider usingunknowninstead ofanyforisObjectparameterUsing
unknowninstead ofanyprovides better type safety and encourages explicit type checking within the function.Apply this diff if you agree:
-export function isObject(obj: any) { +export function isObject(obj: unknown) { return Object.prototype.toString.call(obj) === '[object Object]'; }src/config/config.default.ts (1)
3-15: Consider improving type safety and documentation.
- Replace type assertion with explicit typing for better type safety.
- Add inline documentation for default values.
Apply this diff:
-export default { +export default <const>{ i18n: { + // Default locale for the application defaultLocale: 'en_US', + // Array of paths to locale resource directories dirs: [], + // Query parameter name for locale queryField: 'locale', + // Cookie name for storing locale preference cookieField: 'locale', + // Cookie domain, empty for current domain only cookieDomain: '', + // Cookie expiration time (1 year) cookieMaxAge: '1y', + // Mapping of locale aliases (e.g., zh_CN -> cn) localeAlias: {}, + // Whether to persist locale in cookie writeCookie: true, + // Deprecated: use dirs instead dir: undefined, - } as I18nConfig, + } satisfies I18nConfig, };src/types.ts (3)
7-9: Translate Chinese comments to English for international collaboration.The JSDoc comments contain Chinese text which should be translated to English.
Replace:
- * 默认语言是美式英语，毕竟支持多语言，基本都是以英语为母板 - * 默认值是 `en_US` + * Default language is American English (en_US), as English typically serves as the base language for multilingual support + * Default value is `en_US`
68-69: Improve type safety for rest parameters.The
...args: any[]type is too permissive and could be more strictly typed based on the expected usage.Consider using a more specific type:
- gettext(key: string, ...args: any[]): string; - __(key: string, ...args: any[]): string; + gettext(key: string, ...args: (string | number)[]): string; + __(key: string, ...args: (string | number)[]): string;
77-78: Document optional value parameter.The
valueparameter's purpose and usage are not clear from the type signature.Add JSDoc to clarify the parameter usage:
+ /** + * Get localized text for the given key + * @param locale - The target locale + * @param key - The translation key + * @param value - Optional default value if key is not found + * @param args - Values to interpolate into the translated string + */ gettext(locale: string, key: string, value?: any, ...args: any[]): string;src/app.ts (1)
90-98: Consider using arrow function to preservethiscontext.The
gettextInContextfunction could be simplified using an arrow function.Apply this diff:
- function gettextInContext(key: string, ...args: any[]) { - const ctx = app.ctxStorage.getStore()!; - return ctx.gettext(key, ...args); - } + const gettextInContext = (key: string, ...args: any[]) => { + const ctx = app.ctxStorage.getStore(); + if (!ctx) { + throw new Error('Context store is not available'); + } + return ctx.gettext(key, ...args); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
package.json(2 hunks)src/app.ts(1 hunks)src/app/extend/application.ts(1 hunks)src/app/extend/context.ts(1 hunks)src/config/config.default.ts(1 hunks)src/locales.ts(1 hunks)src/types.ts(1 hunks)src/utils.ts(1 hunks)test/fixtures/apps/loader/a/config/locales/zh-CN.ts(1 hunks)test/fixtures/apps/loader/a/config/locales/zh-CN.yaml(1 hunks)test/fixtures/apps/loader/a/config/locales/zh_CN.properties(1 hunks)test/i18n.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- test/fixtures/apps/loader/a/config/locales/zh_CN.properties
- test/fixtures/apps/loader/a/config/locales/zh-CN.yaml
🔇 Additional comments (12)
package.json (7)
2-6: LGTM! Package identity changes follow best practices.The package rename to
@eggjs/i18nand public access configuration are correctly implemented.
8-13: LGTM! Dual module format support is well configured.The exports configuration correctly supports both ESM and CommonJS, with proper TypeScript source exposure.
18-18: LGTM! Repository URLs are consistently updated.All repository-related URLs are correctly updated to reflect the new repository name.
Also applies to: 27-27, 29-29
60-69: LGTM! Comprehensive build and test scripts.The scripts configuration provides a complete development workflow with proper type checking, linting, testing, and build steps.
70-96: LGTM! Excellent dual-format module configuration.The module configuration follows Node.js best practices for dual CJS/ESM packages:
- Proper type declarations for both formats
- Clear entry points for different module systems
- Correct package exports configuration
34-59: Verify removal of debug package and consider pinning dependency versions.
- The
debugpackage has been removed. Verify that all debug logging has been migrated to a new solution.- Consider pinning dependency versions more strictly for better reproducibility.
Consider updating the dependencies to use exact versions or tighter version ranges for better reproducibility:
- "@eggjs/core": "^6.2.13", + "@eggjs/core": "~6.2.13",✅ Verification successful
Debug package removal verified - successfully migrated to Node.js built-in debuglog
The codebase has been properly migrated from the
debugpackage to Node's built-indebuglogutility from thenode:utilmodule, which is a good practice for reducing external dependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining debug package usage rg "require\(['\"']debug['\"]" || rg "import.*debug.*from"Length of output: 308
31-31: Verify breaking change documentation.The Node.js version requirement has been significantly increased to >= 18.19.0. This is a breaking change that needs to be prominently documented in the changelog and release notes.
src/app/extend/application.ts (2)
69-79: 🛠️ Refactor suggestionCorrect Variable Name to Follow Naming Conventions
The constant
Object_INDEX_REshould be renamed to follow naming conventions. Since it's a constant, consider renaming it toOBJECT_INDEX_REfor clarity and consistency.Apply this diff to rename the constant:
- const Object_INDEX_RE = /\{(.+?)\}/g; + const OBJECT_INDEX_RE = /\{(.+?)\}/g;And update its usage:
- return text.replace(Object_INDEX_RE, (original, matched) => { + return text.replace(OBJECT_INDEX_RE, (original, matched) => {Likely invalid or redundant comment.
9-10: 🛠️ Refactor suggestionEnsure Proper Type Augmentation for Application Class
The
I18nApplicationclass extendsEggCoreand adds new properties and methods. To ensure TypeScript recognizes these extensions, consider using module augmentation to properly declare the new members.Apply this diff to augment the
EggApplicationinterface:declare module '@eggjs/core' { interface EggApplication { [I18N_RESOURCES]: Record<string, Record<string, string>>; isSupportLocale(locale: string): boolean; gettext(locale: string, key: string, value?: any, ...args: any[]): string; __(locale: string, key: string, value?: any, ...args: any[]): string; } }Ensure that this augmentation is placed in a
.d.tsfile and is correctly referenced.✅ Verification successful
Type Augmentation Suggestion is Correct and Beneficial
The suggested type augmentation is accurate and should be added to
src/typings/index.d.ts, which is already set up for declaration merging. This will properly type the extensions to theEggApplicationinterface and improve TypeScript's ability to recognize these members.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find all TypeScript declaration files echo "=== TypeScript Declaration Files ===" fd -e d.ts # Look for existing type augmentations echo -e "\n=== Existing Type Augmentations ===" rg "declare module.*EggApplication" -A 5 # Examine I18nApplication implementation echo -e "\n=== I18nApplication Implementation ===" ast-grep --pattern 'class I18nApplication extends EggCore { $$$ }'Length of output: 3778
Script:
#!/bin/bash # Check contents of the typings file echo "=== Contents of index.d.ts ===" cat src/typings/index.d.ts # Also check if EggCore is imported/used elsewhere echo -e "\n=== EggCore Usage ===" rg "import.*EggCore" -A 1Length of output: 537
src/app/extend/context.ts (2)
183-189: Specify SameSite Attribute for Locale CookieFor enhanced security against CSRF attacks, specify the
sameSiteattribute for the locale cookie. Consider setting it to'lax'or'strict'based on the application's requirements.Apply this diff to add the
sameSiteattribute:const cookieOptions = { // make sure browser javascript can read the cookie httpOnly: true, maxAge: cookieMaxAge as number, signed: false, domain: cookieDomain, overwrite: true, + sameSite: 'lax', };Decide on the appropriate
sameSitevalue ('lax','strict', or'none') based on the application's needs.✅ Verification successful
SameSite=Lax is Appropriate for Locale Cookie
The locale cookie is used for UI customization (storing language preferences) and doesn't contain sensitive data. Setting
sameSite: 'lax'provides a good balance between security and functionality while matching modern browser defaults.const cookieOptions = { // make sure browser javascript can read the cookie httpOnly: false, maxAge: cookieMaxAge as number, signed: false, domain: cookieDomain, overwrite: true, + sameSite: 'lax', };🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Check the complete context.ts file for better understanding cat src/app/extend/context.ts # Search for CORS-related configurations rg -l "cors" --type ts # Search for other cookie configurations rg "cookie" --type ts -C 2Length of output: 11858
155-157: Check for Header Sent Before Setting CookieBefore setting the locale cookie, the code checks
!this.headerSent. However,headerSentmight not always reflect the correct state. Consider usingthis.res.headersSentfor accuracy.Ensure that
this.headerSentcorrectly indicates if the headers have been sent. If not, update the condition:- if (writeCookie && cookieLocale !== locale && !this.headerSent) { + if (writeCookie && cookieLocale !== locale && !this.res.headersSent) {test/fixtures/apps/loader/a/config/locales/zh-CN.ts (1)
1-4: Locale configuration exported correctlyThe locale data for 'zh-CN' is properly exported with the expected keys and values. This ensures correct localization for the Chinese locale.
[skip ci] ## [3.0.0](v2.1.1...v3.0.0) (2025-01-11) ### ⚠ BREAKING CHANGES * drop Node.js < 18.19.0 support part of eggjs/egg#3644 eggjs/egg#5257 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit Based on the comprehensive changes, here are the updated release notes: - **New Features** - Migrated package to TypeScript with improved type safety. - Enhanced internationalization (i18n) support with more flexible configuration. - Added comprehensive GitHub Actions workflows for CI/CD. - **Improvements** - Updated Node.js compatibility to version 18.19.0+. - Modernized module system with ES module support. - Refined configuration and localization mechanisms. - **Breaking Changes** - Package renamed from `egg-i18n` to `@eggjs/i18n`. - Switched from CommonJS to ES module syntax. - Removed legacy configuration files and testing infrastructure. - **Chores** - Cleaned up and simplified project structure. - Updated dependencies and development tooling. - Improved documentation and README. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ### Features * support cjs and esm both by tshy ([#14](#14)) ([ccc8eaa](ccc8eaa))
BREAKING CHANGE: drop Node.js < 18.19.0 support
part of eggjs/egg#3644
eggjs/egg#5257
Summary by CodeRabbit
Based on the comprehensive changes, here are the updated release notes:
New Features
Improvements
Breaking Changes
egg-i18nto@eggjs/i18n.Chores