refactor: migrate i18n.js to TypeScript as i18n.ts #601
Conversation
WalkthroughThe changes replace the JavaScript implementation of the internationalization (i18n) module in Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/simulator/src/i18n.ts (4)
3-5: Refine typing forwindow.locale.
Avoid castingwindowtoany. Extend the globalWindowinterface in a.d.tsfile or inline to declarelocale?: string. Then you can safely usewindow.locale ?? 'en'withoutany:declare global { interface Window { locale?: string } } const userLocale = window.locale ?? 'en'
9-11: RenamefinalFallbacktofallbackLocale.
fallbackLocaleis more descriptive thanfinalFallback. For clarity, update the constant name and its usages:- const finalFallback = 'en' + const fallbackLocale = 'en'
12-16: Use JSON module imports instead ofrequire.
Usingrequirefor JSON requiresresolveJsonModuleandesModuleInterop. Consider importing statically:import enMessages from './i18n/en.json' const messages: Record<string, any> = { [fallbackLocale]: enMessages, }This aligns with ES module semantics and improves bundler compatibility.
24-26: Annotatebananatype and export.
Explicitly type thebananavariable (const banana: Banana = new Banana()) and consider exporting its type to aid downstream consumers:import Banana from 'banana-i18n' export type { Banana } const banana: Banana = new Banana() // ... export default banana
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/simulator/src/i18n.js(0 hunks)src/simulator/src/i18n.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/simulator/src/i18n.js
🔇 Additional comments (2)
src/simulator/src/i18n.ts (2)
1-2: Verify default import frombanana-i18n.
The code uses a default import; ensure the library’s typings supportimport Banana from 'banana-i18n'and thatesModuleInterop/allowSyntheticDefaultImportsare correctly configured.
6-8: Review load order:setLocalebeforeload.
You callbanana.setLocale(userLocale)before loading any messages. Verify that the library handles this order correctly or consider movingsetLocaleafterbanana.load(messages)to ensure translations are registered before the locale is applied.
| // Try to load user's locale file | ||
| try { | ||
| messages[locale] = require(`./i18n/${locale}.json`) | ||
| } catch (err) { | ||
| console.warn(`Could not load locale file for ${locale}, falling back to ${finalFallback}.`) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set fallback locale on load failure.
If loading the user locale fails, banana.locale remains the unavailable locale. Reset it to the fallback to avoid missing translations at runtime:
try {
messages[locale] = require(`./i18n/${locale}.json`)
} catch (err) {
console.warn(`Could not load locale file for ${locale}, falling back to ${fallbackLocale}.`)
+ banana.setLocale(fallbackLocale)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/simulator/src/i18n.ts around lines 17 to 23, when loading the user's
locale file fails, the code only logs a warning but does not reset the locale to
the fallback. Modify the catch block to set banana.locale to finalFallback after
logging the warning, ensuring the application uses the fallback locale and
avoids missing translations at runtime.
|
@Arnabdaz @niladrix719 pls review the pr , thanks |
Fixes #414
Describe the changes you have made in this PR -
converted the i18n.js file to i18n.ts file as asked in issue
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit