Merged
Conversation
📝 WalkthroughWalkthroughThis refactoring converts the i18n system from dynamic runtime language loading to static resource initialization. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Size Change: +316 kB (+4.35%) Total Size: 7.6 MB
ℹ️ View Unchanged
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/vechain-kit/src/utils/i18n.ts (1)
12-28: Remove duplicated branch in bundle initialization.Both
ifandelseexecute the sameaddResourceBundlecall, so thehasNamespaceconditional is redundant and can be collapsed.♻️ Suggested simplification
- if (!hasNamespace) { - i18nInstance.addResourceBundle( - lang, - 'translation', - resources[lang as keyof typeof resources].translation, - true, - true, - ); - } else { - i18nInstance.addResourceBundle( - lang, - 'translation', - resources[lang as keyof typeof resources].translation, - true, - true, - ); - } + i18nInstance.addResourceBundle( + lang, + 'translation', + resources[lang as keyof typeof resources].translation, + true, + true, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vechain-kit/src/utils/i18n.ts` around lines 12 - 28, The if/else around hasNamespace is redundant because both branches call i18nInstance.addResourceBundle with the same arguments; remove the conditional and invoke i18nInstance.addResourceBundle(lang, 'translation', resources[lang as keyof typeof resources].translation, true, true) once. Keep references to hasNamespace, i18nInstance.addResourceBundle, and resources when editing so you update the correct initialization logic in i18n.ts.packages/vechain-kit/i18n.ts (1)
22-45: DerivesupportedLanguagesfromresourcesto prevent drift.These are now two separate sources of truth. Deriving the language list from
resourcesavoids future mismatches.♻️ Suggested change
-export const supportedLanguages = [ - 'en', 'de', 'it', 'fr', 'es', 'zh', 'ja', 'ru', 'ro', - 'vi', 'nl', 'ko', 'sv', 'tw', 'tr', 'hi', 'pt', -]; - export const resources = { ... }; + +export const supportedLanguages = Object.keys(resources);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vechain-kit/i18n.ts` around lines 22 - 45, The supportedLanguages array is a separate source of truth from resources and can drift; replace the hardcoded supportedLanguages with a derived value from resources (e.g., compute supportedLanguages using Object.keys(resources) or similar) so the list always reflects the resource keys; update any exports/types that rely on supportedLanguages accordingly and keep the symbol names supportedLanguages and resources unchanged so callers continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vechain-kit/src/providers/VeChainKitProvider.tsx`:
- Around line 454-459: Normalize and validate language inputs before syncing
state or calling i18n: introduce or use a supportedLanguages (or
defaultLanguage) set and a small normalizer function (e.g., normalizeLanguage)
that maps variants like "en-US" -> "en" and returns a fallback if unsupported;
apply this normalizer when reading storedLanguage (from
getLocalStorageItem('i18nextLng')), when initializing currentLanguage from the
language prop, and inside setLanguage(lang) before updating state or calling
i18n.changeLanguage so only validated/normalized codes are stored and used.
- Around line 460-467: The update guard is being cleared synchronously before
the async i18n.changeLanguage completes; keep isUpdatingFromPropRef.current true
until the Promise resolves by moving the assignment into the Promise.finally
handler: when you set isUpdatingFromPropRef.current = true around
i18n.changeLanguage(...) (in the blocks that call i18n.changeLanguage and then
possibly call setCurrentLanguageState), chain .finally(() => {
isUpdatingFromPropRef.current = false; }) so the guard remains active until the
language change completes; apply this pattern to all occurrences that call
i18n.changeLanguage (the block using
isUpdatingFromPropRef/currentLanguage/setCurrentLanguageState and the two other
locations noted).
---
Nitpick comments:
In `@packages/vechain-kit/i18n.ts`:
- Around line 22-45: The supportedLanguages array is a separate source of truth
from resources and can drift; replace the hardcoded supportedLanguages with a
derived value from resources (e.g., compute supportedLanguages using
Object.keys(resources) or similar) so the list always reflects the resource
keys; update any exports/types that rely on supportedLanguages accordingly and
keep the symbol names supportedLanguages and resources unchanged so callers
continue to work.
In `@packages/vechain-kit/src/utils/i18n.ts`:
- Around line 12-28: The if/else around hasNamespace is redundant because both
branches call i18nInstance.addResourceBundle with the same arguments; remove the
conditional and invoke i18nInstance.addResourceBundle(lang, 'translation',
resources[lang as keyof typeof resources].translation, true, true) once. Keep
references to hasNamespace, i18nInstance.addResourceBundle, and resources when
editing so you update the correct initialization logic in i18n.ts.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
packages/vechain-kit/i18n.tspackages/vechain-kit/package.jsonpackages/vechain-kit/src/components/AccountModal/Contents/FAQ/FAQContent.tsxpackages/vechain-kit/src/components/AccountModal/Contents/KitSettings/LanguageSettingsContent.tsxpackages/vechain-kit/src/providers/VeChainKitProvider.tsxpackages/vechain-kit/src/utils/i18n.tspackages/vechain-kit/tsdown.config.ts
💤 Files with no reviewable changes (1)
- packages/vechain-kit/tsdown.config.ts
Comment on lines
+454
to
459
| const storedLanguage = | ||
| typeof window !== 'undefined' | ||
| ? getLocalStorageItem('i18nextLng') | ||
| : null; | ||
| const initialLanguage = storedLanguage || currentLanguage; | ||
|
|
There was a problem hiding this comment.
Normalize language inputs before syncing state/i18n.
storedLanguage, prop language, and setLanguage(lang) are used as-is. Unsupported codes can move currentLanguage to a value with no resource bundle and break selector consistency.
🛡️ Suggested guard for supported languages
+import i18n, { supportedLanguages } from '../../i18n';
...
+const normalizeLanguage = (lng: string | null | undefined) =>
+ lng && supportedLanguages.includes(lng) ? lng : 'en';
...
-const initialLanguage = storedLanguage || currentLanguage;
+const initialLanguage = normalizeLanguage(storedLanguage || currentLanguage);
...
-if (language && !storedLanguage && language !== currentLanguage) {
+const normalizedPropLanguage = normalizeLanguage(language);
+if (!storedLanguage && normalizedPropLanguage !== currentLanguage) {
isUpdatingFromPropRef.current = true;
- i18n.changeLanguage(language);
- setCurrentLanguageState(language);
+ i18n.changeLanguage(normalizedPropLanguage);
+ setCurrentLanguageState(normalizedPropLanguage);
isUpdatingFromPropRef.current = false;
}
...
-const setLanguage = (lang: string) => {
+const setLanguage = (lang: string) => {
+ const normalized = normalizeLanguage(lang);
isUpdatingFromPropRef.current = true;
- i18n.changeLanguage(lang);
- setCurrentLanguageState(lang);
+ i18n.changeLanguage(normalized);
+ setCurrentLanguageState(normalized);
isUpdatingFromPropRef.current = false;
};Also applies to: 477-482, 554-559
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vechain-kit/src/providers/VeChainKitProvider.tsx` around lines 454 -
459, Normalize and validate language inputs before syncing state or calling
i18n: introduce or use a supportedLanguages (or defaultLanguage) set and a small
normalizer function (e.g., normalizeLanguage) that maps variants like "en-US" ->
"en" and returns a fallback if unsupported; apply this normalizer when reading
storedLanguage (from getLocalStorageItem('i18nextLng')), when initializing
currentLanguage from the language prop, and inside setLanguage(lang) before
updating state or calling i18n.changeLanguage so only validated/normalized codes
are stored and used.
Comment on lines
+460
to
+467
| if (initialLanguage && i18n.language !== initialLanguage) { | ||
| isUpdatingFromPropRef.current = true; | ||
| i18n.changeLanguage(initialLanguage); | ||
| if (initialLanguage !== currentLanguage) { | ||
| setCurrentLanguageState(initialLanguage); | ||
| } | ||
| isUpdatingFromPropRef.current = false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/vechain-kit/src/providers && head -500 VeChainKitProvider.tsx | tail -100Repository: vechain/vechain-kit
Length of output: 3547
🏁 Script executed:
cat -n packages/vechain-kit/src/providers/VeChainKitProvider.tsx | sed -n '450,470p'Repository: vechain/vechain-kit
Length of output: 956
🏁 Script executed:
cat -n packages/vechain-kit/src/providers/VeChainKitProvider.tsx | sed -n '470,490p'Repository: vechain/vechain-kit
Length of output: 1086
🏁 Script executed:
cat -n packages/vechain-kit/src/providers/VeChainKitProvider.tsx | sed -n '545,565p'Repository: vechain/vechain-kit
Length of output: 1010
🌐 Web query:
i18next v24 changeLanguage return type Promise documentation
💡 Result:
In i18next v24, changeLanguage is documented as:
i18next.changeLanguage(lng, callback) // -> returns a Promise[1]
And the docs’ Promise example shows the Promise resolves with the t function, i.e. effectively:
- Return type:
Promise<TFunction>[1]
Background (why it returns a Promise at all): i18next changed these async APIs (including changeLanguage) to “also return a Promise” starting in v12+ [2].
Sources: [1] [2]
Wrap changeLanguage with .finally() to keep the update guard active until the async operation resolves.
In i18next v24, changeLanguage returns a Promise<TFunction>. The current code clears isUpdatingFromPropRef synchronously, causing the guard to drop before the language change completes. Since the languageChanged listener checks this guard to prevent loops, clearing it early allows unwanted onLanguageChange callbacks to fire.
Apply this pattern to all three locations:
Corrected pattern
- i18n.changeLanguage(initialLanguage);
- ...
- isUpdatingFromPropRef.current = false;
+ void i18n.changeLanguage(initialLanguage).finally(() => {
+ isUpdatingFromPropRef.current = false;
+ });Affects lines 460–467, 479–482, and 556–559.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (initialLanguage && i18n.language !== initialLanguage) { | |
| isUpdatingFromPropRef.current = true; | |
| i18n.changeLanguage(initialLanguage); | |
| if (initialLanguage !== currentLanguage) { | |
| setCurrentLanguageState(initialLanguage); | |
| } | |
| isUpdatingFromPropRef.current = false; | |
| } | |
| if (initialLanguage && i18n.language !== initialLanguage) { | |
| isUpdatingFromPropRef.current = true; | |
| void i18n.changeLanguage(initialLanguage).finally(() => { | |
| isUpdatingFromPropRef.current = false; | |
| }); | |
| if (initialLanguage !== currentLanguage) { | |
| setCurrentLanguageState(initialLanguage); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vechain-kit/src/providers/VeChainKitProvider.tsx` around lines 460 -
467, The update guard is being cleared synchronously before the async
i18n.changeLanguage completes; keep isUpdatingFromPropRef.current true until the
Promise resolves by moving the assignment into the Promise.finally handler: when
you set isUpdatingFromPropRef.current = true around i18n.changeLanguage(...) (in
the blocks that call i18n.changeLanguage and then possibly call
setCurrentLanguageState), chain .finally(() => { isUpdatingFromPropRef.current =
false; }) so the guard remains active until the language change completes; apply
this pattern to all occurrences that call i18n.changeLanguage (the block using
isUpdatingFromPropRef/currentLanguage/setCurrentLanguageState and the two other
locations noted).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removed dyanmic language loading since it was causing issues with vite
Summary by CodeRabbit
Release Notes