feat(i18n): add en.meta.json generation script#1761
feat(i18n): add en.meta.json generation script#1761shuuji3 wants to merge 6 commits intonpmx-dev:mainfrom
en.meta.json generation script#1761Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
shuuji3
force-pushed
the
feat/generate-en-meta-json
branch
from
ec2a5b0 to
7e980e9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
shuuji3
force-pushed
the
feat/generate-en-meta-json
branch
from
da9f64a to
68b0110
Compare
shuuji3
force-pushed
the
feat/generate-en-meta-json
branch
2 times, most recently
from
40ae5b6 to
4b13bdf
Compare
shuuji3
force-pushed
the
feat/generate-en-meta-json
branch
from
4b13bdf to
8f1acd7
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds tooling and automation to maintain English i18n metadata. Introduces a GitHub Actions workflow that triggers on changes to i18n/locales/en.json and commits updates to i18n/locales/en.meta.json. Adds a CLI, utilities, and a script to compute en.meta.json from en.json (tracking per-key commit hashes and a $meta block), TypeScript types and tsconfig for the scripts, an npm script, unit tests for the metadata generation, and a vitest alias for resolving script imports. Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
scripts/tsconfig.json (1)
8-12: Minor configuration inconsistency.The
declarationanddeclarationMapoptions have no effect whennoEmit: trueis set, as no output files are generated. Consider removing these redundant options for clarity.♻️ Suggested fix
"skipLibCheck": true, "noEmit": true, "allowImportingTsExtensions": true, - "declaration": true, - "types": ["node"], - "declarationMap": true + "types": ["node"]scripts/i18n-meta/cli.ts (1)
10-15: Add explicit type guard for array access.Accessing
positionals[0]without a guard is not strictly type-safe. While the current logic works correctly (showing help when undefined), an explicit check aligns better with the coding guidelines.♻️ Suggested fix
function main() { const { positionals } = parseArgs({ allowPositionals: true }) + const command = positionals[0] - if (positionals[0] !== 'update-en-meta') { + if (command !== 'update-en-meta') { showHelp() return } updateEnMetaJson() }As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
.github/workflows/i18n-meta.yml (1)
38-48: Consider adding error handling for the commit step.If the
pnpm i18n:meta:update-en-metastep fails or exits unexpectedly, the workflow continues to the commit step. Consider adding explicit error handling or usingset -ein the shell script.♻️ Suggested improvement
- name: ⬆︎ Commit and Push changes run: | + set -e git config --global user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git add i18n/locales/en.meta.jsonscripts/i18n-meta/update-en-meta-json.ts (1)
23-27: Silent fallback to empty object when no commit hash is available.If
getCurrentCommitHash()returnsnullor empty, the function silently uses an empty object as metadata. This could mask issues in environments where Git isn't properly configured. Consider logging a warning.♻️ Suggested improvement
const currentCommitHash = getCurrentCommitHash() + if (!currentCommitHash) { + console.warn('⚠️ Could not determine current commit hash – metadata will be empty') + } const enMetaJson = currentCommitHash ? makeEnMetaJson(oldEnMetaJson, newEnJson, currentCommitHash) : ({} as EnMetaJson)scripts/i18n-meta/git-utils.ts (1)
41-44: Use order-insensitive comparison for translation change detection.At Line 44,
JSON.stringify(oldObj) !== JSON.stringify(newObj)is key-order sensitive, so equivalent objects with different key order will be treated as changed.Suggested fix
+function sortKeysRecursively(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(sortKeysRecursively) + } + if (value && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value as Record<string, unknown>) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([k, v]) => [k, sortKeysRecursively(v)]), + ) + } + return value +} + export function checkTranslationChanges(oldMeta: EnMetaJson, newMeta: EnMetaJson): boolean { const oldObj = omitMeta(oldMeta) const newObj = omitMeta(newMeta) - return JSON.stringify(oldObj) !== JSON.stringify(newObj) + return JSON.stringify(sortKeysRecursively(oldObj)) !== JSON.stringify(sortKeysRecursively(newObj)) }
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/i18n-meta.ymlpackage.jsonscripts/i18n-meta/cli.tsscripts/i18n-meta/git-utils.tsscripts/i18n-meta/types.d.tsscripts/i18n-meta/update-en-meta-json.tsscripts/tsconfig.jsontest/unit/scripts/generate-en-meta-json.spec.tsvitest.config.ts
There was a problem hiding this comment.
🧹 Nitpick comments (3)
scripts/i18n-meta/update-en-meta-json.ts (1)
47-61: Consider adding explicit return type annotation for clarity.The logic is sound: flattening both structures, comparing text values, and preserving or updating commit hashes as appropriate. The nullish coalescing at line 58 correctly handles new keys where
lastCommitwould beundefined.One minor note: the
as Record<string, string>casts at lines 47-48 are imprecise since accessing a missing key returnsundefinedat runtime. The current logic handles this correctly, but for stricter type-safety you could use a safer accessor pattern.♻️ Optional: safer key access pattern
- const lastSeenText = oldMetaFlat[`${key}.text`] - const lastCommit = oldMetaFlat[`${key}.commit`] + const lastSeenText = oldMetaFlat[`${key}.text`] as string | undefined + const lastCommit = oldMetaFlat[`${key}.commit`] as string | undefinedThis makes the potential
undefinedexplicit, aligning runtime behaviour with the type system.scripts/i18n-meta/utils.ts (2)
5-7: Add explicit return type annotation forgetCurrentCommitHash.The function can return
string | null(from thegithelper), but this isn't explicitly declared. Adding the return type improves API clarity for callers.♻️ Suggested improvement
-export function getCurrentCommitHash() { +export function getCurrentCommitHash(): string | null { return git('rev-parse HEAD') }
48-50: Consider wrapping JSON.parse in try-catch for clearer error messages.If the file contains invalid JSON, the error will propagate with a generic parse error. A wrapper could provide context about which file failed.
♻️ Optional: improved error context
function readJson<T>(filePath: string): T { - return JSON.parse(fs.readFileSync(filePath, 'utf-8')) as T + try { + return JSON.parse(fs.readFileSync(filePath, 'utf-8')) as T + } catch (error) { + throw new Error(`Failed to parse JSON from ${filePath}: ${error instanceof Error ? error.message : error}`) + } }
1 participant
🔗 Linked issue
#1747
🧭 Context
Currently, there is no mechanism to detect changes in
en.json. Previously, we had to choose one of the following approaches when English text needed an update:This first step records metadata, tracking which commit updated each English key, which allows us to monitor changes in the English locale file effectively.
📚 Description
Implemented the first two tasks for automated
en.jsonchange detection:en.meta.jsonalongsideen.json.en.jsonis changed (added, removed, or modified) since the previous update, the corresponding values inen.meta.jsonare updated with the latest commit hash and the new string value.mainbranch, only when the commit modifiesi18n/locales/en.json.mainbranch history.The example auto-commit of
en.meta.jsoncan be found in this CI test branch: https://github.com/shuuji3/npmx.dev/commits/feat/generate-en-meta-json-ci-test(Note: In the test branch, I pushed directly to
mainbranch andlunaria/files/*is included. But they are usually merged along with PR, so they should not be included in auto-commit)You can also check the Vitest file to see how this script records commit hash and text.