-
Notifications
You must be signed in to change notification settings - Fork 5
WIP: Add Lingo i18n support (BREAKING) #305
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
Added lingo.dev for multilingual support with Spanish translation: - Installed lingo.dev compiler and React provider - Configured Vite build to compile translations - Added LingoProviderWrapper to main.tsx - Added LocaleSwitcher to TopNav (desktop + mobile) KNOWN ISSUE: Lingo wraps text nodes which breaks Radix UI components that use refs internally (Dialog, DropdownMenu, AlertDialog). This causes the profile dialog and other interactive components to not work. Need to either: 1. Find a way to exclude Radix components from translation 2. Use a different i18n solution 3. Accept the limitations and manually mark problem components Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Deploying maple with
|
| Latest commit: |
d6708b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5a32055e.maple-ca8.pages.dev |
| Branch Preview URL: | https://lingo.maple-ca8.pages.dev |
WalkthroughAdding internationalization support through the lingo.dev library. Changes include adding the dependency, wrapping the App component with LingoProviderWrapper, integrating a LocaleSwitcher UI component into navigation, and configuring the Vite compiler for locale management targeting English and Spanish. Changes
Sequence DiagramsequenceDiagram
participant Browser
participant App as App Component
participant LingoProvider as LingoProviderWrapper
participant Dictionary as Dictionary Loader
participant LocaleSwitcher as LocaleSwitcher UI
Browser->>App: Initialize application
App->>LingoProvider: Wrap App with provider
LingoProvider->>Dictionary: Load initial locale (en)
Dictionary-->>LingoProvider: Dictionary loaded
LingoProvider-->>App: Locale context ready
LocaleSwitcher->>LingoProvider: User selects locale (es)
LingoProvider->>Dictionary: loadDictionary(es)
Dictionary-->>LingoProvider: Spanish dictionary loaded
LingoProvider-->>App: Update locale context
App->>Browser: Re-render with new locale
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Greptile OverviewGreptile SummaryThis PR adds multilingual support using lingo.dev with Spanish translations. The implementation includes:
Critical Issue (Acknowledged by Author): The Lingo text-wrapping approach fundamentally breaks Radix UI components. When Lingo wraps text nodes, it interferes with ref forwarding that Radix components (Dialog, DropdownMenu, AlertDialog) depend on for positioning and functionality. This makes critical UI features like the profile dialog and dropdown menus unusable. Recommendation: As noted in the PR description, this should NOT be merged in its current state. Consider these alternatives:
The integration is technically clean otherwise, but the core architectural incompatibility with Radix UI makes this approach non-viable for production. Confidence Score: 0/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant Main as main.tsx
participant Lingo as LingoProviderWrapper
participant App as App Component
participant TopNav
participant Switcher as LocaleSwitcher
participant Dict as dictionary.js
User->>Browser: Load Application
Browser->>Main: Initialize App
Main->>Lingo: Wrap with LingoProviderWrapper
Lingo->>Dict: loadDictionary(locale)
Dict-->>Lingo: Return translations
Lingo->>App: Render with translation context
App->>TopNav: Render navigation
TopNav->>Switcher: Display LocaleSwitcher(en, es)
Note over Lingo,App: ⚠️ Lingo wraps all text nodes<br/>Breaking Radix UI refs
User->>Switcher: Click language option
Switcher->>Lingo: Change locale to 'es'
Lingo->>Dict: loadDictionary('es')
Dict-->>Lingo: Spanish translations
Lingo->>App: Re-render with Spanish
User->>App: Try to open Dialog/Dropdown
App->>App: ❌ Ref forwarding fails
Note over App: Components don't open due to<br/>text wrapping interference
|
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.
6 files reviewed, 3 comments
| lingoCompiler.vite({ | ||
| sourceRoot: "src", | ||
| targetLocales: ["es"], | ||
| models: "lingo.dev" | ||
| })({ |
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.
style: the Lingo compiler wraps the entire Vite config, which may complicate debugging and maintenance
Consider a more explicit composition pattern:
| lingoCompiler.vite({ | |
| sourceRoot: "src", | |
| targetLocales: ["es"], | |
| models: "lingo.dev" | |
| })({ | |
| export default defineConfig(() => { | |
| const baseConfig = { | |
| plugins: [TanStackRouterVite(), react(), derPlugin()], | |
| resolve: { | |
| alias: { | |
| "@": path.resolve(__dirname, "./src") | |
| } | |
| } | |
| }; | |
| return lingoCompiler.vite({ | |
| sourceRoot: "src", | |
| targetLocales: ["es"], | |
| models: "lingo.dev" | |
| })(baseConfig); | |
| }) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/vite.config.ts
Line: 10:14
Comment:
**style:** the Lingo compiler wraps the entire Vite config, which may complicate debugging and maintenance
Consider a more explicit composition pattern:
```suggestion
export default defineConfig(() => {
const baseConfig = {
plugins: [TanStackRouterVite(), react(), derPlugin()],
resolve: {
alias: {
"@": path.resolve(__dirname, "./src")
}
}
};
return lingoCompiler.vite({
sourceRoot: "src",
targetLocales: ["es"],
models: "lingo.dev"
})(baseConfig);
})
```
How can I resolve this? If you propose a fix, please make it concise.| <LingoProviderWrapper loadDictionary={(locale) => loadDictionary(locale)}> | ||
| <App /> | ||
| </LingoProviderWrapper> | ||
| </StrictMode> |
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.
logic: wrapping text content breaks Radix UI's ref forwarding mechanism - Dialog, DropdownMenu, and AlertDialog components will fail to open
The PR description acknowledges this critical issue. Lingo wraps all text nodes which interferes with components that rely on refs for positioning and functionality.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/main.tsx
Line: 19:22
Comment:
**logic:** wrapping text content breaks Radix UI's ref forwarding mechanism - Dialog, DropdownMenu, and AlertDialog components will fail to open
The PR description acknowledges this critical issue. Lingo wraps all text nodes which interferes with components that rely on refs for positioning and functionality.
How can I resolve this? If you propose a fix, please make it concise.| <div className="flex items-center gap-4"> | ||
| {/* Language Switcher */} | ||
| <div className="hidden sm:block"> | ||
| <LocaleSwitcher locales={["en", "es"]} /> |
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.
style: hardcoded locale list limits scalability
| <LocaleSwitcher locales={["en", "es"]} /> | |
| <LocaleSwitcher locales={["en", "es", "fr", "de"]} /> |
Consider moving supported locales to a config file for easier maintenance as more languages are added.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/components/TopNav.tsx
Line: 71:71
Comment:
**style:** hardcoded locale list limits scalability
```suggestion
<LocaleSwitcher locales={["en", "es", "fr", "de"]} />
```
Consider moving supported locales to a config file for easier maintenance as more languages are added.
How can I resolve this? If you propose a fix, please make it concise.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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/main.tsx (1)
19-21: Simplify the loadDictionary prop.The arrow function wrapper
(locale) => loadDictionary(locale)is redundant. You can pass the function directly.Apply this diff to simplify:
- <LingoProviderWrapper loadDictionary={(locale) => loadDictionary(locale)}> + <LingoProviderWrapper loadDictionary={loadDictionary}> <App /> </LingoProviderWrapper>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
frontend/package.json(1 hunks)frontend/src/components/TopNav.tsx(3 hunks)frontend/src/main.tsx(2 hunks)frontend/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use 2-space indentation, double quotes, and a 100-character line limit for formatting
Use camelCase for variable and function names
Use try/catch with specific error types for error handling
Files:
frontend/src/components/TopNav.tsxfrontend/vite.config.tsfrontend/src/main.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript typing and avoid
anywhen possible
Files:
frontend/src/components/TopNav.tsxfrontend/vite.config.tsfrontend/src/main.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-19T21:31:44.925Z
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T21:31:44.925Z
Learning: Applies to src/**/*.{tsx} : Use React context for global state management when needed
Applied to files:
frontend/src/components/TopNav.tsx
🧬 Code graph analysis (2)
frontend/vite.config.ts (1)
frontend/vite-der-plugin.ts (1)
derPlugin(3-23)
frontend/src/main.tsx (1)
frontend/src/app.tsx (1)
App(50-112)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
frontend/package.json (1)
45-45: LGTM!The dependency addition is correctly formatted and positioned.
frontend/src/main.tsx (1)
5-5: LGTM!The import statement is correct for Lingo React client integration.
frontend/src/components/TopNav.tsx (4)
7-7: LGTM!The import statement is correct for the Lingo LocaleSwitcher component.
69-72: LocaleSwitcher configuration looks correct, but verify with Vite config.The desktop LocaleSwitcher is properly positioned and configured with
locales={["en", "es"]}. However, ensure this aligns with thetargetLocalesconfiguration invite.config.ts(currently only["es"]).
125-127: Mobile LocaleSwitcher implementation is correct, but verify with Vite config.The mobile LocaleSwitcher implementation follows the same pattern as the desktop version and includes appropriate styling. Ensure the
locales={["en", "es"]}configuration aligns withvite.config.ts.
1-134: Lingo integration carries unmitigated Radix UI compatibility risks—verify AccountMenu and nested dialogs.Lingo 0.114.5 is active in the codebase, but no workarounds (e.g.,
data-lingo-skip) are implemented. While TopNav.tsx's LocaleSwitcher integration itself is syntactically sound, the broader codebase has severe exposure:
- AccountMenu nests AlertDialog > Dialog > DropdownMenu (all Radix primitives)—highest risk for ref forwarding failures
- 20+ Dialog components, 6+ DropdownMenu, 10+ AlertDialog components across the app depend on proper ref handling
The PR objectives document that Lingo breaks these components' text wrapping and ref forwarding. Before merging, either:
- Implement
data-lingo-skipon all Radix UI wrapper components- Switch to an alternative i18n library
- Test AccountMenu and nested modals thoroughly to confirm no breakage
frontend/vite.config.ts (1)
10-13: Review comment is incorrect—configuration is properly aligned.The Lingo compiler configuration is actually correct. According to Lingo documentation, sourceLocale is the source language (default "en") and targetLocales is the array of locale codes you want generated.
The current setup:
sourceLocale: "en"(implicit default)targetLocales: ["es"](Spanish translations to be generated)LocaleSwitcher locales={["en", "es"]}(users can select source or target language)This is the intended design: the source locale ("en") is not included in
targetLocalesbecause source strings don't require compilation—only target languages do. Users can still switch to English via LocaleSwitcher since it's the active source locale. No runtime issues will occur.Likely an incorrect or invalid review comment.
Overview
Added lingo.dev for multilingual support with Spanish translation.
Changes
Lingo breaks Radix UI components! The text wrapping causes ref forwarding to fail in:
This makes critical UI features unusable.
Possible Solutions
data-lingo-skipTesting
Run
bun run devand try to:This PR is for documentation purposes to show the Lingo integration attempt and its issues.
Summary by CodeRabbit