added i18n support for plugins #10182
Conversation
WalkthroughThe pull request introduces enhancements to the internationalization (i18n) configuration and app configuration management. In Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Config as careConfig
participant I18n as i18n Module
participant Backend as Translation Backend
App->>Config: Retrieve careApps
Config-->>App: Return app configurations
App->>I18n: Initialize with namespaces
I18n->>I18n: Generate namespace URLs
I18n->>Backend: Fetch translations dynamically
Backend-->>I18n: Return translation resources
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
khavinshankar
requested review from
gigincg,
bodhish,
rithviknishad and
Jacobjeevan
Deploying care-fe with
|
| Latest commit: |
dd280b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://580d9205.care-fe.pages.dev |
| Branch Preview URL: | https://plugs-i18n.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
plugs-i18n
|
| Run status |
|
| Run duration | 02m 53s |
| Commit |
|
| Committer | Khavin Shankar |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
5
|
| View all changes introduced in this branch ↗︎ | |
![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 (3)
src/i18n.ts (2)
17-27: Consider validating or defaulting empty input
Ifinputis empty or whitespace only, the function returns"https://"—which may fail later. A fallback or validation for an empty string can help avoid such errors.
43-63: Optionally provide a fallback for missing translations
If translation fetch fails, the callback passes an error, causing the i18n load to break. Returning an empty object can keep the UI partially functional rather than halting.Possible fallback approach:
.catch((error) => { console.error( `Failed to load translations for ${language}/${namespace}:`, error, ); - callback(error, null); + callback(null, {}); });care.config.ts (1)
124-124: Safeguard against invalid string patterns
Splitting the string twice may fail if the app name doesn't followorganization/app@branch. Consider a fallback or error handling if a slash is missing.- name: app.split("@")[0].split("/")[1], + const [org, appName] = app.split("@")[0].split("/"); + name: appName || org,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
care.config.ts(1 hunks)package.json(2 hunks)src/i18n.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (6)
src/i18n.ts (4)
1-1: No issues with this import
Essential import for referencing thecareConfig.
5-5: No issues with the new dependency import
TheresourcesToBackendimport is properly integrated and aligns with the i18n enhancement.
66-66: Dynamic namespaces are well implemented
Adding...careConfig.careApps.map(app => app.name)ensures robust multi-app translation support.
73-73: Default namespace choice looks good
SettingdefaultNSto"care_fe"is consistent with your existing codebase.package.json (2)
103-103: New i18n resource dependency
Good addition for enabling dynamic resource loading.
142-142: Incremental type definition update
Updating@types/nodeto^22.10.10is generally safe.
| const namespaceToUrl = (namespace: string) => { | ||
| const careApp = careConfig.careApps.find((app) => app.name === namespace); | ||
|
|
||
| if (!careApp) { | ||
| return ""; | ||
| } | ||
|
|
||
| return toURL(careApp.branch); | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Confirmed: Missing branch handling will cause runtime errors
The code has a critical issue where careApp.branch can be undefined when using GitHub Pages format ("organization/repository"), causing toURL(undefined) to throw runtime errors. This needs to be fixed as it affects the application's stability.
- In
src/i18n.ts: Add null check and fallback forcareApp.branchbefore passing totoURL - Consider adding validation for required branch field in care.config.ts
🔗 Analysis chain
Handle potential missing or empty branch field
If careApp.branch is empty or undefined, returning https://undefined could lead to failed translations fetching. Recommend a fallback or an explicit error message.
Please confirm branch is always valid:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Checking if `branch` is guaranteed for all items in careApps
rg 'careApps(\s+)?(:|=)' -A 10
Length of output: 1974
Script:
#!/bin/bash
# Check for REACT_ENABLED_APPS validation and other branch usages
rg "REACT_ENABLED_APPS" -A 5
rg "careApp.*branch" -A 2
Length of output: 2001
Script:
#!/bin/bash
# Check toURL implementation
rg "function toURL|const toURL" -A 5
Length of output: 323
Proposed Changes
Summary by CodeRabbit
Release Notes
New Features
Dependencies
i18next-resources-to-backendpackage@types/nodeto latest versionImprovements