fix: fetch a region-tagged locale's base tag when it's configured - #4124
fix: fetch a region-tagged locale's base tag when it's configured#4124Vincentdevreede wants to merge 3 commits into
Conversation
|
Note 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:
Walkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
This looks like a bug, at the same time I can imagine users are already defining their fallback locale files using the file arrays (e.g. Explicitly configuring fallbacks does work (example below), so I guess we only need to parse the codes and add these as fallbackLocales if such locales exist. // in nuxt.config.ts i18n config
export default defineNuxtConfig({
// ...
i18n: {
locales: [
{ code: 'en', language: 'en', file: 'en.json', name: 'English' },
{ code: 'en-US', language: 'en-US', file: 'en-US.json', name: 'English (US)' },
]
}
})// i18n.config.ts
export default defineI18nConfig(() => {
return {
fallbackLocale: {
'en-US': ['en'], // en.json is loaded when locale is en-US
},
}
})
This is true for some projects (with dynamic locale files or files that need to be run from client-side/nuxt), but most projects (static files) will fetch through the messages endpoint where the server already has the locale messages loaded/merged. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/shared/locales.ts (1)
39-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDeduplicate the final fallback chain.
Line 40 excludes base tags that are already requested locales. It does not exclude repeated derived base tags or a derived base tag that is also in
fallbackLocale.For example,
getFallbackLocaleCodes({ 'en-US': ['en'] }, ['en-US'])returns['en', 'en']. Two regional locales with the same configured base tag also repeat that base tag. Return an ordered unique list after all fallback sources are appended. Add regressions for both cases.Proposed fix
+function uniqueLocaleCodes(codes: string[]) { + return [...new Set(codes)] +} + export function getFallbackLocaleCodes(fallback: FallbackLocale, locales: string[]): string[] { const baseTags = locales.flatMap(configuredBaseTags).filter(tag => !locales.includes(tag)) - if (fallback === false) { return baseTags } - if (isArray(fallback)) { return [...baseTags, ...fallback] } + if (fallback === false) { return uniqueLocaleCodes(baseTags) } + if (isArray(fallback)) { return uniqueLocaleCodes([...baseTags, ...fallback]) } const fallbackLocales: string[] = [...baseTags] // ... - return fallbackLocales + return uniqueLocaleCodes(fallbackLocales) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/shared/locales.ts` around lines 39 - 56, Update getFallbackLocaleCodes to deduplicate the completed fallback chain after combining derived base tags, configured fallback values, and string/object fallbacks, while preserving first-occurrence order. Add regressions covering a fallback value duplicating a derived base tag and multiple regional locales producing the same base tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/runtime/shared/locales.ts`:
- Around line 39-56: Update getFallbackLocaleCodes to deduplicate the completed
fallback chain after combining derived base tags, configured fallback values,
and string/object fallbacks, while preserving first-occurrence order. Add
regressions covering a fallback value duplicating a derived base tag and
multiple regional locales producing the same base tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56dc3f3b-e046-4159-a66f-6e8cb95e51e6
📒 Files selected for processing (2)
src/runtime/shared/locales.tstest/locales.test.ts
Vincentdevreede
changed the title
|
@BobbieGoede Updated this to an actual fix rather than just a docs note. getFallbackLocaleCodes now adds a region-tagged locale's base tag as an implicit fallback when that base tag is itself configured (e.g. en alongside en-US), matching vue-i18n's own behavior. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/shared/locales.ts`:
- Around line 46-53: Update the conditional formatting in the fallback handling
block so each else if follows the preceding closing brace on the same line,
resolving the ESLint brace-style violations without changing behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1facb29a-e4ec-4408-8026-27f19e63ed82
📒 Files selected for processing (2)
src/runtime/shared/locales.tstest/locales.test.ts
5b9b4e2 to
7579b99
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
7579b99 to
8081eb3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@BobbieGoede Opened #4128 on top of this branch. |
2 participants
Summary
vue-i18n automatically falls back a region-tagged locale to its base language tag (
en-UStriesen) before it even looks atfallbackLocale. This is documented, default behavior in vue-i18n itself. Nuxt i18n's lazy loading never fetched that base tag's messages unless it was listed explicitly infallbackLocale, so this fallback silently did nothing for anyone with a region-tagged locale likeen-USalongsideen.getFallbackLocaleCodesnow adds a locale's base tag to its fallbacks when that base tag is itself a configured locale, walking multiple dash-separated levels where needed (de-DE-bavarian→de-DE→de). If the base tag isn't configured at all, nothing changes, there's no file to load for it and no reason to widen the lazy-loading footprint for a tag nobody defined.Also fixes a duplicate entry this introduced: an explicit
fallbackLocaleentry can name the same locale the base-tag walk already adds on its own (writing{'en-US': ['en']} achieves exactly whaten-US's implicitenalready does), so the result is deduped.SummaryVue I18n automatically falls back a region-tagged locale to its base language tag (fr-CA tries fr), but that only works for messages already loaded in memory. Since Nuxt i18n lazy-loads locale files, it only fetches what's explicitly listed in fallbackLocale, so that implicit chain silently does nothing unless the base tag is configured too.I ran into this and initially thought it was a bug, but on reflection I think it's probably intentional: automatically fetching a base tag's file on top of whatever's explicitly configured would mean loading more locale files than the user asked for, which cuts against the whole point of lazy loading. So instead of changing behavior, this PR just documents it, with a callout showing how to get the same effect by listing the base tag yourself in fallbackLocale.@BobbieGoede can you confirm that's the intended reasoning? If not, and this is actually considered a bug, let me know and I'll put together a code fix instead of the docs note.This is an older summary from before I force-pushed the changes. I’m just keeping it here for reference.
Summary by CodeRabbit
Bug Fixes
Tests