fix: i18n locale resolution (spurious es/fr 404s; short locales served English)#31
Conversation
…erved English) i18next's default load:'all' requested both the full locale (es_ES → 200) and its base language (es → 404, no such dir), logging a 404 each load; and a site whose WP locale is a short code (es, fr) requested only that → 404 → silently fell back to English instead of its language. Fix: load:'currentOnly' (request only the exact locale, no redundant base) plus a loadPath function that maps the requested code to an available directory via resolveLocaleDir (short es → es_ES, etc.). Extracted to a pure, unit-tested src/shared/locale-resolve.mjs. Verified: every locale now makes exactly one request to the correct dir; short codes resolve to their region.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: wcpos/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesLocale resolver and i18n CDN path integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71363e543b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fix triage
Working through #1 now; will reply in-thread and push. |
) BCP-47 hyphen forms (zh-TW, es-MX, pt-PT) collapsed to the base language's first region (zh_CN, es_ES, pt_BR) because the lookup split on '-'/'_' and fell straight to the base. Normalize '-'→'_' and match an exact supported dir first. Added the multi-region hyphen cases to the test (the prior fr-FR case passed only because fr has a single region).
|
✅ Fix round complete.
CI green, all review threads resolved. Ready for review/merge — not self-merging. |
What & why
Reported from production console:
GET …/src/translations/es/wp-admin-landing-shared.json 404(andfr, etc.).Two bugs in the i18n config, confirmed by reproducing the actual network requests per locale:
load: 'all'requested both the full localees_ES(exists → 200) and the base languagees(no such dir → 404). Page worked, but logged a 404 each load.es,fr) requested only that → 404 → silent English fallback instead of the user's language.Fix
load: 'currentOnly'— request only the exact locale (our locale files are key-complete perlint-translations, so no region→base fallback layer is needed).loadPathis now a function that maps the requested code to an available directory viaresolveLocaleDir(shortes→es_ES,pt→pt_BR,zh→zh_CN; unknown → passthrough → 404 → bundled English).src/shared/locale-resolve.mjs(tests/locale-resolve.test.mjs).Verified
Reproduced before/after with a headless harness — every locale now makes exactly one request to the correct dir; short codes resolve to their region; the
es/fr404s are gone.Please review before merge — I'm not self-merging these.
🤖 Generated with Claude Code
Summary by CodeRabbit