Skip to content

Comments

chore: prevent adding missing entries to wrong locale files#1157

Merged
danielroe merged 5 commits intonpmx-dev:mainfrom
userquin:prevent-updating-country-variants-files
Feb 7, 2026
Merged

chore: prevent adding missing entries to wrong locale files#1157
danielroe merged 5 commits intonpmx-dev:mainfrom
userquin:prevent-updating-country-variants-files

Conversation

@userquin
Copy link
Member

@userquin userquin commented Feb 7, 2026

This PR is the continuation of #1063 where we didn't prevent fix wrong locale files (locales):

  • country locale files inside country variants cannot be fixed: e.g. running nr i18n:check:fix es-ES is wrong, we should run nr i18n:check:fix es
  • target locale inside country variants cannot be fixed: e.g. this is wrong nr i18n:check:fix es-419, since i18n will do the work for us (we should fix only language locale)
  • allow run i18n:check:fix: won't save files with mergeLocale: true

This PR also includes a fix when running build:lunaria script, won't add extra \n at the end.

/cc @shuuji3

@vercel
Copy link

vercel bot commented Feb 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 7, 2026 4:07pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 7, 2026 4:07pm
npmx-lunaria Ignored Ignored Feb 7, 2026 4:07pm

Request Review

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

📝 Walkthrough

Walkthrough

This pull request makes two changes to translation tooling: it appends a trailing newline when writing JSON locale files in lunaria/prepare-json-files.ts, and it updates scripts/compare-translations.ts to add a singleLocale parameter and a guard that errors/exits when --fix is used against a merged/variant locale. It also changes when updated JSON is written for localized files to avoid writing for merged locales unless removals or fixes apply to non-merged locales.

Possibly related PRs

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly relates to the changeset, describing the prevention of i18n fixes to incorrect locale files and a trailing newline fix in the build script.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

@shuuji3 shuuji3 self-requested a review February 7, 2026 15:36
Copy link
Member

@shuuji3 shuuji3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I tested with the same text files in #1063 and existing JSON. And, lunaria doesn't strip the newline at EOF 😀

There's only one remaining issue, when it runs in all locale mode (runAllLocales()) that is without specifying a specific locale in the command, it still try processing ar-EG. We should filter out that such case.

> pnpm run i18n:check:fix

> npmx@0.0.0 i18n:check:fix /home/shuuji3/dev/npmx.dev
> node scripts/compare-translations.ts --fix

Error: Locale "ar-EG" cannot be fixed, fix the ar locale instead!
 ELIFECYCLE  Command failed with exit code 1.

@userquin
Copy link
Member Author

userquin commented Feb 7, 2026

@shuuji3 Changes in this PR will prevent to run nr i18n:check:fix, maybe we can pass a new flag to just ignore the fix when running previous command), the script will fail, the problem is about mix fixing files and collecting info.

@userquin
Copy link
Member Author

userquin commented Feb 7, 2026

There's only one remaining issue, when it runs in all locale mode (runAllLocales()) that is without specifying a specific locale in the command, it still try processing ar-EG. We should filter out that such case.

I'm going to update the logic... syncLocaleData will just ignore the update when mergeLocale is true

@userquin
Copy link
Member Author

userquin commented Feb 7, 2026

@shuuji3 to collect the info, the processLocale won't exit and won't save the files with mergeLocale: true:

image

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Copy link
Member

@shuuji3 shuuji3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It all seems to be OK now 🙂

@danielroe danielroe added this pull request to the merge queue Feb 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 7, 2026
@danielroe danielroe added this pull request to the merge queue Feb 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 7, 2026
@danielroe danielroe added this pull request to the merge queue Feb 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 7, 2026
@danielroe danielroe added this pull request to the merge queue Feb 7, 2026
Merged via the queue into npmx-dev:main with commit 3b077e1 Feb 7, 2026
17 checks passed
@userquin userquin deleted the prevent-updating-country-variants-files branch February 7, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants