Uh oh!
There was an error while loading. Please reload this page.
chore(i18n): add en.meta.json generation script - #1761
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
ec2a5b0 to
7e980e9CompareCodecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
da9f64a to
68b0110Compare40ae5b6 to
4b13bdfCompare4b13bdf to
8f1acd7CompareNote Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 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 a scripts tsconfig, an npm script, and unit tests for the metadata generation. Also adds a vitest alias to resolve script imports. Possibly related issues
Possibly related PRs
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 |
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
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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}`)+ } }
en.meta.json generation scripten.meta.json generation scripttrueberryless
commented
Jun 13, 2026
Hi, @shuuji3! Sorry to be that guy, but it seems this PR is quite outdated. Do you know the current state of it? Do we still aim to provide more info for translation strings or is the current setup with schema and I took the liberty to help you out and resolve the merge conflicts, so this is up-to-date again 👍 |
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
e18e dependency analysisNo dependency warnings found. |
zizmor suggestions
# Conflicts: # package.json # pnpm-lock.yaml
🔗 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.