feat: add Simplified Chinese (zh-CN) language support#78
Open
Leck88 wants to merge 4 commits into
Open
Conversation
Contributor
|
Someone is attempting to deploy a commit to the ravixalgorithm's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds English and Chinese dictionaries and a getDictionary registry, a client LocaleContext/LocaleProvider with hooks, server RootLayout locale bootstrapping, client LanguageSwitcher/Nav/Footer localization, middleware x-locale header, graceful DB/auth startup paths, constants update, and dependency/npmrc changes. Changesi18n core & provider
Client UI localization
Middleware
DB and auth startup safety
Constants and dependencies
Sequence Diagram(s)sequenceDiagram
participant User as User (UI)
participant LangSwitch as LanguageSwitcher
participant Router as Next.js Router
participant Provider as LocaleProvider
participant Cookie as NEXT_LOCALE cookie
User->>LangSwitch: select locale
LangSwitch->>Router: push new pathname (with locale segment)
LangSwitch->>Cookie: set NEXT_LOCALE (path=/, max-age=1y)
Router->>Provider: render with new locale (via RootLayout or client state)
Provider->>Provider: getDictionary(newLocale) and update dictionary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/constants.ts (1)
1-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove
NAV_ITEMSconstant—it is unused.The
NAV_ITEMSexport inlib/constants.tsis no longer referenced anywhere in the codebase. SinceNavItems.tsxnow builds navigation fromdict.navinstead, this constant can be safely deleted to reduce clutter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/constants.ts` around lines 1 - 6, Remove the unused exported constant NAV_ITEMS from the file (delete the NAV_ITEMS declaration/export), and verify there are no remaining imports or references to NAV_ITEMS elsewhere in the codebase (e.g., any files that might still import NAV_ITEMS); if any are found, remove those imports/usages or update them to use dict.nav instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/layout.tsx`:
- Around line 25-26: Replace the duplicated locale definitions in app/layout.tsx
by importing the centralized constants from the i18n module: remove the local
const locales and const defaultLocale and instead import the exported symbols
(e.g., locales and defaultLocale or locales and derive default from i18n) from
i18n/index.ts (the file that defines dictionaries and exports locales), then use
those imported names in layout.tsx (referencing the existing locales and
defaultLocale identifiers in your layout component).
In `@components/LanguageSwitcher.tsx`:
- Around line 36-49: Replace the hardcoded SelectItem entries with a dynamic map
over your i18n config: import supportedLocales and localeNames from
i18n/index.ts, then inside the SelectContent iterate supportedLocales to render
a SelectItem for each locale using the locale string for the value and
localeNames[locale] (or a fallback) for the display text; keep existing
props/handlers (locale, handleLocaleChange, isPending,
SelectTrigger/SelectValue) and preserve the current className/styling on
SelectItem and SelectContent.
- Around line 21-33: The handler handleLocaleChange currently hardcodes locales,
manually mutates cookies, and does fragile path segment logic; replace this by
importing and calling setLocale from LocaleProvider (which updates state and
persists NEXT_LOCALE) and use supportedLocales from i18n to detect/validate
current locale in the path, then compute the new pathname robustly (handling
empty paths/trailing slashes) and call router.push with the updated path; remove
the direct document.cookie write and the hardcoded 'en'/'zh-CN' checks so locale
logic is centralized in setLocale/supportedLocales.
In `@components/LocaleProvider.tsx`:
- Line 35: The NEXT_LOCALE cookie assignment in LocaleProvider (document.cookie
in components/LocaleProvider.tsx) is missing a SameSite attribute; update the
cookie string in the locale setter (where document.cookie =
`NEXT_LOCALE=${newLocale}...`) to include a SameSite attribute (e.g., append
`;SameSite=Lax`) and, if you need cross-site usage, use `SameSite=None;Secure`
instead to ensure the cookie is sent securely.
In `@middleware/index.ts`:
- Around line 5-6: The locales array and defaultLocale constant are duplicated
here; remove the local definitions of locales and defaultLocale and import them
from the centralized i18n module (exported from i18n/index.ts), then update any
references in this file to use the imported symbols (locales, defaultLocale) so
the middleware reuses the single source of truth.
---
Outside diff comments:
In `@lib/constants.ts`:
- Around line 1-6: Remove the unused exported constant NAV_ITEMS from the file
(delete the NAV_ITEMS declaration/export), and verify there are no remaining
imports or references to NAV_ITEMS elsewhere in the codebase (e.g., any files
that might still import NAV_ITEMS); if any are found, remove those
imports/usages or update them to use dict.nav instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e7ef71d-70c7-4ef7-9214-9c54df163e14
📒 Files selected for processing (12)
app/layout.tsxcomponents/Footer.tsxcomponents/LanguageSwitcher.tsxcomponents/LocaleProvider.tsxcomponents/NavItems.tsxhooks/useDictionary.tshooks/useLocale.tsi18n/en.tsi18n/index.tsi18n/zh-CN.tslib/constants.tsmiddleware/index.ts
Comment on lines
+25
to
+26
| const locales: Locale[] = ['en', 'zh-CN']; | ||
| const defaultLocale: Locale = 'en'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Eliminate duplication: import locale constants from i18n module.
The locales array and defaultLocale are duplicated from i18n/index.ts. This violates DRY and creates a maintenance risk where the two definitions could diverge.
♻️ Proposed refactor to use centralized constants
-const locales: Locale[] = ['en', 'zh-CN'];
-const defaultLocale: Locale = 'en';
+import { defaultLocale } from '@/i18n';
+
+const locales: Locale[] = ['en', 'zh-CN']; // Note: consider exporting this from i18n/index.ts as wellOr better yet, if you export locales from i18n/index.ts:
-const locales: Locale[] = ['en', 'zh-CN'];
-const defaultLocale: Locale = 'en';
+import { defaultLocale } from '@/i18n';
+import type { Locale } from '@/i18n';
+
+// Derive locales from dictionaries keys
+const locales = Object.keys(dictionaries) as Locale[];However, the cleanest approach is to export a locales array from i18n/index.ts:
In i18n/index.ts:
export const locales = Object.keys(dictionaries) as Locale[];Then in layout.tsx:
-const locales: Locale[] = ['en', 'zh-CN'];
-const defaultLocale: Locale = 'en';
+import { defaultLocale, dictionaries } from '@/i18n';
+import type { Locale } from '@/i18n';
+
+const locales = Object.keys(dictionaries) as Locale[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/layout.tsx` around lines 25 - 26, Replace the duplicated locale
definitions in app/layout.tsx by importing the centralized constants from the
i18n module: remove the local const locales and const defaultLocale and instead
import the exported symbols (e.g., locales and defaultLocale or locales and
derive default from i18n) from i18n/index.ts (the file that defines dictionaries
and exports locales), then use those imported names in layout.tsx (referencing
the existing locales and defaultLocale identifiers in your layout component).
Comment on lines
+21
to
+33
| const handleLocaleChange = (newLocale: string) => { | ||
| startTransition(() => { | ||
| const segments = pathname.split('/'); | ||
| // Remove existing locale segment if present | ||
| if (segments[1] === 'en' || segments[1] === 'zh-CN') { | ||
| segments[1] = newLocale; | ||
| } else { | ||
| segments.splice(1, 0, newLocale); | ||
| } | ||
| router.push(segments.join('/')); | ||
| document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`; | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Refactor to use setLocale from LocaleProvider and eliminate hardcoded locale values.
This implementation has several maintainability concerns:
-
Hardcoded locale values (line 25): The explicit checks for
'en'and'zh-CN'duplicate the locale configuration that should be centralized ini18n/index.ts. When new locales are added, this code must be manually updated. -
Manual cookie manipulation (line 31): According to the review context,
LocaleProvideralready exports asetLocalefunction that "updates both state and persists the NEXT_LOCALE cookie with one-year max age." Manually setting the cookie here bypasses that contract and creates two separate cookie-setting paths. -
Fragile path manipulation (lines 23-30): The logic assumes
segments[1]is always safe to access and replace, but doesn't handle edge cases like empty paths, trailing slashes, or already-localized nested routes.
♻️ Recommended refactoring approach
Import setLocale from the LocaleProvider context and supportedLocales from i18n configuration:
+import { useContext } from 'react';
+import { LocaleContext } from '@/components/LocaleProvider';
+import { supportedLocales } from '@/i18n';Then simplify the handler to delegate locale persistence and path logic:
+const { setLocale } = useContext(LocaleContext);
+
const handleLocaleChange = (newLocale: string) => {
startTransition(() => {
+ setLocale(newLocale);
const segments = pathname.split('/');
- // Remove existing locale segment if present
- if (segments[1] === 'en' || segments[1] === 'zh-CN') {
+ if (supportedLocales.includes(segments[1] as any)) {
segments[1] = newLocale;
} else {
segments.splice(1, 0, newLocale);
}
router.push(segments.join('/'));
- document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`;
});
};This ensures a single source of truth for supported locales and consistent cookie handling.
📝 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
| const handleLocaleChange = (newLocale: string) => { | |
| startTransition(() => { | |
| const segments = pathname.split('/'); | |
| // Remove existing locale segment if present | |
| if (segments[1] === 'en' || segments[1] === 'zh-CN') { | |
| segments[1] = newLocale; | |
| } else { | |
| segments.splice(1, 0, newLocale); | |
| } | |
| router.push(segments.join('/')); | |
| document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`; | |
| }); | |
| }; | |
| import { useContext } from 'react'; | |
| import { LocaleContext } from '@/components/LocaleProvider'; | |
| import { supportedLocales } from '@/i18n'; | |
| // Add this line in the component before the handleLocaleChange function: | |
| const { setLocale } = useContext(LocaleContext); | |
| const handleLocaleChange = (newLocale: string) => { | |
| startTransition(() => { | |
| setLocale(newLocale); | |
| const segments = pathname.split('/'); | |
| if (supportedLocales.includes(segments[1] as any)) { | |
| segments[1] = newLocale; | |
| } else { | |
| segments.splice(1, 0, newLocale); | |
| } | |
| router.push(segments.join('/')); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/LanguageSwitcher.tsx` around lines 21 - 33, The handler
handleLocaleChange currently hardcodes locales, manually mutates cookies, and
does fragile path segment logic; replace this by importing and calling setLocale
from LocaleProvider (which updates state and persists NEXT_LOCALE) and use
supportedLocales from i18n to detect/validate current locale in the path, then
compute the new pathname robustly (handling empty paths/trailing slashes) and
call router.push with the updated path; remove the direct document.cookie write
and the hardcoded 'en'/'zh-CN' checks so locale logic is centralized in
setLocale/supportedLocales.
Comment on lines
+36
to
+49
| <Select value={locale} onValueChange={handleLocaleChange} disabled={isPending}> | ||
| <SelectTrigger className="w-[130px] h-9 bg-gray-700 border-gray-600 text-gray-300 hover:bg-gray-600 hover:text-white"> | ||
| <Globe className="h-4 w-4 mr-2" /> | ||
| <SelectValue placeholder="Language" /> | ||
| </SelectTrigger> | ||
| <SelectContent className="bg-gray-800 border-gray-700 text-gray-200"> | ||
| <SelectItem value="en" className="hover:bg-gray-700 focus:bg-gray-700"> | ||
| English | ||
| </SelectItem> | ||
| <SelectItem value="zh-CN" className="hover:bg-gray-700 focus:bg-gray-700"> | ||
| 简体中文 | ||
| </SelectItem> | ||
| </SelectContent> | ||
| </Select> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Render locale options dynamically from i18n configuration.
The SelectItem elements hardcode both locale values ("en", "zh-CN") and display labels ("English", "简体中文"). According to the review context, i18n/index.ts exports both supportedLocales and human-readable localeNames. Hardcoding these values creates a maintenance burden when adding new locales.
♻️ Proposed fix to render options dynamically
Import the i18n configuration:
+import { supportedLocales, localeNames } from '@/i18n';Replace the hardcoded SelectItem list with a mapped array:
<SelectContent className="bg-gray-800 border-gray-700 text-gray-200">
- <SelectItem value="en" className="hover:bg-gray-700 focus:bg-gray-700">
- English
- </SelectItem>
- <SelectItem value="zh-CN" className="hover:bg-gray-700 focus:bg-gray-700">
- 简体中文
- </SelectItem>
+ {supportedLocales.map((loc) => (
+ <SelectItem key={loc} value={loc} className="hover:bg-gray-700 focus:bg-gray-700">
+ {localeNames[loc]}
+ </SelectItem>
+ ))}
</SelectContent>📝 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
| <Select value={locale} onValueChange={handleLocaleChange} disabled={isPending}> | |
| <SelectTrigger className="w-[130px] h-9 bg-gray-700 border-gray-600 text-gray-300 hover:bg-gray-600 hover:text-white"> | |
| <Globe className="h-4 w-4 mr-2" /> | |
| <SelectValue placeholder="Language" /> | |
| </SelectTrigger> | |
| <SelectContent className="bg-gray-800 border-gray-700 text-gray-200"> | |
| <SelectItem value="en" className="hover:bg-gray-700 focus:bg-gray-700"> | |
| English | |
| </SelectItem> | |
| <SelectItem value="zh-CN" className="hover:bg-gray-700 focus:bg-gray-700"> | |
| 简体中文 | |
| </SelectItem> | |
| </SelectContent> | |
| </Select> | |
| import { supportedLocales, localeNames } from '@/i18n'; | |
| // ... other imports and component code ... | |
| <Select value={locale} onValueChange={handleLocaleChange} disabled={isPending}> | |
| <SelectTrigger className="w-[130px] h-9 bg-gray-700 border-gray-600 text-gray-300 hover:bg-gray-600 hover:text-white"> | |
| <Globe className="h-4 w-4 mr-2" /> | |
| <SelectValue placeholder="Language" /> | |
| </SelectTrigger> | |
| <SelectContent className="bg-gray-800 border-gray-700 text-gray-200"> | |
| {supportedLocales.map((loc) => ( | |
| <SelectItem key={loc} value={loc} className="hover:bg-gray-700 focus:bg-gray-700"> | |
| {localeNames[loc]} | |
| </SelectItem> | |
| ))} | |
| </SelectContent> | |
| </Select> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/LanguageSwitcher.tsx` around lines 36 - 49, Replace the hardcoded
SelectItem entries with a dynamic map over your i18n config: import
supportedLocales and localeNames from i18n/index.ts, then inside the
SelectContent iterate supportedLocales to render a SelectItem for each locale
using the locale string for the value and localeNames[locale] (or a fallback)
for the display text; keep existing props/handlers (locale, handleLocaleChange,
isPending, SelectTrigger/SelectValue) and preserve the current className/styling
on SelectItem and SelectContent.
|
|
||
| const changeLocale = (newLocale: Locale) => { | ||
| setLocale(newLocale); | ||
| document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`; |
Contributor
There was a problem hiding this comment.
Add SameSite attribute to cookie for security.
The NEXT_LOCALE cookie should include a SameSite attribute to protect against CSRF attacks. Consider adding at minimum SameSite=Lax.
🔒 Proposed fix to add SameSite attribute
- document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`;
+ document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000;SameSite=Lax`;📝 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
| document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000`; | |
| document.cookie = `NEXT_LOCALE=${newLocale};path=/;max-age=31536000;SameSite=Lax`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/LocaleProvider.tsx` at line 35, The NEXT_LOCALE cookie assignment
in LocaleProvider (document.cookie in components/LocaleProvider.tsx) is missing
a SameSite attribute; update the cookie string in the locale setter (where
document.cookie = `NEXT_LOCALE=${newLocale}...`) to include a SameSite attribute
(e.g., append `;SameSite=Lax`) and, if you need cross-site usage, use
`SameSite=None;Secure` instead to ensure the cookie is sent securely.
Comment on lines
+5
to
+6
| const locales: Locale[] = ['en', 'zh-CN']; | ||
| const defaultLocale: Locale = 'en'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Eliminate duplication: import locale constants from i18n module.
The locales array and defaultLocale are duplicated from i18n/index.ts. This is the same duplication issue as in app/layout.tsx and violates DRY. Three copies of these constants now exist across the codebase.
♻️ Proposed refactor to use centralized constants
+import { defaultLocale, dictionaries } from '@/i18n';
import type { Locale } from '@/i18n';
-const locales: Locale[] = ['en', 'zh-CN'];
-const defaultLocale: Locale = 'en';
+const locales = Object.keys(dictionaries) as Locale[];Alternatively, export locales from i18n/index.ts and import it here.
📝 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
| const locales: Locale[] = ['en', 'zh-CN']; | |
| const defaultLocale: Locale = 'en'; | |
| import { defaultLocale, dictionaries } from '@/i18n'; | |
| import type { Locale } from '@/i18n'; | |
| const locales = Object.keys(dictionaries) as Locale[]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@middleware/index.ts` around lines 5 - 6, The locales array and defaultLocale
constant are duplicated here; remove the local definitions of locales and
defaultLocale and import them from the centralized i18n module (exported from
i18n/index.ts), then update any references in this file to use the imported
symbols (locales, defaultLocale) so the middleware reuses the single source of
truth.
Leck88
force-pushed
the
zh-i18n
branch
2 times, most recently
from
3c99daf to
610d3de
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
database/mongoose.ts (1)
32-35: ⚖️ Poor tradeoffConsider consistency in error-handling strategy.
The function now returns
nullwith a warning whenMONGODB_URIis missing (lines 32-34), but still throws whenmongoose.connect()fails (line 48). This creates two different error-handling patterns for connection failures:
- Missing URI → returns
null(soft fail)- Connection failure → throws (hard fail)
This mixed approach may confuse consumers. Consider whether both cases should throw, or both should return
null, depending on whether the application should fail fast or gracefully degrade when MongoDB is unavailable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@database/mongoose.ts` around lines 32 - 35, The code currently returns null when MONGODB_URI is missing but throws on mongoose.connect failure; pick one consistent strategy and implement it: either (A) make both soft-fail by changing the mongoose.connect error path to catch the error and return null (wrap the await mongoose.connect(...) in try/catch and return null on failure), or (B) make both hard-fail by replacing the console.warn + return null when MONGODB_URI is falsy with throwing a clear Error (e.g., `throw new Error("MONGODB_URI is not set")`) so the function consistently throws; update calls that expect the function’s result accordingly. Ensure you modify the code around the MONGODB_URI check and the await mongoose.connect(...) call to reflect the chosen approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@database/mongoose.ts`:
- Around line 31-35: connectToDatabase should declare an explicit return type
(e.g., Promise<mongoose.Connection | null> or Promise<YourDbClientType | null>)
and consistently return that type when MONGODB_URI is missing; update the
signature of connectToDatabase to reflect Promise<... | null>. Then update every
consumer that assumes a non-null result—specifically any code that uses the
returned value to access .connection.db or uses it without a null-check—to
handle the null case: either guard with if (!db) { throw/new error/early-return
} or add proper conditional logic before accessing .connection.db; ensure call
sites that assign the result to a variable check for null before property access
and adjust types accordingly.
In `@lib/better-auth/auth.ts`:
- Around line 20-21: The code uses mongoose.connection.db (const db =
mongoose.connection; const database = db.db) without guarding that .db is
initialized; update the logic in the connectToDatabase / auth initialization to
check that mongoose.connection.db is present before using it (e.g., if
(!mongoose.connection || !mongoose.connection.db) throw or return a clear error,
or wait/retry until mongoose.connection.db is set) and ensure the adapter
receives a valid database object; reference the variables db and database and
the connectToDatabase flow when adding this defensive validation.
---
Nitpick comments:
In `@database/mongoose.ts`:
- Around line 32-35: The code currently returns null when MONGODB_URI is missing
but throws on mongoose.connect failure; pick one consistent strategy and
implement it: either (A) make both soft-fail by changing the mongoose.connect
error path to catch the error and return null (wrap the await
mongoose.connect(...) in try/catch and return null on failure), or (B) make both
hard-fail by replacing the console.warn + return null when MONGODB_URI is falsy
with throwing a clear Error (e.g., `throw new Error("MONGODB_URI is not set")`)
so the function consistently throws; update calls that expect the function’s
result accordingly. Ensure you modify the code around the MONGODB_URI check and
the await mongoose.connect(...) call to reflect the chosen approach.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 730f8f9e-61cb-4257-b5b6-654d4a6d0ff9
📒 Files selected for processing (14)
app/layout.tsxcomponents/Footer.tsxcomponents/LanguageSwitcher.tsxcomponents/LocaleProvider.tsxcomponents/NavItems.tsxdatabase/mongoose.tshooks/useDictionary.tshooks/useLocale.tsi18n/en.tsi18n/index.tsi18n/zh-CN.tslib/better-auth/auth.tslib/constants.tsmiddleware/index.ts
✅ Files skipped from review due to trivial changes (1)
- i18n/zh-CN.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- hooks/useDictionary.ts
- hooks/useLocale.ts
- i18n/index.ts
- components/LanguageSwitcher.tsx
- lib/constants.ts
- i18n/en.ts
- app/layout.tsx
- components/LocaleProvider.tsx
- middleware/index.ts
- components/Footer.tsx
- components/NavItems.tsx
Comment on lines
31
to
35
| export const connectToDatabase = async () => { | ||
| if (!MONGODB_URI) { | ||
| throw new Error("MongoDB URI is missing"); | ||
| console.warn("MongoDB URI is missing — skipping database connection"); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all consumers of connectToDatabase to verify they handle null
# Search for imports and usage of connectToDatabase
rg -n "connectToDatabase" --type ts --type tsx -C 3Repository: Open-Dev-Society/OpenStock
Length of output: 98
🏁 Script executed:
# First, let's examine the mongoose.ts file to see the actual function signature and implementation
cat -n database/mongoose.tsRepository: Open-Dev-Society/OpenStock
Length of output: 1786
🏁 Script executed:
# Search for connectToDatabase usage with corrected file type (remove tsx)
rg -n "connectToDatabase" --type ts -C 3Repository: Open-Dev-Society/OpenStock
Length of output: 11379
Add explicit return type and fix consumers to handle null return value.
The function signature lacks an explicit return type but can return null when MONGODB_URI is missing (line 34). This creates a type safety gap where consumers might not handle the null case, leading to potential runtime errors.
Update the function signature:
Return type fix
-export const connectToDatabase = async () => {
+export const connectToDatabase = async (): Promise<typeof mongoose | null> => {Critical: Multiple consumers do not check for null before using the return value:
scripts/test-db.ts:5,lib/inngest/functions.ts:214,lib/inngest/functions.ts:278,lib/actions/watchlist.actions.ts:11,lib/actions/watchlist.actions.ts:35,lib/actions/watchlist.actions.ts:48,lib/actions/watchlist.actions.ts:60,lib/actions/alert.actions.ts:15,lib/actions/alert.actions.ts:32,lib/actions/alert.actions.ts:44,lib/actions/alert.actions.ts:57call it without null checkslib/inngest/functions.ts:302,lib/inngest/functions.ts:340,lib/actions/watchlist.actions.ts:75,lib/actions/user.actions.ts:7,lib/actions/auth.actions.ts:41assign the result to a variable and access.connection.dbwithout checking if the returned value is null first
Only lib/better-auth/auth.ts:16-18 correctly checks for null and throws an appropriate error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@database/mongoose.ts` around lines 31 - 35, connectToDatabase should declare
an explicit return type (e.g., Promise<mongoose.Connection | null> or
Promise<YourDbClientType | null>) and consistently return that type when
MONGODB_URI is missing; update the signature of connectToDatabase to reflect
Promise<... | null>. Then update every consumer that assumes a non-null
result—specifically any code that uses the returned value to access
.connection.db or uses it without a null-check—to handle the null case: either
guard with if (!db) { throw/new error/early-return } or add proper conditional
logic before accessing .connection.db; ensure call sites that assign the result
to a variable check for null before property access and adjust types
accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/better-auth/auth.ts`:
- Around line 22-24: The code currently sets authInitFailed = true and
permanently short-circuits to createMockAuth() (seen where authInitFailed is
read and where it is set around lines 22 and 63-66), which prevents recovery
from transient MongoDB outages; change this to a retry/cooldown strategy: when
initialization fails, do not permanently set a blocking flag—instead schedule
retries with exponential backoff (or set a timestamp/until value) and return the
mock only temporarily while a background retry attempts to re-run the real
initialization (call the same init function that previously set authInitFailed)
and clear the failure state on success; ensure both the early-return check
(authInitFailed read) and the failure path (where authInitFailed is assigned)
are updated so the code uses a time-based/backoff retry policy rather than an
irreversible boolean lockout.
- Around line 11-15: createMockAuth currently returns null which causes unsafe
null dereferences; replace it with a deterministic "unavailable" auth object
(implementing the same interface used by real auth) that provides safe,
predictable fallbacks (e.g., methods like authenticate, getUser, verifyToken
return rejected Promises or explicit errors / no-op safe values) so callers
never receive null and TypeScript type-safety is preserved; update
createMockAuth to construct and return that object instead of null and ensure
its shape matches the real auth type/signature.
- Around line 40-41: Replace the permissive fallbacks for the BetterAuth
configuration so production never uses predictable defaults: check
process.env.NODE_ENV (or equivalent environment flag) and if in production,
validate that process.env.BETTER_AUTH_SECRET and process.env.BETTER_AUTH_URL are
present and throw a clear error (including which variable is missing) instead of
using "dev-secret-change-in-production" or "http://localhost:3000"; keep the
existing fallbacks only for non-production (development/test) runs and update
the error message to reference the config keys used in the auth config object
(secret and baseURL) so callers can fix environment setup quickly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08aff3fb-c251-4e72-a9c4-c4ded1614963
📒 Files selected for processing (14)
app/layout.tsxcomponents/Footer.tsxcomponents/LanguageSwitcher.tsxcomponents/LocaleProvider.tsxcomponents/NavItems.tsxdatabase/mongoose.tshooks/useDictionary.tshooks/useLocale.tsi18n/en.tsi18n/index.tsi18n/zh-CN.tslib/better-auth/auth.tslib/constants.tsmiddleware/index.ts
✅ Files skipped from review due to trivial changes (1)
- i18n/en.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- hooks/useLocale.ts
- hooks/useDictionary.ts
- i18n/zh-CN.ts
- lib/constants.ts
- i18n/index.ts
- database/mongoose.ts
- middleware/index.ts
- components/NavItems.tsx
- components/Footer.tsx
- app/layout.tsx
- components/LanguageSwitcher.tsx
- components/LocaleProvider.tsx
Comment on lines
+11
to
+15
| const createMockAuth = () => { | ||
| // Return a minimal mock so the module can load without MongoDB | ||
| // All auth operations will fail gracefully at runtime | ||
| return null as any; | ||
| }; |
Contributor
There was a problem hiding this comment.
createMockAuth returns null, so fallback is not actually safe.
Line 14 (null as any) turns failures into downstream null dereferences and drops type safety.
🔧 Suggested fix (deterministic unavailable-auth object)
const createMockAuth = () => {
- // Return a minimal mock so the module can load without MongoDB
- // All auth operations will fail gracefully at runtime
- return null as any;
+ // Return a deterministic placeholder that fails with a clear error
+ return new Proxy(
+ {},
+ {
+ get() {
+ throw new Error("Auth is unavailable: MongoDB initialization failed");
+ },
+ },
+ ) as ReturnType<typeof betterAuth>;
};📝 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
| const createMockAuth = () => { | |
| // Return a minimal mock so the module can load without MongoDB | |
| // All auth operations will fail gracefully at runtime | |
| return null as any; | |
| }; | |
| const createMockAuth = () => { | |
| // Return a deterministic placeholder that fails with a clear error | |
| return new Proxy( | |
| {}, | |
| { | |
| get() { | |
| throw new Error("Auth is unavailable: MongoDB initialization failed"); | |
| }, | |
| }, | |
| ) as ReturnType<typeof betterAuth>; | |
| }; |
🧰 Tools
🪛 ESLint
[error] 14-14: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/better-auth/auth.ts` around lines 11 - 15, createMockAuth currently
returns null which causes unsafe null dereferences; replace it with a
deterministic "unavailable" auth object (implementing the same interface used by
real auth) that provides safe, predictable fallbacks (e.g., methods like
authenticate, getUser, verifyToken return rejected Promises or explicit errors /
no-op safe values) so callers never receive null and TypeScript type-safety is
preserved; update createMockAuth to construct and return that object instead of
null and ensure its shape matches the real auth type/signature.
Comment on lines
+22
to
24
| if (authInitFailed) { | ||
| return createMockAuth(); | ||
| } |
Contributor
There was a problem hiding this comment.
Transient MongoDB failures permanently disable auth initialization.
Line 22 short-circuits forever once Line 65 sets authInitFailed = true, so a temporary startup outage requires a process restart to recover.
🔧 Suggested fix (retry after cooldown instead of permanent lockout)
-let authInitFailed = false;
+let lastAuthInitFailureAt = 0;
+const AUTH_RETRY_COOLDOWN_MS = 30_000;
@@
- if (authInitFailed) {
+ if (
+ lastAuthInitFailureAt &&
+ Date.now() - lastAuthInitFailureAt < AUTH_RETRY_COOLDOWN_MS
+ ) {
return createMockAuth();
}
@@
- return authInstance;
+ lastAuthInitFailureAt = 0;
+ return authInstance;
} catch (err) {
console.warn("[Auth] MongoDB not available, using mock auth:", err);
- authInitFailed = true;
+ lastAuthInitFailureAt = Date.now();
return createMockAuth();
}Also applies to: 63-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/better-auth/auth.ts` around lines 22 - 24, The code currently sets
authInitFailed = true and permanently short-circuits to createMockAuth() (seen
where authInitFailed is read and where it is set around lines 22 and 63-66),
which prevents recovery from transient MongoDB outages; change this to a
retry/cooldown strategy: when initialization fails, do not permanently set a
blocking flag—instead schedule retries with exponential backoff (or set a
timestamp/until value) and return the mock only temporarily while a background
retry attempts to re-run the real initialization (call the same init function
that previously set authInitFailed) and clear the failure state on success;
ensure both the early-return check (authInitFailed read) and the failure path
(where authInitFailed is assigned) are updated so the code uses a
time-based/backoff retry policy rather than an irreversible boolean lockout.
Comment on lines
+40
to
+41
| secret: process.env.BETTER_AUTH_SECRET || "dev-secret-change-in-production", | ||
| baseURL: process.env.BETTER_AUTH_URL || "http://localhost:3000", |
Contributor
There was a problem hiding this comment.
Do not allow weak auth env fallbacks in production.
Line 40 falls back to a predictable secret and Line 41 to localhost. In production this can weaken session/token security and generate invalid callback/reset URLs.
🔧 Suggested fix (fail fast in production if env is missing)
+const isProd = process.env.NODE_ENV === "production";
+const authSecret = process.env.BETTER_AUTH_SECRET;
+const authBaseURL = process.env.BETTER_AUTH_URL;
+
+if (isProd && (!authSecret || !authBaseURL)) {
+ throw new Error("BETTER_AUTH_SECRET and BETTER_AUTH_URL are required in production");
+}
+
authInstance = betterAuth({
database: mongodbAdapter(database),
- secret: process.env.BETTER_AUTH_SECRET || "dev-secret-change-in-production",
- baseURL: process.env.BETTER_AUTH_URL || "http://localhost:3000",
+ secret: authSecret ?? "dev-secret-change-in-production",
+ baseURL: authBaseURL ?? "http://localhost:3000",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/better-auth/auth.ts` around lines 40 - 41, Replace the permissive
fallbacks for the BetterAuth configuration so production never uses predictable
defaults: check process.env.NODE_ENV (or equivalent environment flag) and if in
production, validate that process.env.BETTER_AUTH_SECRET and
process.env.BETTER_AUTH_URL are present and throw a clear error (including which
variable is missing) instead of using "dev-secret-change-in-production" or
"http://localhost:3000"; keep the existing fallbacks only for non-production
(development/test) runs and update the error message to reference the config
keys used in the auth config object (secret and baseURL) so callers can fix
environment setup quickly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/better-auth/auth.ts (2)
40-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast on missing auth config in production.
These fallbacks are fine for local development, but in production they weaken token/session security and can generate invalid reset/callback URLs. Keep the defaults only outside production and throw if
BETTER_AUTH_SECRETorBETTER_AUTH_URLis missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/better-auth/auth.ts` around lines 40 - 41, The current assignment of secret and baseURL in auth.ts uses unsafe defaults (secret: process.env.BETTER_AUTH_SECRET || "dev-secret-change-in-production", baseURL: process.env.BETTER_AUTH_URL || "http://localhost:3000"); change this so that in non-production (NODE_ENV !== "production") the existing defaults remain, but when NODE_ENV === "production" the code throws a clear error if BETTER_AUTH_SECRET or BETTER_AUTH_URL is missing; locate the secret and baseURL configuration (the secret and baseURL fields) and replace the simple || fallbacks with explicit checks that throw descriptive errors in production while keeping development defaults otherwise.
9-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStartup failures still collapse auth into a permanent
nullfallback.Once Line 65 sets
authInitFailed, every latergetAuth()call short-circuits tocreateMockAuth(), and that mock is stillnull as any. A transient MongoDB outage therefore becomes a permanent auth outage until restart, with downstream null dereferences hidden by the declared return type.Also applies to: 22-24, 63-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/better-auth/auth.ts` around lines 9 - 15, authInitFailed currently causes getAuth() to always return createMockAuth() which is implemented as null, producing permanent null fallbacks; change createMockAuth() to return a minimal safe stub object (implementing the auth API used elsewhere) that throws clear runtime errors instead of null, and remove/avoid making authInitFailed a permanent short‑circuit: either only set it for the current init attempt or remove the global short‑circuit and let getAuth()/initAuth() retry initialization on subsequent calls so transient Mongo outages recover; update references to authInitFailed, createMockAuth, and getAuth to use the safe stub and retry behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/LocaleProvider.tsx`:
- Around line 25-31: LocaleProvider currently seeds state from props once, so if
parent updates initialLocale or dictionary later the provider stays stale; add
an effect that watches initialLocale and dictionary and calls
setLocale(initialLocale) and setCurrentDictionary(dictionary ||
getDictionary(initialLocale)) (or update currentDictionary via getDictionary
when only initialLocale changes) so the component syncs to prop changes; update
the existing useEffect usages (the one that calls getDictionary(locale)) to
coexist with this prop-sync effect and reference LocaleProvider, initialLocale,
dictionary, setLocale, setCurrentDictionary, and getDictionary when making the
changes.
---
Duplicate comments:
In `@lib/better-auth/auth.ts`:
- Around line 40-41: The current assignment of secret and baseURL in auth.ts
uses unsafe defaults (secret: process.env.BETTER_AUTH_SECRET ||
"dev-secret-change-in-production", baseURL: process.env.BETTER_AUTH_URL ||
"http://localhost:3000"); change this so that in non-production (NODE_ENV !==
"production") the existing defaults remain, but when NODE_ENV === "production"
the code throws a clear error if BETTER_AUTH_SECRET or BETTER_AUTH_URL is
missing; locate the secret and baseURL configuration (the secret and baseURL
fields) and replace the simple || fallbacks with explicit checks that throw
descriptive errors in production while keeping development defaults otherwise.
- Around line 9-15: authInitFailed currently causes getAuth() to always return
createMockAuth() which is implemented as null, producing permanent null
fallbacks; change createMockAuth() to return a minimal safe stub object
(implementing the auth API used elsewhere) that throws clear runtime errors
instead of null, and remove/avoid making authInitFailed a permanent
short‑circuit: either only set it for the current init attempt or remove the
global short‑circuit and let getAuth()/initAuth() retry initialization on
subsequent calls so transient Mongo outages recover; update references to
authInitFailed, createMockAuth, and getAuth to use the safe stub and retry
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12445659-3b8b-4ffd-aec9-8fed57bc34c5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
app/layout.tsxcomponents/Footer.tsxcomponents/LanguageSwitcher.tsxcomponents/LocaleProvider.tsxcomponents/NavItems.tsxdatabase/mongoose.tshooks/useDictionary.tshooks/useLocale.tsi18n/en.tsi18n/index.tsi18n/zh-CN.tslib/better-auth/auth.tslib/constants.tsmiddleware/index.tspackage.json
✅ Files skipped from review due to trivial changes (3)
- hooks/useLocale.ts
- i18n/zh-CN.ts
- i18n/en.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- middleware/index.ts
- hooks/useDictionary.ts
- lib/constants.ts
- components/LanguageSwitcher.tsx
- components/NavItems.tsx
- components/Footer.tsx
- app/layout.tsx
- database/mongoose.ts
Comment on lines
+25
to
+31
| export function LocaleProvider({ children, locale: initialLocale, dictionary }: LocaleProviderProps) { | ||
| const [locale, setLocale] = useState<Locale>(initialLocale); | ||
| const [currentDictionary, setCurrentDictionary] = useState<Dictionary>(dictionary); | ||
|
|
||
| useEffect(() => { | ||
| setCurrentDictionary(getDictionary(locale)); | ||
| }, [locale]); |
Contributor
There was a problem hiding this comment.
Sync provider state when locale props change after mount.
useState(initialLocale) and useState(dictionary) only read props once. If parent props change later, provider state can become stale and show the wrong language until a manual toggle.
Proposed fix
export function LocaleProvider({ children, locale: initialLocale, dictionary }: LocaleProviderProps) {
const [locale, setLocale] = useState<Locale>(initialLocale);
const [currentDictionary, setCurrentDictionary] = useState<Dictionary>(dictionary);
+ useEffect(() => {
+ setLocale(initialLocale);
+ setCurrentDictionary(dictionary);
+ }, [initialLocale, dictionary]);
+
useEffect(() => {
setCurrentDictionary(getDictionary(locale));
}, [locale]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/LocaleProvider.tsx` around lines 25 - 31, LocaleProvider currently
seeds state from props once, so if parent updates initialLocale or dictionary
later the provider stays stale; add an effect that watches initialLocale and
dictionary and calls setLocale(initialLocale) and
setCurrentDictionary(dictionary || getDictionary(initialLocale)) (or update
currentDictionary via getDictionary when only initialLocale changes) so the
component syncs to prop changes; update the existing useEffect usages (the one
that calls getDictionary(locale)) to coexist with this prop-sync effect and
reference LocaleProvider, initialLocale, dictionary, setLocale,
setCurrentDictionary, and getDictionary when making the changes.
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.
Summary
Add Simplified Chinese (zh-CN) i18n support with cookie-based locale switching.
Changes
How to test
Summary by CodeRabbit
New Features
Bug Fixes / Improvements