feat: add check-locales workflow #3950
Conversation
Signed-off-by: Ansh Goyal <[email protected]>
anshgoyalevil
requested review from
Mayaleeeee,
akshatnema,
asyncapi-bot-eve,
derberg and
sambhavgupta0705
as code owners
WalkthroughThe pull request introduces locale validation into the project. It adds a new GitHub Actions job that executes locale checks during Node.js pull requests. The workflow runs a new script that verifies JSON locale files for consistent keys across languages. If inconsistencies are found, the workflow posts a comment on the pull request; if not, it removes any existing comment. Additionally, a new script in Changes
Sequence Diagram(s)sequenceDiagram
participant PR as "Pull Request"
participant GA as "GitHub Actions"
participant LS as "Locale Script"
participant CM as "Comment Service"
PR->>GA: Trigger Node.js PR workflow
GA->>LS: Execute `npm run test:locales`
LS-->>GA: Return validation result (errors or success)
alt Locale issues detected
GA->>CM: Post comment detailing locale issues
else No locale issues
GA->>CM: Delete existing locale issue comment (if any)
end
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3950 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 22 +1
Lines 671 742 +71
Branches 115 123 +8
=========================================
+ Hits 671 742 +71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (6)
tests/check-locales.test.js (2)
31-39: Consolidate the doublepathmocking
You’re re-mockingpathhere, even though it was already mocked in lines 11–18. This could lead to confusion or unexpected behavior. Consider moving allpathmocking logic into a single place or removing the duplicate mock.- jest.mock('path', () => { - const originalPath = jest.requireActual('path'); - return { - ...originalPath, - dirname: jest.fn().mockReturnValue('/mock/dir'), - resolve: jest.fn().mockReturnValue('/mock/path/locales'), - join: jest.fn().mockImplementation((...args) => args.join('/')) - }; - }); + // Remove or refactor this block if the earlier path mock suffices
42-47: Restore original process.exit in afterEach
While spying onprocess.exitis helpful in tests, consider restoring it in anafterEachblock to avoid side effects on subsequent tests.beforeEach(() => { jest.clearAllMocks(); }); + afterEach(() => { + mockExit.mockRestore(); + });🧰 Tools
🪛 ESLint
[error] 43-43: Delete
··(prettier/prettier)
.github/workflows/if-nodejs-pr-testing.yml (1)
129-129: Remove trailing spaces in YAML
Static analysis flagged trailing spaces in lines 129, 146, 148, and 152. Consider removing them for cleaner formatting.set -e - + # If the command failed, extract and format the error message ... ### Locale Check Results - + We found issues with locale keys: - + ``` - + Please make sure all locale files have the same translation keys.Also applies to: 146-146, 148-148, 152-152
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 129-129: trailing spaces
(trailing-spaces)
scripts/utils/check-locales.ts (3)
1-2: Unify multiplepathimports
You import bothpathanddirnameseparately from 'path'. Consider combining them to reduce duplication.-import fs from 'fs'; -import path from 'path'; +import fs from 'fs'; +import path, { dirname } from 'path';🧰 Tools
🪛 ESLint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 2-2: 'path' imported multiple times.
(import/no-duplicates)
13-19: Remove extra trailing comma and reduce multiline
Static analysis indicates a stylistic preference regardingpath.resolve. You can simplify to a single line without trailing commas.-const localesDir = path.resolve( - currentDirPath, - '..', - '..', - 'public', - 'locales', -); +const localesDir = path.resolve(currentDirPath, '..', '..', 'public', 'locales');🧰 Tools
🪛 ESLint
[error] 13-19: Replace
⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎withcurrentDirPath,·'..',·'..',·'public',·'locales'(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
121-121: Consider alternatives tocontinue
Thecontinuestatement is sometimes discouraged due to readability concerns. You could refactor this logic with anifcondition to reduce jumping to the next iteration.🧰 Tools
🪛 ESLint
[error] 121-121: Expected blank line before this statement.
(padding-line-between-statements)
[error] 121-121: Unexpected use of continue statement.
(no-continue)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/if-nodejs-pr-testing.yml(1 hunks)package.json(1 hunks)scripts/utils/check-locales.ts(1 hunks)tests/check-locales.test.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/check-locales.test.js (1)
scripts/utils/check-locales.ts (3)
extractKeys(170-170)readJSONFilesInDir(170-170)validateLocales(170-170)
🪛 ESLint
tests/check-locales.test.js
[error] 1-1: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
[error] 43-43: Delete ··
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 64-64: Delete ····
(prettier/prettier)
[error] 66-66: Delete ····
(prettier/prettier)
[error] 69-69: Delete ,
(prettier/prettier)
[error] 74-74: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 78-78: Delete ····
(prettier/prettier)
[error] 82-82: Delete ····
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 87-87: Delete ····
(prettier/prettier)
[error] 89-89: Delete ····
(prettier/prettier)
[error] 95-95: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 97-97: Delete ····
(prettier/prettier)
[error] 99-99: Delete ····
(prettier/prettier)
[error] 101-101: Delete ····
(prettier/prettier)
[error] 105-105: Delete ····
(prettier/prettier)
[error] 108-108: Delete ····
(prettier/prettier)
[error] 112-114: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 116-116: Delete ····
(prettier/prettier)
[error] 118-118: Delete ····
(prettier/prettier)
[error] 119-121: Replace ⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎···· with "❌·Language·'de'·is·missing·these·keys:·key2"
(prettier/prettier)
[error] 127-127: Delete ····
(prettier/prettier)
[error] 131-131: Delete ····
(prettier/prettier)
[error] 133-133: Delete ····
(prettier/prettier)
[error] 134-137: Replace ⏎······'Error·validating·locales:',⏎······testError⏎···· with 'Error·validating·locales:',·testError
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 145-145: Delete ····
(prettier/prettier)
[error] 146-149: Replace ⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎···· with 'No·language·directories·found·in',·'/mock/path/locales'
(prettier/prettier)
[error] 156-156: Delete ····
(prettier/prettier)
[error] 161-161: Delete ····
(prettier/prettier)
[error] 163-163: Delete ····
(prettier/prettier)
[error] 164-167: Replace ⏎······expect.stringContaining(Error·reading),⏎······expect.any(String)⏎···· with expect.stringContaining(Error·reading),·expect.any(String)
(prettier/prettier)
[error] 173-173: Delete ····
(prettier/prettier)
[error] 177-177: Delete ····
(prettier/prettier)
[error] 179-179: Delete ····
(prettier/prettier)
[error] 180-183: Replace ⏎······'Error·reading·directory·/mock/dir:',⏎······'Directory·read·error'⏎···· with 'Error·reading·directory·/mock/dir:',·'Directory·read·error'
(prettier/prettier)
[error] 184-184: Delete ····
(prettier/prettier)
[error] 187-187: Delete ··
(prettier/prettier)
[error] 190-190: Expected an error object to be thrown.
(no-throw-literal)
[error] 192-192: Delete ····
(prettier/prettier)
[error] 194-194: Delete ····
(prettier/prettier)
[error] 201-201: Delete ····
(prettier/prettier)
[error] 204-204: Delete ····
(prettier/prettier)
[error] 207-207: Expected an error object to be thrown.
(no-throw-literal)
[error] 209-209: Delete ····
(prettier/prettier)
[error] 217-217: Delete ··
(prettier/prettier)
[error] 220-220: Delete ····
(prettier/prettier)
[error] 222-222: Delete ····
(prettier/prettier)
[error] 226-226: Delete ····
(prettier/prettier)
[error] 229-229: Delete ····
(prettier/prettier)
[error] 231-231: Delete ····
(prettier/prettier)
[error] 233-233: Delete ····
(prettier/prettier)
[error] 236-236: Insert ⏎
(prettier/prettier)
scripts/utils/check-locales.ts
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 2-2: 'path' imported multiple times.
(import/no-duplicates)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 6-6: path import should occur before import of ../utils/logger
(import/order)
[error] 6-6: 'path' imported multiple times.
(import/no-duplicates)
[error] 13-19: Replace ⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎ with currentDirPath,·'..',·'..',·'public',·'locales'
(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 28-41: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-31: Replace ⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎·· with obj:·Record<string,·any>,·prefix:·string·=·''
(prettier/prettier)
[error] 29-29: 'obj' is already declared in the upper scope on line 27 column 22.
(@typescript-eslint/no-shadow)
[error] 30-30: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 35-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Expected blank line before this statement.
(padding-line-between-statements)
[error] 39-39: Delete ,
(prettier/prettier)
[error] 39-39: Unexpected trailing comma.
(comma-dangle)
[error] 42-42: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-69: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Expected blank line after variable declarations.
(newline-after-var)
[error] 59-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-61: Expected blank line before this statement.
(padding-line-between-statements)
[error] 66-66: Expected blank line before this statement.
(padding-line-between-statements)
[error] 68-68: Delete ,
(prettier/prettier)
[error] 68-68: Unexpected trailing comma.
(comma-dangle)
[error] 74-74: Expected blank line before this statement.
(padding-line-between-statements)
[error] 92-92: Expected blank line before this statement.
(padding-line-between-statements)
[error] 101-101: Expected blank line after variable declarations.
(newline-after-var)
[error] 102-102: Expected blank line before this statement.
(padding-line-between-statements)
[error] 104-107: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 106-106: Unexpected trailing comma.
(comma-dangle)
[error] 107-107: Unexpected trailing comma.
(comma-dangle)
[error] 111-113: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 112-112: Unexpected trailing comma.
(comma-dangle)
[error] 118-120: Expected blank line after variable declarations.
(newline-after-var)
[error] 118-120: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 119-119: Unexpected trailing comma.
(comma-dangle)
[error] 121-121: Expected blank line before this statement.
(padding-line-between-statements)
[error] 121-121: Unexpected use of continue statement.
(no-continue)
[error] 123-125: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 124-124: Unexpected trailing comma.
(comma-dangle)
[error] 130-132: Expected blank line after variable declarations.
(newline-after-var)
[error] 130-132: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 131-131: Unexpected trailing comma.
(comma-dangle)
[error] 133-133: Expected blank line before this statement.
(padding-line-between-statements)
[error] 134-134: Delete ,
(prettier/prettier)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 137-139: Replace ⏎········([_,·missing])·=>·missing.length·>·0,⏎······ with ([_,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 138-138: '_' is defined but never used.
(no-unused-vars)
[error] 138-138: Unexpected trailing comma.
(comma-dangle)
[error] 143-148: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 144-146: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 145-145: Unexpected trailing comma.
(comma-dangle)
[error] 152-161: Unexpected negated condition.
(no-negated-condition)
[error] 153-155: Replace ⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······ with '✅·All·locale·files·have·the·same·keys·across·all·languages!'
(prettier/prettier)
[error] 154-154: Unexpected trailing comma.
(comma-dangle)
[error] 157-159: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 158-158: Unexpected trailing comma.
(comma-dangle)
[error] 170-170: Run autofix to sort these exports!
(simple-import-sort/exports)
🪛 YAMLlint (1.35.1)
.github/workflows/if-nodejs-pr-testing.yml
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 146-146: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
package.json (1)
29-29: Good addition for locale validation
This new script aligns perfectly with your new locale checking logic.tests/check-locales.test.js (1)
1-236: Comprehensive test coverage
Overall, this suite provides thorough coverage for locale validation scenarios, including error handling and edge cases. Great job!🧰 Tools
🪛 ESLint
[error] 1-1: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
[error] 43-43: Delete
··(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 64-64: Delete
····(prettier/prettier)
[error] 66-66: Delete
····(prettier/prettier)
[error] 69-69: Delete
,(prettier/prettier)
[error] 74-74: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 76-76: Delete
····(prettier/prettier)
[error] 78-78: Delete
····(prettier/prettier)
[error] 82-82: Delete
····(prettier/prettier)
[error] 85-85: Delete
····(prettier/prettier)
[error] 87-87: Delete
····(prettier/prettier)
[error] 89-89: Delete
····(prettier/prettier)
[error] 95-95: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 97-97: Delete
····(prettier/prettier)
[error] 99-99: Delete
····(prettier/prettier)
[error] 101-101: Delete
····(prettier/prettier)
[error] 105-105: Delete
····(prettier/prettier)
[error] 108-108: Delete
····(prettier/prettier)
[error] 112-114: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 116-116: Delete
····(prettier/prettier)
[error] 118-118: Delete
····(prettier/prettier)
[error] 119-121: Replace
⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎····with"❌·Language·'de'·is·missing·these·keys:·key2"(prettier/prettier)
[error] 127-127: Delete
····(prettier/prettier)
[error] 131-131: Delete
····(prettier/prettier)
[error] 133-133: Delete
····(prettier/prettier)
[error] 134-137: Replace
⏎······'Error·validating·locales:',⏎······testError⏎····with'Error·validating·locales:',·testError(prettier/prettier)
[error] 143-143: Delete
····(prettier/prettier)
[error] 145-145: Delete
····(prettier/prettier)
[error] 146-149: Replace
⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎····with'No·language·directories·found·in',·'/mock/path/locales'(prettier/prettier)
[error] 156-156: Delete
····(prettier/prettier)
[error] 161-161: Delete
····(prettier/prettier)
[error] 163-163: Delete
····(prettier/prettier)
[error] 164-167: Replace
⏎······expect.stringContaining(Error·reading),⏎······expect.any(String)⏎····withexpect.stringContaining(Error·reading),·expect.any(String)(prettier/prettier)
[error] 173-173: Delete
····(prettier/prettier)
[error] 177-177: Delete
····(prettier/prettier)
[error] 179-179: Delete
····(prettier/prettier)
[error] 180-183: Replace
⏎······'Error·reading·directory·/mock/dir:',⏎······'Directory·read·error'⏎····with'Error·reading·directory·/mock/dir:',·'Directory·read·error'(prettier/prettier)
[error] 184-184: Delete
····(prettier/prettier)
[error] 187-187: Delete
··(prettier/prettier)
[error] 190-190: Expected an error object to be thrown.
(no-throw-literal)
[error] 192-192: Delete
····(prettier/prettier)
[error] 194-194: Delete
····(prettier/prettier)
[error] 201-201: Delete
····(prettier/prettier)
[error] 204-204: Delete
····(prettier/prettier)
[error] 207-207: Expected an error object to be thrown.
(no-throw-literal)
[error] 209-209: Delete
····(prettier/prettier)
[error] 217-217: Delete
··(prettier/prettier)
[error] 220-220: Delete
····(prettier/prettier)
[error] 222-222: Delete
····(prettier/prettier)
[error] 226-226: Delete
····(prettier/prettier)
[error] 229-229: Delete
····(prettier/prettier)
[error] 231-231: Delete
····(prettier/prettier)
[error] 233-233: Delete
····(prettier/prettier)
[error] 236-236: Insert
⏎(prettier/prettier)
.github/workflows/if-nodejs-pr-testing.yml (1)
119-163: Workflow logic looks good
The newly introduced steps correctly run and capture thetest:localesoutput, then provide helpful feedback on PRs.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 146-146: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
[error] 152-152: trailing spaces
(trailing-spaces)
scripts/utils/check-locales.ts (1)
1-176: Locale validation logic is robust
The code thoroughly checks for missing keys, logs descriptive errors, and exits with the correct status code. Great job!🧰 Tools
🪛 ESLint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 2-2: 'path' imported multiple times.
(import/no-duplicates)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 6-6:
pathimport should occur before import of../utils/logger(import/order)
[error] 6-6: 'path' imported multiple times.
(import/no-duplicates)
[error] 13-19: Replace
⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎withcurrentDirPath,·'..',·'..',·'public',·'locales'(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 28-41: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-31: Replace
⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎··withobj:·Record<string,·any>,·prefix:·string·=·''(prettier/prettier)
[error] 29-29: 'obj' is already declared in the upper scope on line 27 column 22.
(@typescript-eslint/no-shadow)
[error] 30-30: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 35-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Expected blank line before this statement.
(padding-line-between-statements)
[error] 39-39: Delete
,(prettier/prettier)
[error] 39-39: Unexpected trailing comma.
(comma-dangle)
[error] 42-42: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-69: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Expected blank line after variable declarations.
(newline-after-var)
[error] 59-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-61: Expected blank line before this statement.
(padding-line-between-statements)
[error] 66-66: Expected blank line before this statement.
(padding-line-between-statements)
[error] 68-68: Delete
,(prettier/prettier)
[error] 68-68: Unexpected trailing comma.
(comma-dangle)
[error] 74-74: Expected blank line before this statement.
(padding-line-between-statements)
[error] 92-92: Expected blank line before this statement.
(padding-line-between-statements)
[error] 101-101: Expected blank line after variable declarations.
(newline-after-var)
[error] 102-102: Expected blank line before this statement.
(padding-line-between-statements)
[error] 104-107: Replace
⏎··········file,⏎··········extractKeys(content),⏎········]),withfile,·extractKeys(content)])(prettier/prettier)
[error] 106-106: Unexpected trailing comma.
(comma-dangle)
[error] 107-107: Unexpected trailing comma.
(comma-dangle)
[error] 111-113: Replace
⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎····withflatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))(prettier/prettier)
[error] 112-112: Unexpected trailing comma.
(comma-dangle)
[error] 118-120: Expected blank line after variable declarations.
(newline-after-var)
[error] 118-120: Replace
⏎········(lang)·=>·languageFiles[lang][file],⏎······with(lang)·=>·languageFiles[lang][file](prettier/prettier)
[error] 119-119: Unexpected trailing comma.
(comma-dangle)
[error] 121-121: Expected blank line before this statement.
(padding-line-between-statements)
[error] 121-121: Unexpected use of continue statement.
(no-continue)
[error] 123-125: Replace
⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······withflatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))(prettier/prettier)
[error] 124-124: Unexpected trailing comma.
(comma-dangle)
[error] 130-132: Expected blank line after variable declarations.
(newline-after-var)
[error] 130-132: Replace
⏎············(key)·=>·!langKeysSet.has(key),⏎··········with(key)·=>·!langKeysSet.has(key)(prettier/prettier)
[error] 131-131: Unexpected trailing comma.
(comma-dangle)
[error] 133-133: Expected blank line before this statement.
(padding-line-between-statements)
[error] 134-134: Delete
,(prettier/prettier)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 137-139: Replace
⏎········([_,·missing])·=>·missing.length·>·0,⏎······with([_,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 138-138: '_' is defined but never used.
(no-unused-vars)
[error] 138-138: Unexpected trailing comma.
(comma-dangle)
[error] 143-148: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 144-146: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 145-145: Unexpected trailing comma.
(comma-dangle)
[error] 152-161: Unexpected negated condition.
(no-negated-condition)
[error] 153-155: Replace
⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······with'✅·All·locale·files·have·the·same·keys·across·all·languages!'(prettier/prettier)
[error] 154-154: Unexpected trailing comma.
(comma-dangle)
[error] 157-159: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 158-158: Unexpected trailing comma.
(comma-dangle)
[error] 170-170: Run autofix to sort these exports!
(simple-import-sort/exports)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/check-locales.test.js (5)
1-40: Consolidate the duplicate path module mockThere's an unnecessary duplication of the
pathmodule mock on lines 11-18 and 31-39. While the second mock addsdirname, it would be better to consolidate these into a single mock to improve readability and maintainability.- jest.mock('path', () => { - const originalPath = jest.requireActual('path'); - return { - ...originalPath, - resolve: jest.fn().mockReturnValue('/mock/path/locales'), - join: jest.fn().mockImplementation((...args) => args.join('/')) - }; - }); - - // ... other code - - jest.mock('path', () => { - const originalPath = jest.requireActual('path'); - return { - ...originalPath, - dirname: jest.fn().mockReturnValue('/mock/dir'), - resolve: jest.fn().mockReturnValue('/mock/path/locales'), - join: jest.fn().mockImplementation((...args) => args.join('/')) - }; - }); + jest.mock('path', () => { + const originalPath = jest.requireActual('path'); + return { + ...originalPath, + dirname: jest.fn().mockReturnValue('/mock/dir'), + resolve: jest.fn().mockReturnValue('/mock/path/locales'), + join: jest.fn().mockImplementation((...args) => args.join('/')) + }; + });🧰 Tools
🪛 ESLint
[error] 1-1: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
28-29: Add .ts file extensions to importsAdd the file extensions to improve module resolution and conform to ESLint rules.
- const { validateLocales, extractKeys, readJSONFilesInDir } = require('../scripts/utils/check-locales'); - const { logger } = require('../scripts/utils/logger'); + const { validateLocales, extractKeys, readJSONFilesInDir } = require('../scripts/utils/check-locales.ts'); + const { logger } = require('../scripts/utils/logger.ts');🧰 Tools
🪛 ESLint
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
73-92: Remove unused variableThe variable
localesDiris defined but never used in the test. Consider removing it for cleaner code.- const localesDir = '/mock/path/locales';🧰 Tools
🪛 ESLint
[error] 74-74: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 76-76: Delete
····(prettier/prettier)
[error] 78-78: Delete
····(prettier/prettier)
[error] 82-82: Delete
····(prettier/prettier)
[error] 85-85: Delete
····(prettier/prettier)
[error] 87-87: Delete
····(prettier/prettier)
[error] 89-89: Delete
····(prettier/prettier)
94-123: Remove unnecessary else after returnOn lines 112-114, remove the unnecessary
elseblock after the return statement to simplify the code.if (filePath.includes('en')) { return '{"key1":"value1","key2":"value2"}'; - } else { - return '{"key1":"value1"}'; } + return '{"key1":"value1"}';Also, remove the unused
localesDirvariable similar to the previous test.🧰 Tools
🪛 ESLint
[error] 95-95: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 97-97: Delete
····(prettier/prettier)
[error] 99-99: Delete
····(prettier/prettier)
[error] 101-101: Delete
····(prettier/prettier)
[error] 105-105: Delete
····(prettier/prettier)
[error] 108-108: Delete
····(prettier/prettier)
[error] 112-114: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 116-116: Delete
····(prettier/prettier)
[error] 118-118: Delete
····(prettier/prettier)
[error] 119-121: Replace
⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎····with"❌·Language·'de'·is·missing·these·keys:·key2"(prettier/prettier)
152-169: Simplify error creationThe error creation at line 159 is unnecessarily complex. The
new Error(testError)is creating an Error with an Error object as its message. Simplify it as follows:- throw new Error(testError); + throw testError;🧰 Tools
🪛 ESLint
[error] 156-156: Delete
····(prettier/prettier)
[error] 161-161: Delete
····(prettier/prettier)
[error] 163-163: Delete
····(prettier/prettier)
[error] 164-167: Replace
⏎······expect.stringContaining(Error·reading),⏎······expect.any(String)⏎····withexpect.stringContaining(Error·reading),·expect.any(String)(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/check-locales.test.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/check-locales.test.js (1)
scripts/utils/check-locales.ts (3)
extractKeys(170-170)readJSONFilesInDir(170-170)validateLocales(170-170)
🪛 ESLint
tests/check-locales.test.js
[error] 1-1: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
[error] 43-43: Delete ··
(prettier/prettier)
[error] 57-57: Delete ····
(prettier/prettier)
[error] 64-64: Delete ····
(prettier/prettier)
[error] 66-66: Delete ····
(prettier/prettier)
[error] 69-69: Delete ,
(prettier/prettier)
[error] 74-74: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 76-76: Delete ····
(prettier/prettier)
[error] 78-78: Delete ····
(prettier/prettier)
[error] 82-82: Delete ····
(prettier/prettier)
[error] 85-85: Delete ····
(prettier/prettier)
[error] 87-87: Delete ····
(prettier/prettier)
[error] 89-89: Delete ····
(prettier/prettier)
[error] 95-95: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 97-97: Delete ····
(prettier/prettier)
[error] 99-99: Delete ····
(prettier/prettier)
[error] 101-101: Delete ····
(prettier/prettier)
[error] 105-105: Delete ····
(prettier/prettier)
[error] 108-108: Delete ····
(prettier/prettier)
[error] 112-114: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 116-116: Delete ····
(prettier/prettier)
[error] 118-118: Delete ····
(prettier/prettier)
[error] 119-121: Replace ⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎···· with "❌·Language·'de'·is·missing·these·keys:·key2"
(prettier/prettier)
[error] 127-127: Delete ····
(prettier/prettier)
[error] 131-131: Delete ····
(prettier/prettier)
[error] 133-133: Delete ····
(prettier/prettier)
[error] 134-137: Replace ⏎······'Error·validating·locales:',⏎······testError⏎···· with 'Error·validating·locales:',·testError
(prettier/prettier)
[error] 143-143: Delete ····
(prettier/prettier)
[error] 145-145: Delete ····
(prettier/prettier)
[error] 146-149: Replace ⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎···· with 'No·language·directories·found·in',·'/mock/path/locales'
(prettier/prettier)
[error] 156-156: Delete ····
(prettier/prettier)
[error] 161-161: Delete ····
(prettier/prettier)
[error] 163-163: Delete ····
(prettier/prettier)
[error] 164-167: Replace ⏎······expect.stringContaining(Error·reading),⏎······expect.any(String)⏎···· with expect.stringContaining(Error·reading),·expect.any(String)
(prettier/prettier)
[error] 173-173: Delete ····
(prettier/prettier)
[error] 177-177: Delete ····
(prettier/prettier)
[error] 179-179: Delete ····
(prettier/prettier)
[error] 180-183: Replace ⏎······'Error·reading·directory·/mock/dir:',⏎······'Directory·read·error'⏎···· with 'Error·reading·directory·/mock/dir:',·'Directory·read·error'
(prettier/prettier)
[error] 184-184: Delete ····
(prettier/prettier)
[error] 187-187: Delete ··
(prettier/prettier)
[error] 192-192: Delete ····
(prettier/prettier)
[error] 194-194: Delete ····
(prettier/prettier)
[error] 201-201: Delete ····
(prettier/prettier)
[error] 204-204: Delete ····
(prettier/prettier)
[error] 209-209: Delete ····
(prettier/prettier)
[error] 217-217: Delete ··
(prettier/prettier)
[error] 220-220: Delete ····
(prettier/prettier)
[error] 222-222: Delete ····
(prettier/prettier)
[error] 226-226: Delete ····
(prettier/prettier)
[error] 229-229: Delete ····
(prettier/prettier)
[error] 231-231: Delete ····
(prettier/prettier)
[error] 233-233: Delete ····
(prettier/prettier)
[error] 236-236: Insert ⏎
(prettier/prettier)
🔇 Additional comments (9)
tests/check-locales.test.js (9)
48-52: Looks good!The test for the
extractKeysfunction is well-implemented and verifies that keys are properly extracted from a nested JSON object.
54-71: Looks good!The test for
readJSONFilesInDirfunction properly verifies JSON file reading, skipping non-JSON files, and returning the expected output.🧰 Tools
🪛 ESLint
[error] 57-57: Delete
····(prettier/prettier)
[error] 64-64: Delete
····(prettier/prettier)
[error] 66-66: Delete
····(prettier/prettier)
[error] 69-69: Delete
,(prettier/prettier)
125-139: Looks good!This test properly verifies error handling during validation, ensuring that errors are caught, logged, and the process exits with the correct status code.
🧰 Tools
🪛 ESLint
[error] 127-127: Delete
····(prettier/prettier)
[error] 131-131: Delete
····(prettier/prettier)
[error] 133-133: Delete
····(prettier/prettier)
[error] 134-137: Replace
⏎······'Error·validating·locales:',⏎······testError⏎····with'Error·validating·locales:',·testError(prettier/prettier)
141-150: Looks good!This test effectively verifies the behavior when no language directories are found, ensuring proper error logging.
🧰 Tools
🪛 ESLint
[error] 143-143: Delete
····(prettier/prettier)
[error] 145-145: Delete
····(prettier/prettier)
[error] 146-149: Replace
⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎····with'No·language·directories·found·in',·'/mock/path/locales'(prettier/prettier)
170-186: Looks good!This test properly verifies the behavior when directory reading fails, ensuring that the error is properly logged and an empty object is returned.
🧰 Tools
🪛 ESLint
[error] 173-173: Delete
····(prettier/prettier)
[error] 177-177: Delete
····(prettier/prettier)
[error] 179-179: Delete
····(prettier/prettier)
[error] 180-183: Replace
⏎······'Error·reading·directory·/mock/dir:',⏎······'Directory·read·error'⏎····with'Error·reading·directory·/mock/dir:',·'Directory·read·error'(prettier/prettier)
[error] 184-184: Delete
····(prettier/prettier)
188-196: Throw a string instead of an Error objectThe test name suggests it's testing handling of non-Error exceptions, but line 190 throws an Error object. This doesn't match the test description.
- throw new Error('String error'); + throw 'String error';However, this change would reintroduce the ESLint error that was fixed in previous commits. Consider renaming the test to "should handle Error exceptions during validation" instead.
🧰 Tools
🪛 ESLint
[error] 192-192: Delete
····(prettier/prettier)
[error] 194-194: Delete
····(prettier/prettier)
198-216: Looks good!This test properly verifies the behavior when JSON parsing fails, ensuring that an empty object is returned for the file.
🧰 Tools
🪛 ESLint
[error] 201-201: Delete
····(prettier/prettier)
[error] 204-204: Delete
····(prettier/prettier)
[error] 209-209: Delete
····(prettier/prettier)
218-235: Looks good!This test effectively verifies the behavior when a language has no corresponding file, ensuring that validation still succeeds in this case.
🧰 Tools
🪛 ESLint
[error] 220-220: Delete
····(prettier/prettier)
[error] 222-222: Delete
····(prettier/prettier)
[error] 226-226: Delete
····(prettier/prettier)
[error] 229-229: Delete
····(prettier/prettier)
[error] 231-231: Delete
····(prettier/prettier)
[error] 233-233: Delete
····(prettier/prettier)
1-236: Overall assessment of test coverageThe test suite provides comprehensive coverage for the locale validation functionality, testing:
- Key extraction from JSON objects
- JSON file reading from directories
- Successful locale validation
- Detection of missing keys across languages
- Various error handling scenarios
This thorough test coverage ensures the robustness of the locale validation functionality, which aligns well with the PR objective of ensuring consistency of locale keys across different language folders.
🧰 Tools
🪛 ESLint
[error] 1-1: 'path' is assigned a value but never used.
(no-unused-vars)
[error] 28-28: Missing file extension "ts" for "../scripts/utils/check-locales"
(import/extensions)
[error] 29-29: Missing file extension "ts" for "../scripts/utils/logger"
(import/extensions)
[error] 43-43: Delete
··(prettier/prettier)
[error] 57-57: Delete
····(prettier/prettier)
[error] 64-64: Delete
····(prettier/prettier)
[error] 66-66: Delete
····(prettier/prettier)
[error] 69-69: Delete
,(prettier/prettier)
[error] 74-74: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 76-76: Delete
····(prettier/prettier)
[error] 78-78: Delete
····(prettier/prettier)
[error] 82-82: Delete
····(prettier/prettier)
[error] 85-85: Delete
····(prettier/prettier)
[error] 87-87: Delete
····(prettier/prettier)
[error] 89-89: Delete
····(prettier/prettier)
[error] 95-95: 'localesDir' is assigned a value but never used.
(no-unused-vars)
[error] 97-97: Delete
····(prettier/prettier)
[error] 99-99: Delete
····(prettier/prettier)
[error] 101-101: Delete
····(prettier/prettier)
[error] 105-105: Delete
····(prettier/prettier)
[error] 108-108: Delete
····(prettier/prettier)
[error] 112-114: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 116-116: Delete
····(prettier/prettier)
[error] 118-118: Delete
····(prettier/prettier)
[error] 119-121: Replace
⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎····with"❌·Language·'de'·is·missing·these·keys:·key2"(prettier/prettier)
[error] 127-127: Delete
····(prettier/prettier)
[error] 131-131: Delete
····(prettier/prettier)
[error] 133-133: Delete
····(prettier/prettier)
[error] 134-137: Replace
⏎······'Error·validating·locales:',⏎······testError⏎····with'Error·validating·locales:',·testError(prettier/prettier)
[error] 143-143: Delete
····(prettier/prettier)
[error] 145-145: Delete
····(prettier/prettier)
[error] 146-149: Replace
⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎····with'No·language·directories·found·in',·'/mock/path/locales'(prettier/prettier)
[error] 156-156: Delete
····(prettier/prettier)
[error] 161-161: Delete
····(prettier/prettier)
[error] 163-163: Delete
····(prettier/prettier)
[error] 164-167: Replace
⏎······expect.stringContaining(Error·reading),⏎······expect.any(String)⏎····withexpect.stringContaining(Error·reading),·expect.any(String)(prettier/prettier)
[error] 173-173: Delete
····(prettier/prettier)
[error] 177-177: Delete
····(prettier/prettier)
[error] 179-179: Delete
····(prettier/prettier)
[error] 180-183: Replace
⏎······'Error·reading·directory·/mock/dir:',⏎······'Directory·read·error'⏎····with'Error·reading·directory·/mock/dir:',·'Directory·read·error'(prettier/prettier)
[error] 184-184: Delete
····(prettier/prettier)
[error] 187-187: Delete
··(prettier/prettier)
[error] 192-192: Delete
····(prettier/prettier)
[error] 194-194: Delete
····(prettier/prettier)
[error] 201-201: Delete
····(prettier/prettier)
[error] 204-204: Delete
····(prettier/prettier)
[error] 209-209: Delete
····(prettier/prettier)
[error] 217-217: Delete
··(prettier/prettier)
[error] 220-220: Delete
····(prettier/prettier)
[error] 222-222: Delete
····(prettier/prettier)
[error] 226-226: Delete
····(prettier/prettier)
[error] 229-229: Delete
····(prettier/prettier)
[error] 231-231: Delete
····(prettier/prettier)
[error] 233-233: Delete
····(prettier/prettier)
[error] 236-236: Insert
⏎(prettier/prettier)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
tests/check-locales.test.js (1)
31-31: Minor style issueThere's a trailing whitespace in the mockExit implementation.
- const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => { }); + const mockExit = jest.spyOn(process, 'exit').mockImplementation(() => {});🧰 Tools
🪛 ESLint
[error] 31-31: Delete
·(prettier/prettier)
scripts/utils/check-locales.ts (5)
27-43: Well-implemented recursive key extractionThe
extractKeysfunction effectively extracts all keys from a nested JSON object, including proper dot notation for nested structures. However, there's a variable shadowing issue with theobjparameter.function extractKeys(obj: Record<string, any>): string[] { const extractNestedKeys = ( - obj: Record<string, any>, + nestedObj: Record<string, any>, prefix: string = '' ): string[] => { return flatten( - Object.entries(obj).map(([key, value]) => { + Object.entries(nestedObj).map(([key, value]) => { const currentKey = prefix ? `${prefix}.${key}` : key; if (value !== null && typeof value === 'object') { return extractNestedKeys(value, currentKey); } return currentKey; }) ); }; return extractNestedKeys(obj); }🧰 Tools
🪛 ESLint
[error] 28-41: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-31: Replace
⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎··withobj:·Record<string,·any>,·prefix:·string·=·''(prettier/prettier)
[error] 29-29: 'obj' is already declared in the upper scope on line 27 column 22.
(@typescript-eslint/no-shadow)
[error] 30-30: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 35-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Expected blank line before this statement.
(padding-line-between-statements)
[error] 39-39: Delete
,(prettier/prettier)
[error] 39-39: Unexpected trailing comma.
(comma-dangle)
[error] 42-42: Expected blank line before this statement.
(padding-line-between-statements)
80-162: Comprehensive locale validation implementationThe
validateLocalesfunction correctly:
- Identifies language directories
- Reads JSON files for each language
- Extracts keys for all files
- Identifies missing keys across languages
- Reports errors with detailed information
- Exits with appropriate status code
One potential improvement is to optimize the handling of negated conditions:
- if (!hasErrors) { - logger.info( - '✅ All locale files have the same keys across all languages!' - ); - } else { - logger.error( - '\n❌ Some translation keys are missing. Please fix the issues above.' - ); - process.exit(1); - } + if (hasErrors) { + logger.error( + '\n❌ Some translation keys are missing. Please fix the issues above.' + ); + process.exit(1); + } else { + logger.info( + '✅ All locale files have the same keys across all languages!' + ); + }🧰 Tools
🪛 ESLint
[error] 88-88: Expected blank line before this statement.
(padding-line-between-statements)
[error] 97-97: Expected blank line after variable declarations.
(newline-after-var)
[error] 98-98: Expected blank line before this statement.
(padding-line-between-statements)
[error] 100-103: Replace
⏎··········file,⏎··········extractKeys(content),⏎········]),withfile,·extractKeys(content)])(prettier/prettier)
[error] 102-102: Unexpected trailing comma.
(comma-dangle)
[error] 103-103: Unexpected trailing comma.
(comma-dangle)
[error] 107-109: Replace
⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎····withflatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))(prettier/prettier)
[error] 108-108: Unexpected trailing comma.
(comma-dangle)
[error] 114-116: Expected blank line after variable declarations.
(newline-after-var)
[error] 114-116: Replace
⏎········(lang)·=>·languageFiles[lang][file],⏎······with(lang)·=>·languageFiles[lang][file](prettier/prettier)
[error] 115-115: Unexpected trailing comma.
(comma-dangle)
[error] 117-117: Expected blank line before this statement.
(padding-line-between-statements)
[error] 117-117: Unexpected use of continue statement.
(no-continue)
[error] 119-121: Replace
⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······withflatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))(prettier/prettier)
[error] 120-120: Unexpected trailing comma.
(comma-dangle)
[error] 126-128: Expected blank line after variable declarations.
(newline-after-var)
[error] 126-128: Replace
⏎············(key)·=>·!langKeysSet.has(key),⏎··········with(key)·=>·!langKeysSet.has(key)(prettier/prettier)
[error] 127-127: Unexpected trailing comma.
(comma-dangle)
[error] 129-129: Expected blank line before this statement.
(padding-line-between-statements)
[error] 130-130: Delete
,(prettier/prettier)
[error] 130-130: Unexpected trailing comma.
(comma-dangle)
[error] 133-135: Replace
⏎········([_,·missing])·=>·missing.length·>·0,⏎······with([_,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 134-134: '_' is defined but never used.
(no-unused-vars)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 139-144: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 140-142: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 148-157: Unexpected negated condition.
(no-negated-condition)
[error] 149-151: Replace
⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······with'✅·All·locale·files·have·the·same·keys·across·all·languages!'(prettier/prettier)
[error] 150-150: Unexpected trailing comma.
(comma-dangle)
[error] 153-155: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 154-154: Unexpected trailing comma.
(comma-dangle)
117-117: Consider refactoring the continue statementUsing continue statements can make code harder to follow. Consider restructuring this logic for better readability.
- if (langsWithFile.length <= 1) continue; + if (langsWithFile.length <= 1) { + continue; + }Or better yet:
- if (langsWithFile.length <= 1) continue; + if (langsWithFile.length > 1) { + // Rest of the code inside this block + }🧰 Tools
🪛 ESLint
[error] 117-117: Expected blank line before this statement.
(padding-line-between-statements)
[error] 117-117: Unexpected use of continue statement.
(no-continue)
133-145: Fix unused variable and loop function referenceThere's an unused variable
_and a potential issue with referencinghasErrorsin a loop function.- const langsWithMissingKeys = Object.entries(missingKeysByLang).filter( - ([_, missing]) => missing.length > 0 - ); + const langsWithMissingKeys = Object.entries(missingKeysByLang).filter( + ([, missing]) => missing.length > 0 + ); if (langsWithMissingKeys.length > 0) { logger.info(`\nMissing keys in '${file}':`); + let errorsFound = false; langsWithMissingKeys.forEach(([lang, missing]) => { logger.error( `❌ Language '${lang}' is missing these keys: ${missing.join(', ')}` ); - hasErrors = true; + errorsFound = true; }); + hasErrors = errorsFound; }🧰 Tools
🪛 ESLint
[error] 133-135: Replace
⏎········([_,·missing])·=>·missing.length·>·0,⏎······with([_,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 134-134: '_' is defined but never used.
(no-unused-vars)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 139-144: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 140-142: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
1-170: Consider adding more error details in catch blocksThe error handling could be improved by logging more details about the caught errors.
} catch (error) { - logger.error('Error validating locales:'); + logger.error('Error validating locales:', error instanceof Error ? error.message : String(error)); process.exit(1); }Similarly in the
readJSONFilesInDirfunction:} catch (error) { - logger.error(`Error reading directory ${dir}`); + logger.error(`Error reading directory ${dir}:`, error instanceof Error ? error.message : String(error)); return {}; }🧰 Tools
🪛 ESLint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 2-2: 'path' imported multiple times.
(import/no-duplicates)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 6-6:
pathimport should occur before import of../utils/logger(import/order)
[error] 6-6: 'path' imported multiple times.
(import/no-duplicates)
[error] 13-19: Replace
⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎withcurrentDirPath,·'..',·'..',·'public',·'locales'(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 28-41: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-31: Replace
⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎··withobj:·Record<string,·any>,·prefix:·string·=·''(prettier/prettier)
[error] 29-29: 'obj' is already declared in the upper scope on line 27 column 22.
(@typescript-eslint/no-shadow)
[error] 30-30: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 35-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Expected blank line before this statement.
(padding-line-between-statements)
[error] 39-39: Delete
,(prettier/prettier)
[error] 39-39: Unexpected trailing comma.
(comma-dangle)
[error] 42-42: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Expected blank line after variable declarations.
(newline-after-var)
[error] 59-65: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-61: Expected blank line before this statement.
(padding-line-between-statements)
[error] 64-64: Expected blank line before this statement.
(padding-line-between-statements)
[error] 66-66: Delete
,(prettier/prettier)
[error] 66-66: Unexpected trailing comma.
(comma-dangle)
[error] 70-70: Expected blank line before this statement.
(padding-line-between-statements)
[error] 88-88: Expected blank line before this statement.
(padding-line-between-statements)
[error] 97-97: Expected blank line after variable declarations.
(newline-after-var)
[error] 98-98: Expected blank line before this statement.
(padding-line-between-statements)
[error] 100-103: Replace
⏎··········file,⏎··········extractKeys(content),⏎········]),withfile,·extractKeys(content)])(prettier/prettier)
[error] 102-102: Unexpected trailing comma.
(comma-dangle)
[error] 103-103: Unexpected trailing comma.
(comma-dangle)
[error] 107-109: Replace
⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎····withflatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))(prettier/prettier)
[error] 108-108: Unexpected trailing comma.
(comma-dangle)
[error] 114-116: Expected blank line after variable declarations.
(newline-after-var)
[error] 114-116: Replace
⏎········(lang)·=>·languageFiles[lang][file],⏎······with(lang)·=>·languageFiles[lang][file](prettier/prettier)
[error] 115-115: Unexpected trailing comma.
(comma-dangle)
[error] 117-117: Expected blank line before this statement.
(padding-line-between-statements)
[error] 117-117: Unexpected use of continue statement.
(no-continue)
[error] 119-121: Replace
⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······withflatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))(prettier/prettier)
[error] 120-120: Unexpected trailing comma.
(comma-dangle)
[error] 126-128: Expected blank line after variable declarations.
(newline-after-var)
[error] 126-128: Replace
⏎············(key)·=>·!langKeysSet.has(key),⏎··········with(key)·=>·!langKeysSet.has(key)(prettier/prettier)
[error] 127-127: Unexpected trailing comma.
(comma-dangle)
[error] 129-129: Expected blank line before this statement.
(padding-line-between-statements)
[error] 130-130: Delete
,(prettier/prettier)
[error] 130-130: Unexpected trailing comma.
(comma-dangle)
[error] 133-135: Replace
⏎········([_,·missing])·=>·missing.length·>·0,⏎······with([_,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 134-134: '_' is defined but never used.
(no-unused-vars)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 139-144: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 140-142: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 148-157: Unexpected negated condition.
(no-negated-condition)
[error] 149-151: Replace
⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······with'✅·All·locale·files·have·the·same·keys·across·all·languages!'(prettier/prettier)
[error] 150-150: Unexpected trailing comma.
(comma-dangle)
[error] 153-155: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 154-154: Unexpected trailing comma.
(comma-dangle)
[error] 164-164: Run autofix to sort these exports!
(simple-import-sort/exports)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/utils/check-locales.ts(1 hunks)tests/check-locales.test.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/check-locales.test.js (1)
scripts/utils/check-locales.ts (3)
extractKeys(164-164)readJSONFilesInDir(164-164)validateLocales(164-164)
🪛 ESLint
scripts/utils/check-locales.ts
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 2-2: 'path' imported multiple times.
(import/no-duplicates)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 6-6: path import should occur before import of ../utils/logger
(import/order)
[error] 6-6: 'path' imported multiple times.
(import/no-duplicates)
[error] 13-19: Replace ⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎ with currentDirPath,·'..',·'..',·'public',·'locales'
(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 28-41: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-31: Replace ⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎·· with obj:·Record<string,·any>,·prefix:·string·=·''
(prettier/prettier)
[error] 29-29: 'obj' is already declared in the upper scope on line 27 column 22.
(@typescript-eslint/no-shadow)
[error] 30-30: Unexpected trailing comma.
(comma-dangle)
[error] 34-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 35-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Expected blank line before this statement.
(padding-line-between-statements)
[error] 39-39: Delete ,
(prettier/prettier)
[error] 39-39: Unexpected trailing comma.
(comma-dangle)
[error] 42-42: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Expected blank line after variable declarations.
(newline-after-var)
[error] 59-65: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-61: Expected blank line before this statement.
(padding-line-between-statements)
[error] 64-64: Expected blank line before this statement.
(padding-line-between-statements)
[error] 66-66: Delete ,
(prettier/prettier)
[error] 66-66: Unexpected trailing comma.
(comma-dangle)
[error] 70-70: Expected blank line before this statement.
(padding-line-between-statements)
[error] 88-88: Expected blank line before this statement.
(padding-line-between-statements)
[error] 97-97: Expected blank line after variable declarations.
(newline-after-var)
[error] 98-98: Expected blank line before this statement.
(padding-line-between-statements)
[error] 100-103: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 102-102: Unexpected trailing comma.
(comma-dangle)
[error] 103-103: Unexpected trailing comma.
(comma-dangle)
[error] 107-109: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 108-108: Unexpected trailing comma.
(comma-dangle)
[error] 114-116: Expected blank line after variable declarations.
(newline-after-var)
[error] 114-116: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 115-115: Unexpected trailing comma.
(comma-dangle)
[error] 117-117: Expected blank line before this statement.
(padding-line-between-statements)
[error] 117-117: Unexpected use of continue statement.
(no-continue)
[error] 119-121: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 120-120: Unexpected trailing comma.
(comma-dangle)
[error] 126-128: Expected blank line after variable declarations.
(newline-after-var)
[error] 126-128: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 127-127: Unexpected trailing comma.
(comma-dangle)
[error] 129-129: Expected blank line before this statement.
(padding-line-between-statements)
[error] 130-130: Delete ,
(prettier/prettier)
[error] 130-130: Unexpected trailing comma.
(comma-dangle)
[error] 133-135: Replace ⏎········([_,·missing])·=>·missing.length·>·0,⏎······ with ([_,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 134-134: '_' is defined but never used.
(no-unused-vars)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 139-144: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 140-142: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 148-157: Unexpected negated condition.
(no-negated-condition)
[error] 149-151: Replace ⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······ with '✅·All·locale·files·have·the·same·keys·across·all·languages!'
(prettier/prettier)
[error] 150-150: Unexpected trailing comma.
(comma-dangle)
[error] 153-155: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 154-154: Unexpected trailing comma.
(comma-dangle)
[error] 164-164: Run autofix to sort these exports!
(simple-import-sort/exports)
tests/check-locales.test.js
[error] 31-31: Delete ·
(prettier/prettier)
[error] 58-58: Delete ,
(prettier/prettier)
[error] 105-107: Replace ⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎···· with "❌·Language·'de'·is·missing·these·keys:·key2"
(prettier/prettier)
[error] 112-112: Delete ·
(prettier/prettier)
[error] 116-118: Replace ⏎······'Error·validating·locales:'⏎···· with 'Error·validating·locales:'
(prettier/prettier)
[error] 127-130: Replace ⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎···· with 'No·language·directories·found·in',·'/mock/path/locales'
(prettier/prettier)
[error] 138-138: Delete ·
(prettier/prettier)
[error] 142-144: Replace ⏎······expect.stringContaining(Error·reading)⏎···· with expect.stringContaining(Error·reading)
(prettier/prettier)
[error] 150-150: Delete ·
(prettier/prettier)
[error] 154-156: Replace ⏎······'Error·reading·directory·/mock/dir'⏎···· with 'Error·reading·directory·/mock/dir'
(prettier/prettier)
[error] 209-209: Insert ⏎
(prettier/prettier)
🔇 Additional comments (14)
tests/check-locales.test.js (12)
1-18: Test setup looks good!The test file properly mocks the necessary dependencies (fs, logger, path) for isolated testing. The structure is clean and follows Jest best practices.
30-41: Good test coverage for theextractKeysfunctionThis test validates that the function correctly handles nested JSON objects and produces dot notation keys as expected.
🧰 Tools
🪛 ESLint
[error] 31-31: Delete
·(prettier/prettier)
43-60: Test forreadJSONFilesInDiris thoroughThe test correctly validates that:
- Only JSON files are read from the directory
- Each file's content is properly parsed
- Non-JSON files are ignored
This ensures the function behaves as expected in real-world scenarios.
🧰 Tools
🪛 ESLint
[error] 58-58: Delete
,(prettier/prettier)
62-80: Well-structured happy path test forvalidateLocalesThe test effectively verifies that when all locale files have the same keys across languages, the validation passes without errors and logs the expected success message.
82-109: Good negative test case for missing keysThis test effectively verifies that when keys are missing in locale files, the validation fails with appropriate error messages and exits with code 1.
🧰 Tools
🪛 ESLint
[error] 105-107: Replace
⏎······"❌·Language·'de'·is·missing·these·keys:·key2"⏎····with"❌·Language·'de'·is·missing·these·keys:·key2"(prettier/prettier)
111-120: Error handling test is comprehensiveThe test verifies that the validation function properly handles errors during execution and exits with the correct status code.
🧰 Tools
🪛 ESLint
[error] 112-112: Delete
·(prettier/prettier)
[error] 116-118: Replace
⏎······'Error·validating·locales:'⏎····with'Error·validating·locales:'(prettier/prettier)
122-131: Edge case handling for empty language directoriesGood coverage of the scenario where no language directories are found, ensuring proper error logging.
🧰 Tools
🪛 ESLint
[error] 127-130: Replace
⏎······'No·language·directories·found·in',⏎······'/mock/path/locales'⏎····with'No·language·directories·found·in',·'/mock/path/locales'(prettier/prettier)
133-146: Test for JSON file reading errorsThis test ensures that when JSON file reading fails, the function logs an error and returns an empty object for that file rather than crashing, which is a good defensive programming practice.
🧰 Tools
🪛 ESLint
[error] 138-138: Delete
·(prettier/prettier)
[error] 142-144: Replace
⏎······expect.stringContaining(Error·reading)⏎····withexpect.stringContaining(Error·reading)(prettier/prettier)
147-159: Directory reading error handling testThe test validates proper error handling when directory reading fails, making the code more robust against file system issues.
🧰 Tools
🪛 ESLint
[error] 150-150: Delete
·(prettier/prettier)
[error] 154-156: Replace
⏎······'Error·reading·directory·/mock/dir'⏎····with'Error·reading·directory·/mock/dir'(prettier/prettier)
161-169: Exception handling test for non-Error objectsThe test correctly verifies behavior when errors are thrown, ensuring process.exit is called.
171-189: JSON parsing error handling testThe test confirms that JSON parsing errors are properly handled by mocking the global JSON.parse function, and the original function is restored afterward, which is a good testing practice.
191-208: Edge case test for languages with no filesThis test verifies that the validation function handles the case when a language has no files properly.
scripts/utils/check-locales.ts (2)
51-72: Robust implementation of JSON file readingThe
readJSONFilesInDirfunction is well-implemented with proper error handling for both file reading and directory access errors. It correctly filters for JSON files and returns an empty object when errors occur.🧰 Tools
🪛 ESLint
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-67: Expected blank line before this statement.
(padding-line-between-statements)
[error] 58-58: Expected blank line after variable declarations.
(newline-after-var)
[error] 59-65: Expected blank line before this statement.
(padding-line-between-statements)
[error] 60-60: Expected blank line after variable declarations.
(newline-after-var)
[error] 61-61: Expected blank line before this statement.
(padding-line-between-statements)
[error] 64-64: Expected blank line before this statement.
(padding-line-between-statements)
[error] 66-66: Delete
,(prettier/prettier)
[error] 66-66: Unexpected trailing comma.
(comma-dangle)
[error] 70-70: Expected blank line before this statement.
(padding-line-between-statements)
164-169: Clean module export and entry pointThe export statement and entry point logic are well-implemented. The
istanbul ignore nextdirective appropriately excludes the entry point from test coverage.🧰 Tools
🪛 ESLint
[error] 164-164: Run autofix to sort these exports!
(simple-import-sort/exports)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/utils/check-locales.ts (1)
1-5: 🛠️ Refactor suggestionFix import statements.
There are several issues with the imports:
- Import ordering doesn't follow the project's convention
- Useless path segments in the logger import
-import fs from 'fs'; -import path, { dirname } from 'path'; -import { fileURLToPath } from 'url'; -import lodash from 'lodash'; -import { logger } from '../utils/logger'; +import fs from 'fs'; +import lodash from 'lodash'; +import path, { dirname } from 'path'; +import { fileURLToPath } from 'url'; +import { logger } from './logger';🧰 Tools
🪛 ESLint
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
🧹 Nitpick comments (8)
.github/workflows/if-nodejs-pr-testing.yml (1)
129-140: Fix trailing whitespaces in the YAML file.There are several lines with trailing whitespaces which should be removed to maintain code quality.
- + - + - + - +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 140-140: trailing spaces
(trailing-spaces)
scripts/utils/check-locales.ts (7)
12-18: Fix formatting issues.There are trailing commas and unnecessary line breaks in the
path.resolve()call.const localesDir = path.resolve( - currentDirPath, - '..', - '..', - 'public', - 'locales', + currentDirPath, '..', '..', 'public', 'locales' );🧰 Tools
🪛 ESLint
[error] 12-18: Replace
⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎withcurrentDirPath,·'..',·'..',·'public',·'locales'(prettier/prettier)
[error] 17-17: Unexpected trailing comma.
(comma-dangle)
50-71: Fix formatting and improve error handling.There are several formatting issues including trailing commas and missing blank lines. Also, the error handling should log the actual error for better debugging.
function readJSONFilesInDir(dir: string): Record<string, any> { try { const files = fs.readdirSync(dir); + return fromPairs( files .filter((file) => path.extname(file) === '.json') .map((file) => { const filePath = path.join(dir, file); + try { const content = JSON.parse(fs.readFileSync(filePath, 'utf8')); + return [file, content]; } catch (error) { - logger.error(`Error reading ${filePath}`); + logger.error(`Error reading ${filePath}:`, error); + return [file, {}]; } - }), + }) ); } catch (error) { - logger.error(`Error reading directory ${dir}`); + logger.error(`Error reading directory ${dir}:`, error); + return {}; } }🧰 Tools
🪛 ESLint
[error] 52-52: Expected blank line after variable declarations.
(newline-after-var)
[error] 53-66: Expected blank line before this statement.
(padding-line-between-statements)
[error] 57-57: Expected blank line after variable declarations.
(newline-after-var)
[error] 58-64: Expected blank line before this statement.
(padding-line-between-statements)
[error] 59-59: Expected blank line after variable declarations.
(newline-after-var)
[error] 60-60: Expected blank line before this statement.
(padding-line-between-statements)
[error] 63-63: Expected blank line before this statement.
(padding-line-between-statements)
[error] 65-65: Delete
,(prettier/prettier)
[error] 65-65: Unexpected trailing comma.
(comma-dangle)
[error] 69-69: Expected blank line before this statement.
(padding-line-between-statements)
116-116: Reconsider use of continue statement.The
continuestatement on line 116 skips to the next iteration when a file is only present in one language. Consider refactoring for clarity.- if (langsWithFile.length <= 1) continue; + if (langsWithFile.length <= 1) { + continue; + }🧰 Tools
🪛 ESLint
[error] 116-116: Expected blank line before this statement.
(padding-line-between-statements)
[error] 116-116: Unexpected use of continue statement.
(no-continue)
133-134: Replace unused variable with underscore.The variable
_is defined but never used. Consider using a more descriptive name or replacing it with the standard underscore variable.const langsWithMissingKeys = Object.entries(missingKeysByLang).filter( - ([_, missing]) => missing.length > 0, + ([_lang, missing]) => missing.length > 0 );🧰 Tools
🪛 ESLint
[error] 132-134: Replace
⏎········([_,·missing])·=>·missing.length·>·0,⏎······with([_,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 133-133: '_' is defined but never used.
(no-unused-vars)
[error] 133-133: Unexpected trailing comma.
(comma-dangle)
147-156: Simplify conditional logic.The negated condition can be refactored to a more readable positive condition.
- if (!hasErrors) { - logger.info( - '✅ All locale files have the same keys across all languages!', - ); - } else { + if (hasErrors) { logger.error( - '\n❌ Some translation keys are missing. Please fix the issues above.', + '\n❌ Some translation keys are missing. Please fix the issues above.' ); process.exit(1); + } else { + logger.info( + '✅ All locale files have the same keys across all languages!' + ); }🧰 Tools
🪛 ESLint
[error] 147-156: Unexpected negated condition.
(no-negated-condition)
[error] 148-150: Replace
⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······with'✅·All·locale·files·have·the·same·keys·across·all·languages!'(prettier/prettier)
[error] 149-149: Unexpected trailing comma.
(comma-dangle)
[error] 152-154: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 153-153: Unexpected trailing comma.
(comma-dangle)
157-160: Improve error logging.The current error logging doesn't include the actual error object, making debugging difficult.
} catch (error) { - logger.error('Error validating locales:'); + logger.error('Error validating locales:', error); process.exit(1); }
163-163: Sort exported items alphabetically.For consistency and better readability, consider sorting the exported items alphabetically.
-export { validateLocales, extractKeys, readJSONFilesInDir }; +export { extractKeys, readJSONFilesInDir, validateLocales };🧰 Tools
🪛 ESLint
[error] 163-163: Run autofix to sort these exports!
(simple-import-sort/exports)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/if-nodejs-pr-testing.yml(1 hunks)scripts/utils/check-locales.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
scripts/utils/check-locales.ts
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 12-18: Replace ⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎ with currentDirPath,·'..',·'..',·'public',·'locales'
(prettier/prettier)
[error] 17-17: Unexpected trailing comma.
(comma-dangle)
[error] 27-40: Expected blank line after variable declarations.
(newline-after-var)
[error] 27-30: Replace ⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎·· with obj:·Record<string,·any>,·prefix:·string·=·''
(prettier/prettier)
[error] 28-28: 'obj' is already declared in the upper scope on line 26 column 22.
(@typescript-eslint/no-shadow)
[error] 29-29: Unexpected trailing comma.
(comma-dangle)
[error] 33-33: Expected blank line after variable declarations.
(newline-after-var)
[error] 34-36: Expected blank line before this statement.
(padding-line-between-statements)
[error] 37-37: Expected blank line before this statement.
(padding-line-between-statements)
[error] 38-38: Delete ,
(prettier/prettier)
[error] 38-38: Unexpected trailing comma.
(comma-dangle)
[error] 41-41: Expected blank line before this statement.
(padding-line-between-statements)
[error] 52-52: Expected blank line after variable declarations.
(newline-after-var)
[error] 53-66: Expected blank line before this statement.
(padding-line-between-statements)
[error] 57-57: Expected blank line after variable declarations.
(newline-after-var)
[error] 58-64: Expected blank line before this statement.
(padding-line-between-statements)
[error] 59-59: Expected blank line after variable declarations.
(newline-after-var)
[error] 60-60: Expected blank line before this statement.
(padding-line-between-statements)
[error] 63-63: Expected blank line before this statement.
(padding-line-between-statements)
[error] 65-65: Delete ,
(prettier/prettier)
[error] 65-65: Unexpected trailing comma.
(comma-dangle)
[error] 69-69: Expected blank line before this statement.
(padding-line-between-statements)
[error] 87-87: Expected blank line before this statement.
(padding-line-between-statements)
[error] 96-96: Expected blank line after variable declarations.
(newline-after-var)
[error] 97-97: Expected blank line before this statement.
(padding-line-between-statements)
[error] 99-102: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 101-101: Unexpected trailing comma.
(comma-dangle)
[error] 102-102: Unexpected trailing comma.
(comma-dangle)
[error] 106-108: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 107-107: Unexpected trailing comma.
(comma-dangle)
[error] 113-115: Expected blank line after variable declarations.
(newline-after-var)
[error] 113-115: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 114-114: Unexpected trailing comma.
(comma-dangle)
[error] 116-116: Expected blank line before this statement.
(padding-line-between-statements)
[error] 116-116: Unexpected use of continue statement.
(no-continue)
[error] 118-120: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 119-119: Unexpected trailing comma.
(comma-dangle)
[error] 125-127: Expected blank line after variable declarations.
(newline-after-var)
[error] 125-127: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 126-126: Unexpected trailing comma.
(comma-dangle)
[error] 128-128: Expected blank line before this statement.
(padding-line-between-statements)
[error] 129-129: Delete ,
(prettier/prettier)
[error] 129-129: Unexpected trailing comma.
(comma-dangle)
[error] 132-134: Replace ⏎········([_,·missing])·=>·missing.length·>·0,⏎······ with ([_,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 133-133: '_' is defined but never used.
(no-unused-vars)
[error] 133-133: Unexpected trailing comma.
(comma-dangle)
[error] 138-143: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 139-141: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 140-140: Unexpected trailing comma.
(comma-dangle)
[error] 147-156: Unexpected negated condition.
(no-negated-condition)
[error] 148-150: Replace ⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······ with '✅·All·locale·files·have·the·same·keys·across·all·languages!'
(prettier/prettier)
[error] 149-149: Unexpected trailing comma.
(comma-dangle)
[error] 152-154: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 153-153: Unexpected trailing comma.
(comma-dangle)
[error] 163-163: Run autofix to sort these exports!
(simple-import-sort/exports)
🪛 YAMLlint (1.35.1)
.github/workflows/if-nodejs-pr-testing.yml
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 140-140: trailing spaces
(trailing-spaces)
[error] 155-155: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/if-nodejs-pr-testing.yml (3)
119-144: Implementation looks good!The locale check implementation correctly captures and formats the output from the locale test script. The error handling with
set +eandset -eensures that the workflow continues even if the locale check fails, which is appropriate for this kind of validation.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 129-129: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 140-140: trailing spaces
(trailing-spaces)
147-163: LGTM - PR comment mechanism is properly implemented.The PR comment mechanism properly uses the sticky-pull-request-comment action with appropriate headers and conditions, consistent with the existing markdown checking implementation.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 155-155: trailing spaces
(trailing-spaces)
[error] 157-157: trailing spaces
(trailing-spaces)
[error] 161-161: trailing spaces
(trailing-spaces)
164-171: LGTM - Cleanup mechanism implemented correctly.The cleanup step correctly removes the comment when no issues are found, which keeps the PR clean and ensures users only see relevant feedback.
scripts/utils/check-locales.ts (1)
165-168: LGTM - Direct script execution logic.The direct script execution logic is properly implemented with istanbul ignore for test coverage.
Co-authored-by: Akshat Nema <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
scripts/utils/check-locales.ts (3)
56-56: 🛠️ Refactor suggestionInclude error details in log message
The error logging should include the actual error object to provide more context for debugging.
- logger.error(`Error reading ${filePath}`); + logger.error(`Error reading ${filePath}`, error);
149-149: 🛠️ Refactor suggestionConsider throwing an error instead of exiting the process
Using
process.exit(1)forcefully terminates the process, which may not be ideal when this function is used as a module in other contexts.- process.exit(1); + throw new Error('Locale validation failed: some translation keys are missing');This would allow calling code to handle the error appropriately, especially if this module is imported and used in different contexts.
62-62: 🛠️ Refactor suggestionInclude error details in directory error log
Similar to the file error logging, the directory error logging should include the actual error object.
- logger.error(`Error reading directory ${dir}`); + logger.error(`Error reading directory ${dir}`, error);
🧹 Nitpick comments (4)
scripts/utils/check-locales.ts (4)
1-5: Optimize import statementsThe import statements could be improved for better organization and clarity:
- The import path for logger is using a relative path that could be simplified
- The imports should be sorted for consistency
import fs from 'fs'; -import path, { dirname } from 'path'; +import lodash from 'lodash'; +import path, { dirname } from 'path'; import { fileURLToPath } from 'url'; -import lodash from 'lodash'; -import { logger } from '../utils/logger'; +import { logger } from './logger';🧰 Tools
🪛 ESLint
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
12-12: Simplify path resolutionThe path resolution can be simplified to a single line for better readability.
-const localesDir = path.resolve(currentDirPath, '..', '..', 'public', 'locales'); +const localesDir = path.resolve(currentDirPath, '..', '..', 'public', 'locales');
20-36: Fix variable shadowing in extractKeys functionThe nested function has a parameter
objthat shadows the outer function's parameter, which could cause confusion for maintainers.function extractKeys(obj: Record<string, any>): string[] { const extractNestedKeys = ( - obj: Record<string, any>, + nestedObj: Record<string, any>, prefix: string = '', ): string[] => { + return flatten( - Object.entries(obj).map(([key, value]) => { + Object.entries(nestedObj).map(([key, value]) => { const currentKey = prefix ? `${prefix}.${key}` : key; + if (value !== null && typeof value === 'object') { return extractNestedKeys(value, currentKey); } + return currentKey; - }), + }) ); }; + return extractNestedKeys(obj); }I understand from the retrieved learnings that you prefer not to worry about variable shadowing, so feel free to ignore this suggestion if you believe it doesn't impact code readability in your context.
🧰 Tools
🪛 ESLint
[error] 21-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 21-24: Replace
⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎··withobj:·Record<string,·any>,·prefix:·string·=·''(prettier/prettier)
[error] 22-22: 'obj' is already declared in the upper scope on line 20 column 22.
(@typescript-eslint/no-shadow)
[error] 23-23: Unexpected trailing comma.
(comma-dangle)
[error] 27-27: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-30: Expected blank line before this statement.
(padding-line-between-statements)
[error] 31-31: Expected blank line before this statement.
(padding-line-between-statements)
[error] 32-32: Delete
,(prettier/prettier)
[error] 32-32: Unexpected trailing comma.
(comma-dangle)
[error] 35-35: Expected blank line before this statement.
(padding-line-between-statements)
110-110: Consider replacing 'continue' with early return patternThe use of
continueis flagged by ESLint. Consider using an early return pattern or conditional block for better code clarity.- if (langsWithFile.length <= 1) continue; + if (langsWithFile.length <= 1) { + continue; + }Alternatively:
- if (langsWithFile.length <= 1) continue; + // Skip files that exist in only one or zero languages + if (langsWithFile.length > 1) { + // Rest of the code + }🧰 Tools
🪛 ESLint
[error] 110-110: Expected blank line before this statement.
(padding-line-between-statements)
[error] 110-110: Unexpected use of continue statement.
(no-continue)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/utils/check-locales.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
scripts/utils/check-locales.ts (2)
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:138-143
Timestamp: 2025-03-23T12:42:49.503Z
Learning: For the locale validation function in check-locales.ts, the team has decided that modifying the outer scope variable 'hasErrors' inside the forEach loop is acceptable despite the ESLint warning about unsafe references in loop functions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:26-42
Timestamp: 2025-03-23T12:43:10.614Z
Learning: The user anshgoyalevil prefers not to worry about variable shadowing in JavaScript/TypeScript code as they believe scoped variables don't make much difference.
🪛 ESLint
scripts/utils/check-locales.ts
[error] 1-5: Run autofix to sort these imports!
(simple-import-sort/imports)
[error] 5-5: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 21-34: Expected blank line after variable declarations.
(newline-after-var)
[error] 21-24: Replace ⏎····obj:·Record<string,·any>,⏎····prefix:·string·=·'',⏎·· with obj:·Record<string,·any>,·prefix:·string·=·''
(prettier/prettier)
[error] 22-22: 'obj' is already declared in the upper scope on line 20 column 22.
(@typescript-eslint/no-shadow)
[error] 23-23: Unexpected trailing comma.
(comma-dangle)
[error] 27-27: Expected blank line after variable declarations.
(newline-after-var)
[error] 28-30: Expected blank line before this statement.
(padding-line-between-statements)
[error] 31-31: Expected blank line before this statement.
(padding-line-between-statements)
[error] 32-32: Delete ,
(prettier/prettier)
[error] 32-32: Unexpected trailing comma.
(comma-dangle)
[error] 35-35: Expected blank line before this statement.
(padding-line-between-statements)
[error] 46-46: Expected blank line after variable declarations.
(newline-after-var)
[error] 47-60: Expected blank line before this statement.
(padding-line-between-statements)
[error] 51-51: Expected blank line after variable declarations.
(newline-after-var)
[error] 52-58: Expected blank line before this statement.
(padding-line-between-statements)
[error] 53-53: Expected blank line after variable declarations.
(newline-after-var)
[error] 54-54: Expected blank line before this statement.
(padding-line-between-statements)
[error] 57-57: Expected blank line before this statement.
(padding-line-between-statements)
[error] 59-59: Delete ,
(prettier/prettier)
[error] 59-59: Unexpected trailing comma.
(comma-dangle)
[error] 63-63: Expected blank line before this statement.
(padding-line-between-statements)
[error] 81-81: Expected blank line before this statement.
(padding-line-between-statements)
[error] 90-90: Expected blank line after variable declarations.
(newline-after-var)
[error] 91-91: Expected blank line before this statement.
(padding-line-between-statements)
[error] 93-96: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 95-95: Unexpected trailing comma.
(comma-dangle)
[error] 96-96: Unexpected trailing comma.
(comma-dangle)
[error] 100-102: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 101-101: Unexpected trailing comma.
(comma-dangle)
[error] 107-109: Expected blank line after variable declarations.
(newline-after-var)
[error] 107-109: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 108-108: Unexpected trailing comma.
(comma-dangle)
[error] 110-110: Expected blank line before this statement.
(padding-line-between-statements)
[error] 110-110: Unexpected use of continue statement.
(no-continue)
[error] 112-114: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 113-113: Unexpected trailing comma.
(comma-dangle)
[error] 119-121: Expected blank line after variable declarations.
(newline-after-var)
[error] 119-121: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 120-120: Unexpected trailing comma.
(comma-dangle)
[error] 122-122: Expected blank line before this statement.
(padding-line-between-statements)
[error] 123-123: Delete ,
(prettier/prettier)
[error] 123-123: Unexpected trailing comma.
(comma-dangle)
[error] 126-128: Replace ⏎········([_,·missing])·=>·missing.length·>·0,⏎······ with ([_,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 127-127: '_' is defined but never used.
(no-unused-vars)
[error] 127-127: Unexpected trailing comma.
(comma-dangle)
[error] 132-137: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 133-135: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
[error] 141-150: Unexpected negated condition.
(no-negated-condition)
[error] 142-144: Replace ⏎········'✅·All·locale·files·have·the·same·keys·across·all·languages!',⏎······ with '✅·All·locale·files·have·the·same·keys·across·all·languages!'
(prettier/prettier)
[error] 143-143: Unexpected trailing comma.
(comma-dangle)
[error] 146-148: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 147-147: Unexpected trailing comma.
(comma-dangle)
[error] 157-157: Run autofix to sort these exports!
(simple-import-sort/exports)
🔇 Additional comments (3)
scripts/utils/check-locales.ts (3)
132-138: Function in loop references outer variableThe function declared inside the forEach loop references the outer variable
hasErrors, which ESLint flags as an unsafe reference in loop functions.Based on retrieved learnings, I understand this pattern has been accepted by the team for this specific case, so I'll approve this implementation.
🧰 Tools
🪛 ESLint
[error] 132-137: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 133-135: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 134-134: Unexpected trailing comma.
(comma-dangle)
157-157: LGTM: Exports are well-organizedThe exported functions provide a good API for this utility. The script can be used both as a module and as a standalone script.
🧰 Tools
🪛 ESLint
[error] 157-157: Run autofix to sort these exports!
(simple-import-sort/exports)
159-162: LGTM: Script execution logic is well-implementedThe execution logic with istanbul ignore is properly implemented. This allows the script to be run directly while also making it testable.
|
@asyncapi/bounty_team |
Co-authored-by: Akshat Nema <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
scripts/utils/check-locales.ts (1)
6-6:⚠️ Potential issueFix import path for logger to avoid useless path segments.
Use a relative path (
./logger) sincecheck-locales.tsandlogger.tsare in the same directory. This resolves theimport/no-useless-path-segmentserror and ensures the import points to the correct file.Apply this diff:
-import { logger } from '../utils/logger'; +import { logger } from './logger';🧰 Tools
🪛 ESLint
[error] 6-6: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
🧹 Nitpick comments (2)
scripts/utils/check-locales.ts (2)
174-178: Remove redundant try/catch wrapper around script execution.The catch block only rethrows the error, which is unnecessary and triggers a
no-useless-catchwarning. Simplify by callingvalidateLocales()directly.Proposed diff:
- try { - validateLocales(); - } catch (error) { - throw error; - } + validateLocales();🧰 Tools
🪛 ESLint
[error] 174-178: Unnecessary try/catch wrapper.
(no-useless-catch)
146-151: Consider avoidingforEachside effects on outer-scope variables.Modifying
hasErrorsinside aforEachcan trigger lint warnings and be less clear. As an optional improvement, you could switch to afor…ofloop or use a local flag within the loop and then assign tohasErrorsafter, enhancing readability and maintainability.🧰 Tools
🪛 ESLint
[error] 146-151: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 147-149: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 148-148: Unexpected trailing comma.
(comma-dangle)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/utils/check-locales.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
scripts/utils/check-locales.ts (3)
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:138-143
Timestamp: 2025-03-23T12:42:49.503Z
Learning: For the locale validation function in check-locales.ts, the team has decided that modifying the outer scope variable 'hasErrors' inside the forEach loop is acceptable despite the ESLint warning about unsafe references in loop functions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:26-42
Timestamp: 2025-03-23T12:43:10.614Z
Learning: The user anshgoyalevil prefers not to worry about variable shadowing in JavaScript/TypeScript code as they believe scoped variables don't make much difference.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
🪛 Biome (1.9.4)
scripts/utils/check-locales.ts
[error] 177-179: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🪛 ESLint
scripts/utils/check-locales.ts
[error] 6-6: Useless path segments for "../utils/logger", should be "./logger"
(import/no-useless-path-segments)
[error] 22-25: Replace ⏎····nestedObj:·Record<string,·any>,⏎····prefix·=·'',⏎·· with nestedObj:·Record<string,·any>,·prefix·=·''
(prettier/prettier)
[error] 24-24: Unexpected trailing comma.
(comma-dangle)
[error] 35-35: Delete ,
(prettier/prettier)
[error] 35-35: Unexpected trailing comma.
(comma-dangle)
[error] 67-67: Delete ,
(prettier/prettier)
[error] 67-67: Unexpected trailing comma.
(comma-dangle)
[error] 89-89: Expected blank line after variable declarations.
(newline-after-var)
[error] 90-90: Expected blank line before this statement.
(padding-line-between-statements)
[error] 100-100: Expected blank line after variable declarations.
(newline-after-var)
[error] 101-101: Expected blank line before this statement.
(padding-line-between-statements)
[error] 103-106: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 105-105: Unexpected trailing comma.
(comma-dangle)
[error] 106-106: Unexpected trailing comma.
(comma-dangle)
[error] 110-112: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 111-111: Unexpected trailing comma.
(comma-dangle)
[error] 117-119: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 118-118: Unexpected trailing comma.
(comma-dangle)
[error] 122-122: Unexpected use of continue statement.
(no-continue)
[error] 125-127: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 126-126: Unexpected trailing comma.
(comma-dangle)
[error] 132-134: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 133-133: Unexpected trailing comma.
(comma-dangle)
[error] 137-137: Delete ,
(prettier/prettier)
[error] 137-137: Unexpected trailing comma.
(comma-dangle)
[error] 140-142: Replace ⏎········([,·missing])·=>·missing.length·>·0,⏎······ with ([,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 146-151: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 147-149: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 148-148: Unexpected trailing comma.
(comma-dangle)
[error] 156-158: Expected blank line after variable declarations.
(newline-after-var)
[error] 156-158: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 157-157: Unexpected trailing comma.
(comma-dangle)
[error] 159-159: Expected blank line before this statement.
(padding-line-between-statements)
[error] 170-170: Run autofix to sort these exports!
(simple-import-sort/exports)
[error] 174-178: Unnecessary try/catch wrapper.
(no-useless-catch)
⏰ Context from checks skipped due to timeout of 180000ms (3)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
Co-authored-by: Akshat Nema <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
scripts/utils/check-locales.ts (7)
22-37: Improve the recursive function implementation to avoid unnecessary array operationsThe
extractNestedKeysfunction currently builds and flattens arrays for every object level, which can be inefficient for deeply nested objects. Consider using an accumulator pattern to avoid these intermediate array operations.function extractKeys(obj: Record<string, any>): string[] { const extractNestedKeys = ( nestedObj: Record<string, any>, prefix = '', + result: string[] = [] ): string[] => { - return flatten( - Object.entries(nestedObj).map(([key, value]) => { - const currentKey = prefix ? `${prefix}.${key}` : key; - - if (value !== null && typeof value === 'object') { - return extractNestedKeys(value, currentKey); - } - - return currentKey; - }), - ); + for (const [key, value] of Object.entries(nestedObj)) { + const currentKey = prefix ? `${prefix}.${key}` : key; + + if (value !== null && typeof value === 'object') { + extractNestedKeys(value, currentKey, result); + } else { + result.push(currentKey); + } + } + + return result; }; return extractNestedKeys(obj); }🧰 Tools
🪛 ESLint
[error] 22-25: Replace
⏎····nestedObj:·Record<string,·any>,⏎····prefix·=·'',⏎··withnestedObj:·Record<string,·any>,·prefix·=·''(prettier/prettier)
[error] 24-24: Unexpected trailing comma.
(comma-dangle)
[error] 35-35: Delete
,(prettier/prettier)
[error] 35-35: Unexpected trailing comma.
(comma-dangle)
63-66: Consider enhancing error handling for invalid JSON filesWhen a JSON file can't be parsed, you're returning an empty object. This might hide issues because keys will appear to be missing rather than indicating a problem with the file itself.
try { const content = JSON.parse(fs.readFileSync(filePath, 'utf8')); return [file, content]; } catch (error) { logger.error(`Error reading ${filePath}`, error); - return [file, {}]; + // Mark files with parsing errors distinctly instead of treating them as empty + return [file, { __ERROR__: `Failed to parse: ${error.message}` }]; }
83-92: Add directory existence check before reading languagesThe code assumes that the locales directory exists, but it might not in certain environments or during initial setup. Adding an explicit check would make the error message more helpful.
function validateLocales(): void { try { + // Check if the locales directory exists before proceeding + if (!fs.existsSync(localesDir)) { + const error = new Error(`Locales directory not found at ${localesDir}`); + logger.error('Locales directory not found:', error); + throw error; + } + const languages = fs .readdirSync(localesDir) .filter((file) => fs.statSync(path.join(localesDir, file)).isDirectory()); if (languages.length === 0) { const error = new Error(`No language directories found in ${localesDir}`); logger.error('No language directories found:', error); throw error; }🧰 Tools
🪛 ESLint
[error] 89-89: Expected blank line after variable declarations.
(newline-after-var)
[error] 90-90: Expected blank line before this statement.
(padding-line-between-statements)
121-123: Consider logging information about skipped filesCurrently, files that exist in only one language are silently skipped. This is consistent with your design, but logging this information might be helpful for debugging or informational purposes.
if (langsWithFile.length <= 1) { + logger.info(`Skipping '${file}' (only found in ${langsWithFile.length} language${langsWithFile.length === 1 ? '' : 's'})`); continue; }🧰 Tools
🪛 ESLint
[error] 122-122: Unexpected use of continue statement.
(no-continue)
94-108: Performance optimization for file operationsThe current implementation reads and processes each language directory sequentially. For projects with many languages and files, this could be improved with parallel processing.
- for (const lang of languages) { - const langDir = path.join(localesDir, lang); - languageFiles[lang] = readJSONFilesInDir(langDir); - fileKeys[lang] = fromPairs( - Object.entries(languageFiles[lang]).map(([file, content]) => [ - file, - extractKeys(content), - ]), - ); - } + // Process language directories in parallel for better performance + const processLanguage = (lang: string) => { + const langDir = path.join(localesDir, lang); + const files = readJSONFilesInDir(langDir); + return { + lang, + files, + keys: fromPairs( + Object.entries(files).map(([file, content]) => [ + file, + extractKeys(content), + ]), + ), + }; + }; + + // Use Promise.all for parallel processing + const results = await Promise.all(languages.map(processLanguage)); + + // Reconstruct the data structures + results.forEach(({ lang, files, keys }) => { + languageFiles[lang] = files; + fileKeys[lang] = keys; + });Note: This would require making
validateLocalesan async function and updating the call in the self-executing section to handle the Promise.🧰 Tools
🪛 ESLint
[error] 100-100: Expected blank line after variable declarations.
(newline-after-var)
[error] 101-101: Expected blank line before this statement.
(padding-line-between-statements)
[error] 103-106: Replace
⏎··········file,⏎··········extractKeys(content),⏎········]),withfile,·extractKeys(content)])(prettier/prettier)
[error] 105-105: Unexpected trailing comma.
(comma-dangle)
[error] 106-106: Unexpected trailing comma.
(comma-dangle)
155-161: Enhance error reporting with detailed statisticsWhen validation fails, providing more detailed statistics could help users quickly assess the scope of the issues.
if (hasErrors) { + // Calculate statistics for more informative error reporting + const totalMissingCount = Object.values(missingKeysByLang || {}) + .flatMap(keys => keys) + .length; + const affectedLanguagesCount = Object.entries(missingKeysByLang || {}) + .filter(([, keys]) => keys.length > 0) + .length; + const affectedFilesCount = /* calculate from previous loop */; + const error = new Error( - '\n❌ Some translation keys are missing. Please fix the issues above.', + `\n❌ Validation failed: ${totalMissingCount} missing keys across ${affectedLanguagesCount} languages. Please fix the issues above.`, ); logger.error('Translation validation failed:', error); throw error; }🧰 Tools
🪛 ESLint
[error] 156-158: Expected blank line after variable declarations.
(newline-after-var)
[error] 156-158: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 157-157: Unexpected trailing comma.
(comma-dangle)
[error] 159-159: Expected blank line before this statement.
(padding-line-between-statements)
114-153: Consider performance improvements for large datasetsFor large locale datasets with many files and keys, the current implementation could be optimized to reduce memory usage and improve performance.
The outer loop processes each file sequentially, but the inner operations could be streamlined:
- Use Sets instead of Arrays where appropriate to make lookups faster
- Avoid multiple array transformations (filter, map, flatten) that create intermediary arrays
- Consider breaking down the processing into smaller functions for better maintainability
Example optimization for the missing keys detection:
// This function could replace the current nested map/filter operations function findMissingKeys(allLanguageKeys: Record<string, Set<string>>): Record<string, string[]> { // Create a set of all unique keys across all languages const allKeysSet = new Set<string>(); for (const langKeys of Object.values(allLanguageKeys)) { for (const key of langKeys) { allKeysSet.add(key); } } // Find missing keys for each language const missingKeys: Record<string, string[]> = {}; for (const [lang, langKeys] of Object.entries(allLanguageKeys)) { const missing = Array.from(allKeysSet).filter(key => !langKeys.has(key)); if (missing.length > 0) { missingKeys[lang] = missing; } } return missingKeys; }🧰 Tools
🪛 ESLint
[error] 117-119: Replace
⏎········(lang)·=>·languageFiles[lang][file],⏎······with(lang)·=>·languageFiles[lang][file](prettier/prettier)
[error] 118-118: Unexpected trailing comma.
(comma-dangle)
[error] 122-122: Unexpected use of continue statement.
(no-continue)
[error] 125-127: Replace
⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······withflatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))(prettier/prettier)
[error] 126-126: Unexpected trailing comma.
(comma-dangle)
[error] 132-134: Replace
⏎············(key)·=>·!langKeysSet.has(key),⏎··········with(key)·=>·!langKeysSet.has(key)(prettier/prettier)
[error] 133-133: Unexpected trailing comma.
(comma-dangle)
[error] 137-137: Delete
,(prettier/prettier)
[error] 137-137: Unexpected trailing comma.
(comma-dangle)
[error] 140-142: Replace
⏎········([,·missing])·=>·missing.length·>·0,⏎······with([,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 146-151: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 147-149: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 148-148: Unexpected trailing comma.
(comma-dangle)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/utils/check-locales.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
scripts/utils/check-locales.ts (3)
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:138-143
Timestamp: 2025-03-23T12:42:49.503Z
Learning: For the locale validation function in check-locales.ts, the team has decided that modifying the outer scope variable 'hasErrors' inside the forEach loop is acceptable despite the ESLint warning about unsafe references in loop functions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:26-42
Timestamp: 2025-03-23T12:43:10.614Z
Learning: The user anshgoyalevil prefers not to worry about variable shadowing in JavaScript/TypeScript code as they believe scoped variables don't make much difference.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
🧬 Code Graph Analysis (1)
scripts/utils/check-locales.ts (1)
scripts/utils/logger.ts (1)
logger(18-18)
🪛 ESLint
scripts/utils/check-locales.ts
[error] 22-25: Replace ⏎····nestedObj:·Record<string,·any>,⏎····prefix·=·'',⏎·· with nestedObj:·Record<string,·any>,·prefix·=·''
(prettier/prettier)
[error] 24-24: Unexpected trailing comma.
(comma-dangle)
[error] 35-35: Delete ,
(prettier/prettier)
[error] 35-35: Unexpected trailing comma.
(comma-dangle)
[error] 67-67: Delete ,
(prettier/prettier)
[error] 67-67: Unexpected trailing comma.
(comma-dangle)
[error] 89-89: Expected blank line after variable declarations.
(newline-after-var)
[error] 90-90: Expected blank line before this statement.
(padding-line-between-statements)
[error] 100-100: Expected blank line after variable declarations.
(newline-after-var)
[error] 101-101: Expected blank line before this statement.
(padding-line-between-statements)
[error] 103-106: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 105-105: Unexpected trailing comma.
(comma-dangle)
[error] 106-106: Unexpected trailing comma.
(comma-dangle)
[error] 110-112: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 111-111: Unexpected trailing comma.
(comma-dangle)
[error] 117-119: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 118-118: Unexpected trailing comma.
(comma-dangle)
[error] 122-122: Unexpected use of continue statement.
(no-continue)
[error] 125-127: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 126-126: Unexpected trailing comma.
(comma-dangle)
[error] 132-134: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 133-133: Unexpected trailing comma.
(comma-dangle)
[error] 137-137: Delete ,
(prettier/prettier)
[error] 137-137: Unexpected trailing comma.
(comma-dangle)
[error] 140-142: Replace ⏎········([,·missing])·=>·missing.length·>·0,⏎······ with ([,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 141-141: Unexpected trailing comma.
(comma-dangle)
[error] 146-151: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 147-149: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 148-148: Unexpected trailing comma.
(comma-dangle)
[error] 156-158: Expected blank line after variable declarations.
(newline-after-var)
[error] 156-158: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 157-157: Unexpected trailing comma.
(comma-dangle)
[error] 159-159: Expected blank line before this statement.
(padding-line-between-statements)
[error] 170-170: Run autofix to sort these exports!
(simple-import-sort/exports)
⏰ Context from checks skipped due to timeout of 180000ms (3)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
🔇 Additional comments (3)
scripts/utils/check-locales.ts (3)
146-151: Unsafe reference to outer variable in loop functionAs previously discussed, this is modifying the outer scope variable
hasErrorsinside a forEach loop, which triggers an ESLint warning. Based on your preferences (as noted in the learnings), you've indicated this is acceptable to you since it doesn't cause functional issues in this case.🧰 Tools
🪛 ESLint
[error] 146-151: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 147-149: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 148-148: Unexpected trailing comma.
(comma-dangle)
172-175: LGTM: Self-executing code with proper guardsThe use of the
istanbul ignore nextcomment for the self-executing code section is a good practice. It ensures this section is excluded from code coverage reports while allowing the script to be run directly. The check forprocess.argv[1]is also the proper way to detect when the file is being executed directly.
1-6: Imports are well organizedThe imports are properly structured with built-in Node.js modules first, followed by third-party packages, and finally local imports. This follows best practices for import ordering.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/check-locales.test.js (3)
43-60: Well-structured test for readJSONFilesInDir.This test properly verifies that JSON files are read and parsed correctly while non-JSON files are ignored.
However, there's a minor formatting issue flagged by ESLint:
- expect(result).toEqual({ - 'file1.json': { key1: 'value1' }, - 'file2.json': { key2: 'value2' }, - }); + expect(result).toEqual({ + 'file1.json': { key1: 'value1' }, + 'file2.json': { key2: 'value2' } + });🧰 Tools
🪛 ESLint
[error] 58-58: Delete
,(prettier/prettier)
131-144: Good test for directory reading errors.This test ensures proper error handling when reading a directory fails.
There's a minor formatting issue flagged by ESLint:
- expect(logger.error).toHaveBeenCalledWith( - 'Error reading directory /mock/dir', - expect.any(Error) - ); + expect(logger.error).toHaveBeenCalledWith( + 'Error reading directory /mock/dir', expect.any(Error) + );🧰 Tools
🪛 ESLint
[error] 134-134: Delete
·(prettier/prettier)
[error] 138-141: Replace
⏎······'Error·reading·directory·/mock/dir',⏎······expect.any(Error)⏎····with'Error·reading·directory·/mock/dir',·expect.any(Error)(prettier/prettier)
146-152: Misleading test description.The test description mentions "non-Error exceptions", but the mock is actually throwing an Error object.
- it('should handle non-Error exceptions during validation', () => { + it('should handle Error exceptions during validation', () => {scripts/utils/check-locales.ts (2)
8-19: Good utility function setup and path resolution.The path resolution for the locales directory is well-structured.
However, there's a formatting issue flagged by ESLint:
-const localesDir = path.resolve( - currentDirPath, - '..', - '..', - 'public', - 'locales', -); +const localesDir = path.resolve(currentDirPath, '..', '..', 'public', 'locales');This is a minor stylistic issue that doesn't affect functionality.
🧰 Tools
🪛 ESLint
[error] 13-19: Replace
⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎withcurrentDirPath,·'..',·'..',·'public',·'locales'(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
180-185: Clean exports and self-execution logic.The file exports the necessary functions for testing and self-executes validateLocales when run directly.
ESLint suggests sorting the exports, which is a minor improvement:
-export { validateLocales, extractKeys, readJSONFilesInDir }; +export { extractKeys, readJSONFilesInDir, validateLocales };🧰 Tools
🪛 ESLint
[error] 180-180: Run autofix to sort these exports!
(simple-import-sort/exports)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/utils/check-locales.ts(1 hunks)tests/check-locales.test.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
scripts/utils/check-locales.ts (3)
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:138-143
Timestamp: 2025-03-23T12:42:49.503Z
Learning: For the locale validation function in check-locales.ts, the team has decided that modifying the outer scope variable 'hasErrors' inside the forEach loop is acceptable despite the ESLint warning about unsafe references in loop functions.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:26-42
Timestamp: 2025-03-23T12:43:10.614Z
Learning: The user anshgoyalevil prefers not to worry about variable shadowing in JavaScript/TypeScript code as they believe scoped variables don't make much difference.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
🧬 Code Graph Analysis (2)
scripts/utils/check-locales.ts (1)
scripts/utils/logger.ts (1)
logger(18-18)
tests/check-locales.test.js (3)
scripts/utils/check-locales.ts (3)
extractKeys(180-180)readJSONFilesInDir(180-180)validateLocales(180-180)utils/i18n.ts (1)
languages(20-20)scripts/utils/logger.ts (1)
logger(18-18)
🪛 ESLint
scripts/utils/check-locales.ts
[error] 13-19: Replace ⏎··currentDirPath,⏎··'..',⏎··'..',⏎··'public',⏎··'locales',⏎ with currentDirPath,·'..',·'..',·'public',·'locales'
(prettier/prettier)
[error] 18-18: Unexpected trailing comma.
(comma-dangle)
[error] 28-32: Replace ⏎····nestedObj:·Record<string,·any>,⏎····prefix·=·'',⏎····result:·string[]·=·[],⏎·· with nestedObj:·Record<string,·any>,·prefix·=·'',·result:·string[]·=·[]
(prettier/prettier)
[error] 31-31: Unexpected trailing comma.
(comma-dangle)
[error] 74-74: Delete ,
(prettier/prettier)
[error] 74-74: Unexpected trailing comma.
(comma-dangle)
[error] 96-96: Expected blank line after variable declarations.
(newline-after-var)
[error] 97-97: Expected blank line before this statement.
(padding-line-between-statements)
[error] 107-107: Expected blank line after variable declarations.
(newline-after-var)
[error] 108-108: Expected blank line before this statement.
(padding-line-between-statements)
[error] 110-113: Replace ⏎··········file,⏎··········extractKeys(content),⏎········]), with file,·extractKeys(content)])
(prettier/prettier)
[error] 112-112: Unexpected trailing comma.
(comma-dangle)
[error] 113-113: Unexpected trailing comma.
(comma-dangle)
[error] 117-119: Replace ⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎···· with flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))
(prettier/prettier)
[error] 118-118: Unexpected trailing comma.
(comma-dangle)
[error] 124-126: Replace ⏎········(lang)·=>·languageFiles[lang][file],⏎······ with (lang)·=>·languageFiles[lang][file]
(prettier/prettier)
[error] 125-125: Unexpected trailing comma.
(comma-dangle)
[error] 129-131: Replace ⏎··········Skipping·'${file}'·(only·found·in·${langsWithFile.length}·language),⏎········ with Skipping·'${file}'·(only·found·in·${langsWithFile.length}·language)
(prettier/prettier)
[error] 130-130: Unexpected trailing comma.
(comma-dangle)
[error] 132-132: Unexpected use of continue statement.
(no-continue)
[error] 135-137: Replace ⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······ with flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))
(prettier/prettier)
[error] 136-136: Unexpected trailing comma.
(comma-dangle)
[error] 142-144: Replace ⏎············(key)·=>·!langKeysSet.has(key),⏎·········· with (key)·=>·!langKeysSet.has(key)
(prettier/prettier)
[error] 143-143: Unexpected trailing comma.
(comma-dangle)
[error] 147-147: Delete ,
(prettier/prettier)
[error] 147-147: Unexpected trailing comma.
(comma-dangle)
[error] 150-152: Replace ⏎········([,·missing])·=>·missing.length·>·0,⏎······ with ([,·missing])·=>·missing.length·>·0
(prettier/prettier)
[error] 151-151: Unexpected trailing comma.
(comma-dangle)
[error] 156-161: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 157-159: Replace ⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎·········· with ❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}
(prettier/prettier)
[error] 158-158: Unexpected trailing comma.
(comma-dangle)
[error] 166-168: Expected blank line after variable declarations.
(newline-after-var)
[error] 166-168: Replace ⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······ with '\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'
(prettier/prettier)
[error] 167-167: Unexpected trailing comma.
(comma-dangle)
[error] 169-169: Expected blank line before this statement.
(padding-line-between-statements)
[error] 180-180: Run autofix to sort these exports!
(simple-import-sort/exports)
tests/check-locales.test.js
[error] 31-31: Delete ·
(prettier/prettier)
[error] 58-58: Delete ,
(prettier/prettier)
[error] 120-120: Delete ·
(prettier/prettier)
[error] 134-134: Delete ·
(prettier/prettier)
[error] 138-141: Replace ⏎······'Error·reading·directory·/mock/dir',⏎······expect.any(Error)⏎···· with 'Error·reading·directory·/mock/dir',·expect.any(Error)
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 180000ms (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (24)
tests/check-locales.test.js (15)
1-5: Good use of explicit mock object for filesystem operations.The mock filesystem approach is well-structured, making it easy to control the behavior of these functions throughout your tests.
7-8: Clean imports for the tested functions.Your imports correctly reference the functions being tested and the logger dependency.
10-18: Well-structured logger mock.The logger mock properly implements all required methods (error, warn, info) that will be used in the tests.
20-28: Comprehensive path module mock.This mock preserves original path functionality while overriding specific methods needed for testing. The use of
mockImplementationfor thejoinmethod is particularly good for maintaining path-like behavior.
30-35: Good setup for the test suite.The
mockExitspy prevents actual process termination during tests, and thebeforeEachhook ensures clean mocks between test runs.🧰 Tools
🪛 ESLint
[error] 31-31: Delete
·(prettier/prettier)
37-41: Simple and effective test for extractKeys.This test clearly verifies that the
extractKeysfunction correctly extracts dot-notation keys from nested objects.
62-79: Good happy path test for validateLocales.This test properly verifies that the function logs success when all locale files have consistent keys across languages.
81-99: Thorough test for missing keys scenario.This test correctly verifies that validateLocales throws an error when keys are missing in some languages.
101-107: Good error handling test.This test ensures that errors during validation are properly propagated.
109-113: Comprehensive edge case testing.Good test for the scenario when no language directories are found.
115-129: Thorough testing of JSON file reading errors.This test properly verifies error logging and the fallback to an empty object when file reading fails.
🧰 Tools
🪛 ESLint
[error] 120-120: Delete
·(prettier/prettier)
154-172: Careful handling of global JSON.parse override.Good approach using a try-finally block to ensure the global JSON.parse method is restored even if the test fails.
174-191: Good edge case testing for empty language directories.This test thoroughly verifies that the validation correctly handles the scenario when a language has no locale files.
193-211: Appropriate test for success case with multiple languages.This test confirms that validateLocales correctly identifies when all keys match across languages.
213-230: Comprehensive test for skipping files found in only one language.This test properly verifies that files unique to a single language are skipped with appropriate logging.
scripts/utils/check-locales.ts (9)
1-6: Well-organized imports.The imports are properly organized by type, with core Node.js modules first, followed by external dependencies and internal modules.
21-47: Well-documented and efficient key extraction function.The
extractKeysfunction is well-documented with JSDoc comments and effectively extracts nested keys from JSON objects using dot notation. The recursive implementation is clean and works correctly.🧰 Tools
🪛 ESLint
[error] 28-32: Replace
⏎····nestedObj:·Record<string,·any>,⏎····prefix·=·'',⏎····result:·string[]·=·[],⏎··withnestedObj:·Record<string,·any>,·prefix·=·'',·result:·string[]·=·[](prettier/prettier)
[error] 31-31: Unexpected trailing comma.
(comma-dangle)
49-81: Robust file reading with proper error handling.The
readJSONFilesInDirfunction handles both directory reading errors and individual file parsing errors gracefully, with appropriate logging.🧰 Tools
🪛 ESLint
[error] 74-74: Delete
,(prettier/prettier)
[error] 74-74: Unexpected trailing comma.
(comma-dangle)
83-99: Good initial setup in validateLocales.The function properly initializes by checking for language directories and handles the case when none are found with appropriate error reporting.
🧰 Tools
🪛 ESLint
[error] 96-96: Expected blank line after variable declarations.
(newline-after-var)
[error] 97-97: Expected blank line before this statement.
(padding-line-between-statements)
101-115: Thorough language file processing.This section effectively reads all locale files and extracts their keys for each language.
🧰 Tools
🪛 ESLint
[error] 107-107: Expected blank line after variable declarations.
(newline-after-var)
[error] 108-108: Expected blank line before this statement.
(padding-line-between-statements)
[error] 110-113: Replace
⏎··········file,⏎··········extractKeys(content),⏎········]),withfile,·extractKeys(content)])(prettier/prettier)
[error] 112-112: Unexpected trailing comma.
(comma-dangle)
[error] 113-113: Unexpected trailing comma.
(comma-dangle)
117-133: Efficient file skipping logic.The code correctly identifies and skips files that only appear in a single language. Based on the retrieved learning, this is appropriate since Next.js handles missing file errors at runtime.
🧰 Tools
🪛 ESLint
[error] 117-119: Replace
⏎······flatten(Object.values(languageFiles).map((files)·=>·Object.keys(files))),⏎····withflatten(Object.values(languageFiles).map((files)·=>·Object.keys(files)))(prettier/prettier)
[error] 118-118: Unexpected trailing comma.
(comma-dangle)
[error] 124-126: Replace
⏎········(lang)·=>·languageFiles[lang][file],⏎······with(lang)·=>·languageFiles[lang][file](prettier/prettier)
[error] 125-125: Unexpected trailing comma.
(comma-dangle)
[error] 129-131: Replace
⏎··········Skipping·'${file}'·(only·found·in·${langsWithFile.length}·language),⏎········withSkipping·'${file}'·(only·found·in·${langsWithFile.length}·language)(prettier/prettier)
[error] 130-130: Unexpected trailing comma.
(comma-dangle)
[error] 132-132: Unexpected use of continue statement.
(no-continue)
134-163: Comprehensive missing key detection.This section effectively identifies missing keys across languages and logs detailed error messages. The reference to
hasErrorsinside the forEach loop is accepted by the team based on the retrieved learnings.🧰 Tools
🪛 ESLint
[error] 135-137: Replace
⏎········flatten(langsWithFile.map((lang)·=>·fileKeys[lang][file])),⏎······withflatten(langsWithFile.map((lang)·=>·fileKeys[lang][file]))(prettier/prettier)
[error] 136-136: Unexpected trailing comma.
(comma-dangle)
[error] 142-144: Replace
⏎············(key)·=>·!langKeysSet.has(key),⏎··········with(key)·=>·!langKeysSet.has(key)(prettier/prettier)
[error] 143-143: Unexpected trailing comma.
(comma-dangle)
[error] 147-147: Delete
,(prettier/prettier)
[error] 147-147: Unexpected trailing comma.
(comma-dangle)
[error] 150-152: Replace
⏎········([,·missing])·=>·missing.length·>·0,⏎······with([,·missing])·=>·missing.length·>·0(prettier/prettier)
[error] 151-151: Unexpected trailing comma.
(comma-dangle)
[error] 156-161: Function declared in a loop contains unsafe references to variable(s) 'hasErrors'.
(@typescript-eslint/no-loop-func)
[error] 157-159: Replace
⏎············❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')},⏎··········with❌·Language·'${lang}'·is·missing·these·keys:·${missing.join(',·')}(prettier/prettier)
[error] 158-158: Unexpected trailing comma.
(comma-dangle)
165-174: Good error handling for missing keys.The function appropriately throws an error with a clear message when keys are missing, ensuring the validation process fails early and visibly.
🧰 Tools
🪛 ESLint
[error] 166-168: Expected blank line after variable declarations.
(newline-after-var)
[error] 166-168: Replace
⏎········'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.',⏎······with'\n❌·Some·translation·keys·are·missing.·Please·fix·the·issues·above.'(prettier/prettier)
[error] 167-167: Unexpected trailing comma.
(comma-dangle)
[error] 169-169: Expected blank line before this statement.
(padding-line-between-statements)
174-178: Proper error propagation.The function correctly catches, logs, and re-throws any errors that occur during validation.
|
/rtm |
github-project-automation
bot
moved this from In Progress
to Completed
in Bounty Program
5 participants
Description
Related issue(s)
fixes #3777
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests