Skip to content

Fix duplicate react-intl message ids breaking yarn build-locales - #7318

Open
sumitdahal7 wants to merge 4 commits into
developfrom
fix/7284-duplicate-message-ids
Open

Fix duplicate react-intl message ids breaking yarn build-locales#7318
sumitdahal7 wants to merge 4 commits into
developfrom
fix/7284-duplicate-message-ids

Conversation

@sumitdahal7

Copy link
Copy Markdown
Collaborator

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation
  • 🧑‍💻 Refactor
  • ✅ Test
  • 🤖 Build or CI
  • ❓ Other (please specify)

Related Issue

Fixes #7284

Describe this PR

yarn build-locales has been failing on develop since 23 June 2025:

Error: Duplicate message id: management.button.cancel

combine-messages refuses to run when two messages.js files declare the same id, even when the defaultMessage is identical. Because that command is the only thing that generates frontend/src/locales/en.json, the catalog has been frozen for over a year. Nothing caught it and CI never ran build-locales, and react-intl silently falls back to the defaultMessage in code when a key is missing from the catalog, so nothing looked broken in English.

The visible cost: 142 strings added since June 2025 never reached Transifex, so no translator has ever seen them. They render as English in all 27 non-English locales, permanently and silently.

The reported id is only the first one the tool hits. A full audit found 13 duplicated ids, in two categories:

Nine identical duplicates. badges/messages.js and levels/messages.js re-declared ids that teamsAndOrgs/messages.js has owned since 2019 — management.button.cancel, .save, management.fields.name, .description, .managers.add, .organisation.image, management.link.manage, management.badges, management.levels. They now import and re-export the canonical descriptors. Same ids, same rendered text, no component changes, existing translations in all 28 locale files untouched.

Four conflicting duplicates, where one id mapped to two different English strings and whichever won depended on extraction order:

  • management.projects.create.review.database + .osm + .sandbox — projectCreate says "Where should edits be saved?" / "OpenStreetMap (Live)" / "Training Sandbox (Practice)"; projectEdit says "Database" / "OSM" / "Sandbox". projectEdit's now use management.projects.edit.database*.
  • project.detail.sandbox — declared three times. header/messages.js's copy is unused (the badge moved to the project detail header in Reposition sandbox mode badge from main header #7278) and is deleted. projectDetail keeps the id; taskSelection's InfoBox becomes project.detail.sandbox.info, pairing with its existing project.detail.sandbox.tooltip sibling.

None of the renamed ids had ever reached Transifex, so no translations are lost.

Regenerated en.json with 148 additions covering a year of accumulated strings, 4 removals for messages actually deleted from source. The catalog now round-trips exactly: 1554 ids declared in code, 1554 keys in the file, zero drift in either direction.

CI guard in pr_test_frontend.yml. It regenerates the catalog and diffs it, so it fails both on a duplicate id (build-locales exits non-zero) and on a stale committed catalog, the second case being the one that actually caused this, since it's easy to add a string and forget to regenerate. docs/developers/translations.md is updated to match: the old wording said running build-locales was "the ideal", which is precisely why it got skipped.

Screenshots

N/A

Alternative Approaches Considered

Renaming the shared ids instead of re-exporting them. This would have created nine new Transifex keys and thrown away translations that already exist in all 28 locale files. Re-exporting keeps the ids stable, which is what preserves the translations.

A pre-commit hook that regenerates automatically. Better developer experience, but it needs frontend/node_modules, which pre-commit.ci's container doesn't have, so it would need a ci: skip block and would still fail for anyone who hasn't installed frontend deps. The CI check alone fully closes the correctness gap. Worth revisiting as its own PR if the CI check proves annoying in practice.

Review Guide

Locally: 279 tests passed across 57 suites; eslint --max-warnings 0 clean on the changed files; CI=true yarn build clean.

Two things to know about the CI step:

  • It deliberately runs before yarn test, so a stale catalog fails fast. Happy to move it after if you'd rather always get test results.
  • It normalizes the trailing newline before diffing. combine-messages writes none; pre-commit's end-of-file-fixer adds one to the committed file. Without that line the check would fail on every single run.

After merge, someone with Transifex access needs to run ./scripts/transifex/tx-docker.sh push -s from the repo root. Regenerating en.json only gets the strings into the source catalog and it doesn't notify translators. Without that step this PR only solves half of #7284. This is the step docs/developers/translations.md describes.

AI Disclosure

  • This PR was created with significant help from AI tools (e.g., Claude, Copilot, ChatGPT)

Claude assisted with the duplicate-id audit across all messages.js files, tracing the git history behind the conflicting ids, and drafting this description. I made the calls on how to resolve each conflict, verified every claim against the repo, and ran the full test and build suite locally. The en.json diff is entirely generator output with no hand edits.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] "yarn build-locales" fails on develop: duplicate react-intl message ids

1 participant