-
-
Notifications
You must be signed in to change notification settings - Fork 660
fix: fix i18n key fallback logic #7968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
COMPARE TO
|
| Name | Diff |
|---|---|
| packages/core/src/middleware/koa-email-i18n.test.ts | 0 Bytes |
| packages/core/src/routes/well-known/well-known.phrases.test.ts | 📈 +3 Bytes |
| packages/core/src/utils/i18n.test.ts | 📈 +1.82 KB |
| packages/core/src/utils/i18n.ts | 📈 +325 Bytes |
| packages/phrases-experience/src/index.ts | 📈 +60 Bytes |
| packages/phrases/src/index.ts | 📈 +52 Bytes |
| packages/toolkit/language-kit/package.json | 📈 +40 Bytes |
| packages/toolkit/language-kit/src/utility.test.ts | 📈 +2.24 KB |
| packages/toolkit/language-kit/src/utility.ts | 📈 +4.19 KB |
| pnpm-lock.yaml | 📈 +81 Bytes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the i18n language tag fallback logic to improve language matching and fallback behavior. The key improvement is the addition of canonical locale normalization using Intl.getCanonicalLocales(), which enables better matching between language variants (e.g., pl can now correctly fallback to pl-PL).
Key Changes:
- Added
findSupportedLanguageTagandmatchSupportedLanguageTagfunctions in@logto/language-kitto centralize language matching logic with improved fallback support - Refactored
getDefaultLanguageTagfunctions in both phrases packages to use the new matching logic instead of the previousfallbackutility - Updated
getExperienceLanguagein core to prioritize custom languages before falling back to built-in languages
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds @silverhand/essentials dependency for language-kit package |
| packages/toolkit/language-kit/package.json | Adds @silverhand/essentials as a dev dependency |
| packages/toolkit/language-kit/src/utility.ts | Implements new language matching functions with canonical locale normalization |
| packages/toolkit/language-kit/src/utility.test.ts | Adds test coverage for new language matching functions |
| packages/phrases/src/index.ts | Refactors getDefaultLanguageTag to use new findSupportedLanguageTag function |
| packages/phrases-experience/src/index.ts | Refactors getDefaultLanguageTag to use new findSupportedLanguageTag function |
| packages/core/src/utils/i18n.ts | Updates getExperienceLanguage to use new language matching functions with separate custom/built-in language handling |
| packages/core/src/utils/i18n.test.ts | Adds test coverage for updated getExperienceLanguage function |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
|
|
||
| const base = canonical.split('-')[0] ?? canonical; |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression canonical.split('-')[0] ?? canonical is redundant. split()[0] will always return a string (at minimum, the original string if no delimiter is found), so it will never be nullish. The nullish coalescing operator ?? has no effect here. Consider simplifying to canonical.split('-')[0].
| const base = canonical.split('-')[0] ?? canonical; | |
| const base = canonical.split('-')[0]; |
| return directMatch; | ||
| } | ||
|
|
||
| const base = canonicalPreferred.split('-')[0] ?? canonicalPreferred; |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression canonicalPreferred.split('-')[0] ?? canonicalPreferred is redundant. split()[0] will always return a string (at minimum, the original string if no delimiter is found), so it will never be nullish. The nullish coalescing operator ?? has no effect here. Consider simplifying to canonicalPreferred.split('-')[0].
| const base = canonicalPreferred.split('-')[0] ?? canonicalPreferred; | |
| const base = canonicalPreferred.split('-')[0]; |
| if (!base) { | ||
| continue; | ||
| } | ||
|
|
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition if (!base) is unreachable. Since base is assigned from split('-')[0], it will always be a non-empty string (at minimum, the value of canonicalPreferred itself). This check can be removed.
| if (!base) { | |
| continue; | |
| } |
ead5cb5 to
678f5bb
Compare
charIeszhao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. But please take a look at the copilot comments
Summary
Fixed issues with i18n language tag fallback logic (resolves LOG-12467). Key improvements include:
Testing
Checklist
.changeset