Skip to content

fix(i18n): refresh server chrome on locale switch#1116

Merged
ding113 merged 3 commits into
devfrom
fix/desktop-i18n-refresh-20260427
Apr 27, 2026
Merged

fix(i18n): refresh server chrome on locale switch#1116
ding113 merged 3 commits into
devfrom
fix/desktop-i18n-refresh-20260427

Conversation

@ding113
Copy link
Copy Markdown
Owner

@ding113 ding113 commented Apr 27, 2026

Summary

  • Fixes stale desktop navigation and server-rendered heading/description copy after switching locale.
  • Passes the [locale] route param explicitly to server-side getTranslations calls for route chrome and page headers.
  • Adds a locale-switch refresh guard so remounted client navigation can refresh the current RSC tree after the provider locale catches up.
  • Adds regression coverage for the language switcher remount path and for implicit server translation calls in route pages/layouts.

Root Cause

App Router can refresh only the nested RSC tree during locale navigation. Some dashboard/settings route chrome and page headers relied on implicit getTranslations("..."), which depends on request locale context from the root layout. On partial refreshes, those server-rendered segments could keep the previous locale while client components under the new NextIntlClientProvider updated correctly.

Screenshots

Desktop settings page after zh-CN -> en switch:

Desktop i18n switch fixed

Mobile settings page after zh-CN -> en switch:

Mobile i18n switch fixed

Verification

  • bun run build
  • bun run lint:fix
  • bun run lint
  • bun run typecheck
  • bun run test
  • Browser smoke, production mode on localhost:13528:
    • desktop zh-CN/settings/config -> en/settings/config: nav, sidebar, h1, h2, descriptions switched to English
    • mobile zh-CN/settings/config -> en/settings/config: bottom nav, h1, h2, descriptions switched to English

Notes: bun run lint still reports pre-existing Biome warnings in unrelated files (leaderboard-view-* tests and src/i18n/pathname.ts) but exits successfully. bun run build still reports existing node:net Edge Runtime warnings in IP utilities but exits successfully.

Greptile Summary

This PR fixes stale server-rendered translations after a locale switch in the App Router by threading the [locale] route param explicitly into every getTranslations call across dashboard and settings pages, and adds a client-side router.refresh() guard in LanguageSwitcher so the RSC tree re-renders after the provider locale catches up. The mechanical server-side changes are consistent and covered by a new regression test; the refresh-guard logic in language-switcher.tsx has one subtle dead-code path (see inline comment).

Confidence Score: 5/5

Safe to merge; the single P2 finding is a dead-code path in the sessionStorage fallback that does not affect observable behaviour

All server-side changes are straightforward and correct locale-threading. The refresh-guard logic works correctly in all tested scenarios; the sessionStorage fallback being unreachable as the deciding value is a maintainability concern, not a runtime bug. No P0/P1 issues found.

src/components/ui/language-switcher.tsx — the storedRefreshTarget dead-code path warrants a closer look

Important Files Changed

Filename Overview
src/components/ui/language-switcher.tsx Adds locale-switch refresh guard using module-level variable + sessionStorage; the sessionStorage read path is logically unreachable as the deciding value (see comment)
src/app/[locale]/dashboard/_components/dashboard-header.tsx Correctly converted from client useTranslations to async server getTranslations with explicit locale prop
src/app/[locale]/settings/layout.tsx Passes locale explicitly to getTranslatedNavItems and DashboardHeader; consistent with rest of the fix
tests/unit/i18n/locale-server-translations.test.ts Regression guard that walks the [locale] tree and asserts no implicit getTranslations("…") calls remain in route/chrome files
src/components/ui/tests/language-switcher.test.tsx New unit tests covering the refresh-guard effect, remount persistence, blocked-storage fallback, and stale-marker guard; test 4 (stale marker) implicitly validates the current inverted-guard behaviour
src/app/[locale]/dashboard/quotas/providers/page.tsx Both ProvidersQuotaPage and the inner ProvidersQuotaContent updated to accept and forward locale explicitly
src/app/[locale]/settings/prices/page.tsx Adds params to page props, extracts locale, and forwards it to both SettingsPricesPage and SettingsPricesContent; correct refactor

Sequence Diagram

sequenceDiagram
    participant User
    participant LanguageSwitcher
    participant Router
    participant NextIntlProvider
    participant RSCTree

    User->>LanguageSwitcher: click "English"
    LanguageSwitcher->>LanguageSwitcher: setIsTransitioning(true)<br/>setPendingLocale("en")<br/>setPendingLocaleRefreshTarget("en")
    LanguageSwitcher->>Router: push(pathname, { locale: "en" })
    Router->>NextIntlProvider: navigate to /en/…
    NextIntlProvider-->>LanguageSwitcher: currentLocale = "en" (re-render)
    Note over LanguageSwitcher: useEffect: pendingLocale("en") === currentLocale("en")
    LanguageSwitcher->>Router: router.refresh()
    LanguageSwitcher->>LanguageSwitcher: clearPendingLocaleRefreshTarget()<br/>setPendingLocale(null)
    Router->>RSCTree: re-fetch server components with explicit locale
    RSCTree-->>User: updated translations rendered
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/components/ui/language-switcher.tsx
Line: 82-84

Comment:
**`storedRefreshTarget` is unreachable as the deciding value**

`storedRefreshTarget` is derived from `sessionStorage` only when `activePendingLocaleRefreshTarget !== null`, but in the `??` chain `activePendingLocaleRefreshTarget` always takes priority over `storedRefreshTarget`. Conversely, when `activePendingLocaleRefreshTarget === null`, `storedRefreshTarget` is forced to `null` and the `sessionStorage` read is skipped. This means the stored session value can never be the final non-null `refreshTarget` — the sessionStorage persistence acts as a write-only fallback that is never actually read as a recovery path.

If the intent is to recover from a module re-initialisation (e.g. HMR between the click and remount), the guard condition should be inverted:

```ts
// read sessionStorage only as a cold-start fallback (no active in-memory signal)
const storedRefreshTarget =
  activePendingLocaleRefreshTarget === null ? getPendingLocaleRefreshTarget() : null;
const refreshTarget = pendingLocale ?? activePendingLocaleRefreshTarget ?? storedRefreshTarget;
```

Note: this would also need the "stale stored marker on mount" test to be updated so that a stale entry is cleaned up rather than silently ignored, because without the `activePendingLocaleRefreshTarget` guard the condition `refreshTarget === currentLocale` could fire on a leftover entry. The test at line 737 would need adjustment accordingly.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "fix(i18n): log locale refresh storage fa..." | Re-trigger Greptile

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

该 PR 将大量页面与布局的 i18n 加载从只传命名空间的 getTranslations("ns") 改为显式传入 { locale, namespace },并调整若干服务器组件以从 route params 解析并传递 locale;同时为语言切换器添加基于 sessionStorage 的 pending-refresh 逻辑并新增相关测试。

Changes

Cohort / File(s) Summary
Dashboard header 与 layout
src/app/[locale]/dashboard/_components/dashboard-header.tsx, src/app/[locale]/dashboard/layout.tsx
DashboardHeader 改为 async 并新增 locale prop,改用 getTranslations({ locale, namespace: "dashboard.nav" })DashboardLayout 将解析出的 locale 传入 Header。
Dashboard sections
src/app/[locale]/dashboard/_components/dashboard-sections.tsx
DashboardLeaderboardSection 增加 locale prop,改用 getTranslations({ locale, namespace: "dashboard" })
Dashboard — 各页面(审计/可用性/排行榜/配额/限速/providers)
src/app/[locale]/dashboard/audit-logs/page.tsx, src/app/[locale]/dashboard/availability/page.tsx, src/app/[locale]/dashboard/leaderboard/page.tsx, src/app/[locale]/dashboard/my-quota/page.tsx, src/app/[locale]/dashboard/providers/page.tsx, src/app/[locale]/dashboard/rate-limits/page.tsx
多处页面改为从 params/await 中解析 locale,并将 { locale, namespace } 传给 getTranslations;部分页面签名更新以接受 params
Dashboard — 额度子路由
src/app/[locale]/dashboard/quotas/layout.tsx, src/app/[locale]/dashboard/quotas/providers/page.tsx, src/app/[locale]/dashboard/quotas/users/page.tsx
QuotasLayout 接受 params 并传入 localegetTranslations;Providers/Users 内容组件改为接收 locale 并使用 locale-aware 翻译调用。
Settings — 导航、layout 与大量页面
src/app/[locale]/settings/_lib/nav-items.ts, src/app/[locale]/settings/layout.tsx, src/app/[locale]/settings/client-versions/page.tsx, src/app/[locale]/settings/config/page.tsx, src/app/[locale]/settings/error-rules/page.tsx, src/app/[locale]/settings/logs/page.tsx, src/app/[locale]/settings/prices/page.tsx, src/app/[locale]/settings/providers/page.tsx, src/app/[locale]/settings/request-filters/page.tsx, src/app/[locale]/settings/sensitive-words/page.tsx, src/app/[locale]/settings/status-page/page.tsx
getTranslatedNavItems 签名改为接收 locale;Settings layout 与多个页面改为从 params 获取 locale,并把 { locale, namespace: "settings" } 传给 getTranslations;若干内容组件接收 locale
Settings prices 与公共 status
src/app/[locale]/settings/prices/page.tsx, src/app/[locale]/status/[slug]/page.tsx
SettingsPricesPage 调整参数以同时接收 params(含 locale)与 searchParams,内容组件接收并使用 locale;Public status 页面改为 locale-aware 的 getTranslations 调用。
Usage-doc layout
src/app/[locale]/usage-doc/layout.tsx
当存在 session 时将解析出的 locale 传入 DashboardHeader
Language-switcher 及测试
src/components/ui/language-switcher.tsx, src/components/ui/__tests__/language-switcher.test.tsx
为语言切换器加入 sessionStorage 的 pending-locale-refresh 流程:在导航后将目标 locale 写入存储并在实际路由 locale 匹配时触发 router.refresh(),在导航失败时清理状态;新增 Vitest 测试覆盖该行为与边界情况。
i18n 验证测试
tests/unit/i18n/locale-server-translations.test.ts
新增单元测试,递归扫描 src/app/[locale],断言所有 getTranslations 调用必须显式传入 locale(禁止使用仅含字符串字面量的调用)。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题清晰准确地总结了主要变更:修复locale切换时服务器渲染的导航和标题内容过时的问题。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed PR description clearly describes the fix for stale translations after locale switch, explains the root cause, provides verification steps, and documents the changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/desktop-i18n-refresh-20260427

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added bug Something isn't working area:i18n area:UX size/L Large PR (< 1000 lines) labels Apr 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request standardizes the usage of getTranslations across server components by explicitly passing the locale parameter, ensuring consistent internationalization behavior throughout the application. Key changes include refactoring the DashboardHeader into an async server component and updating numerous dashboard and settings pages to extract and use the locale from route parameters. The LanguageSwitcher component was also enhanced with a session-based refresh mechanism to synchronize the UI after locale transitions. Additionally, new tests were added to verify the language switcher logic and enforce the explicit locale requirement in server-side translation calls. I have no feedback to provide as there were no review comments to evaluate.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
tests/unit/i18n/locale-server-translations.test.ts (1)

15-40: 测试覆盖范围与匹配模式可进一步加固。

当前用例存在几处可能漏检的场景,建议视情况增强:

  1. 正则 /getTranslations\(\s*["']/ 仅在单行内匹配带引号的首参,遇到换行书写(例如 getTranslations(\n "ns"\n))会漏掉。
  2. 同样存在 locale 漂移风险的 getTranslations() 无参形式(依赖请求级 locale 上下文)未被纳入检测。
  3. isRouteOrServerChromeFile 通过白名单文件名限定范围,新增 server-chrome 组件(如未来添加的桌面/移动导航部件)若不匹配 page.tsx/layout.tsx 与三条 endsWith,回归用例将无法守护。
  4. walk("src/app/[locale]") 依赖运行时 cwd 等于仓库根,建议结合 process.cwd() 或基于 import.meta.url 解析为绝对路径,以提高在不同 runner/工作目录下的稳定性。
♻️ 参考改动:扩大检测范围并使用绝对路径
-import { readdirSync, readFileSync } from "node:fs";
-import { basename, join } from "node:path";
+import { readdirSync, readFileSync } from "node:fs";
+import { basename, join, resolve } from "node:path";
 import { describe, expect, test } from "vitest";
 
-function walk(dir: string): string[] {
+function walk(dir: string): string[] {
   return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
     const fullPath = join(dir, entry.name);
     if (entry.isDirectory()) {
       return walk(fullPath);
     }
     return /\.(ts|tsx)$/.test(entry.name) ? [fullPath] : [];
   });
 }
 
 function isRouteOrServerChromeFile(filePath: string): boolean {
   const fileName = basename(filePath);
 
   return (
     fileName === "page.tsx" ||
     fileName === "layout.tsx" ||
     filePath.endsWith("dashboard-header.tsx") ||
     filePath.endsWith("dashboard-sections.tsx") ||
     filePath.endsWith("settings/_lib/nav-items.ts")
   );
 }
 
 describe("locale server translations", () => {
   test("route pages and server chrome pass locale explicitly to getTranslations", () => {
-    const files = walk("src/app/[locale]").filter(isRouteOrServerChromeFile);
+    const root = resolve(process.cwd(), "src/app/[locale]");
+    const files = walk(root).filter(isRouteOrServerChromeFile);
     const violations = files.flatMap((file) => {
       const content = readFileSync(file, "utf8");
-      return content
-        .split("\n")
-        .map((line, index) => ({ line, lineNumber: index + 1 }))
-        .filter(({ line }) => /getTranslations\(\s*["']/.test(line))
-        .map(({ line, lineNumber }) => `${file}:${lineNumber}: ${line.trim()}`);
+      // Match both `getTranslations("ns")` and `getTranslations()` across lines.
+      const pattern = /getTranslations\s*\(\s*(?:["'][^"']*["']|\))/g;
+      const lines = content.split("\n");
+      const offsets = lines.reduce<number[]>((acc, line, idx) => {
+        acc.push((acc[idx - 1] ?? -1) + line.length + 1);
+        return acc;
+      }, []);
+      const matches: string[] = [];
+      for (const m of content.matchAll(pattern)) {
+        const lineNumber = offsets.findIndex((end) => (m.index ?? 0) <= end) + 1;
+        matches.push(`${file}:${lineNumber}: ${lines[lineNumber - 1].trim()}`);
+      }
+      return matches;
     });
 
     expect(violations).toEqual([]);
   });
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/i18n/locale-server-translations.test.ts` around lines 15 - 40, The
test misses multi-line and no-arg getTranslations calls, is brittle to new
server-chrome filenames, and relies on a relative walk path; update the test by
(1) changing the detection logic in the test (the regex
/getTranslations\(\s*["']/ used in the test) to detect both no-argument calls
like getTranslations() and calls where the first arg is on a new line (use a
multi-line-aware search or read the whole file and test against patterns for
getTranslations\s*\(\s*(?:["']|[\r\n]) and for getTranslations\s*\(\s*\) ), (2)
broaden isRouteOrServerChromeFile to match server-chrome components by
directory/filename patterns (e.g., include files under any server-chrome
directory or match filenames like *.tsx in server chrome folders rather than
only page.tsx/layout.tsx and the three specific endsWith entries), and (3) make
walk("src/app/[locale]") use an absolute path (resolve via process.cwd() or
import.meta.url) so the test is stable across different working directories;
update references to isRouteOrServerChromeFile and the file-walking call
accordingly.
src/components/ui/__tests__/language-switcher.test.tsx (2)

64-70: Nit: 模拟点击对 <button onClick> 来说过度。

DropdownMenuItem 渲染出的原生 <button>,仅 dispatchEvent(new MouseEvent("click", { bubbles: true })) 即可触发 React 的 onClick。mousedown/mouseup 在该 mock 里是空操作,可移除以减少噪音。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/__tests__/language-switcher.test.tsx` around lines 64 - 70,
测试中的 click helper 在模拟 DropdownMenuItem 渲染出的原生 button 时多余地触发了 mousedown 和
mouseup;将 helper 函数 click(当前实现调用 element.dispatchEvent(new
MouseEvent("mousedown"...), new MouseEvent("mouseup"...), new
MouseEvent("click"...)))简化为只触发一次 click 事件(element.dispatchEvent(new
MouseEvent("click", { bubbles: true }))),这样可以正确触发 React 的 onClick 回调并移除无用的
mousedown/mouseup 噪音。

1-9: 测试文件路径与项目约定不一致。

按编码规范,源邻接测试应位于 src/**/*.test.ts,当前文件落在 src/components/ui/__tests__/language-switcher.test.tsx(位于 __tests__/ 子目录)。建议改为源邻接命名(如 src/components/ui/language-switcher.test.tsx),或迁移至 tests/unit/ 以与仓内其他测试保持一致,避免后续 vitest/biome 规则升级时被排除或重复扫描。

As per coding guidelines: "Unit tests should be located in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/__tests__/language-switcher.test.tsx` around lines 1 - 9,
The test for the LanguageSwitcher component is placed in a __tests__
subdirectory which violates the project's test-location convention; move or
rename the file so it becomes source-adjacent (e.g., rename to
language-switcher.test.tsx next to the component) or relocate it to tests/unit/
(e.g., tests/unit/language-switcher.test.tsx), update any import paths if
needed, and ensure your vitest/biome config/test glob still matches the new
location so the LanguageSwitcher tests continue to run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/ui/__tests__/language-switcher.test.tsx`:
- Around line 25-28: The mock for usePathname returns a path with a locale
prefix which doesn't match next-intl's actual behavior; update the test setup so
testState.pathname is set to a path without the locale prefix (e.g.,
"/settings/config") in the beforeEach that mocks usePathname, ensuring the
mocked value aligns with real usePathname() output and that
normalizePathnameForLocaleNavigation() is exercised under realistic input.

In `@src/components/ui/language-switcher.tsx`:
- Around line 75-87: The effect currently consumes sessionStorage via
getPendingLocaleRefreshTarget() on mount which can trigger an unintended
router.refresh(); change the logic so the sessionStorage fallback is only used
when the in-memory pendingLocale state indicates an active user-initiated switch
(i.e., only consume getPendingLocaleRefreshTarget() when pendingLocale is
non-null or when a session-specific marker you write on setPendingLocale
exists); update the useEffect in language-switcher.tsx (referencing
pendingLocale, getPendingLocaleRefreshTarget, clearPendingLocaleRefreshTarget,
setPendingLocale, setIsTransitioning, and router.refresh) to skip reading/acting
on the stored pending locale unless it was set in this session (or pendingLocale
is already non-null), or alternatively add a timestamp/source on write and
validate it before calling router.refresh and clearing the marker.

---

Nitpick comments:
In `@src/components/ui/__tests__/language-switcher.test.tsx`:
- Around line 64-70: 测试中的 click helper 在模拟 DropdownMenuItem 渲染出的原生 button
时多余地触发了 mousedown 和 mouseup;将 helper 函数 click(当前实现调用 element.dispatchEvent(new
MouseEvent("mousedown"...), new MouseEvent("mouseup"...), new
MouseEvent("click"...)))简化为只触发一次 click 事件(element.dispatchEvent(new
MouseEvent("click", { bubbles: true }))),这样可以正确触发 React 的 onClick 回调并移除无用的
mousedown/mouseup 噪音。
- Around line 1-9: The test for the LanguageSwitcher component is placed in a
__tests__ subdirectory which violates the project's test-location convention;
move or rename the file so it becomes source-adjacent (e.g., rename to
language-switcher.test.tsx next to the component) or relocate it to tests/unit/
(e.g., tests/unit/language-switcher.test.tsx), update any import paths if
needed, and ensure your vitest/biome config/test glob still matches the new
location so the LanguageSwitcher tests continue to run.

In `@tests/unit/i18n/locale-server-translations.test.ts`:
- Around line 15-40: The test misses multi-line and no-arg getTranslations
calls, is brittle to new server-chrome filenames, and relies on a relative walk
path; update the test by (1) changing the detection logic in the test (the regex
/getTranslations\(\s*["']/ used in the test) to detect both no-argument calls
like getTranslations() and calls where the first arg is on a new line (use a
multi-line-aware search or read the whole file and test against patterns for
getTranslations\s*\(\s*(?:["']|[\r\n]) and for getTranslations\s*\(\s*\) ), (2)
broaden isRouteOrServerChromeFile to match server-chrome components by
directory/filename patterns (e.g., include files under any server-chrome
directory or match filenames like *.tsx in server chrome folders rather than
only page.tsx/layout.tsx and the three specific endsWith entries), and (3) make
walk("src/app/[locale]") use an absolute path (resolve via process.cwd() or
import.meta.url) so the test is stable across different working directories;
update references to isRouteOrServerChromeFile and the file-walking call
accordingly.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d626c4c5-9eba-4d04-bb14-ad5e3201857a

📥 Commits

Reviewing files that changed from the base of the PR and between 0726ec4 and fd02226.

📒 Files selected for processing (28)
  • src/app/[locale]/dashboard/_components/dashboard-header.tsx
  • src/app/[locale]/dashboard/_components/dashboard-sections.tsx
  • src/app/[locale]/dashboard/audit-logs/page.tsx
  • src/app/[locale]/dashboard/availability/page.tsx
  • src/app/[locale]/dashboard/layout.tsx
  • src/app/[locale]/dashboard/leaderboard/page.tsx
  • src/app/[locale]/dashboard/my-quota/page.tsx
  • src/app/[locale]/dashboard/providers/page.tsx
  • src/app/[locale]/dashboard/quotas/layout.tsx
  • src/app/[locale]/dashboard/quotas/providers/page.tsx
  • src/app/[locale]/dashboard/quotas/users/page.tsx
  • src/app/[locale]/dashboard/rate-limits/page.tsx
  • src/app/[locale]/settings/_lib/nav-items.ts
  • src/app/[locale]/settings/client-versions/page.tsx
  • src/app/[locale]/settings/config/page.tsx
  • src/app/[locale]/settings/error-rules/page.tsx
  • src/app/[locale]/settings/layout.tsx
  • src/app/[locale]/settings/logs/page.tsx
  • src/app/[locale]/settings/prices/page.tsx
  • src/app/[locale]/settings/providers/page.tsx
  • src/app/[locale]/settings/request-filters/page.tsx
  • src/app/[locale]/settings/sensitive-words/page.tsx
  • src/app/[locale]/settings/status-page/page.tsx
  • src/app/[locale]/status/[slug]/page.tsx
  • src/app/[locale]/usage-doc/layout.tsx
  • src/components/ui/__tests__/language-switcher.test.tsx
  • src/components/ui/language-switcher.tsx
  • tests/unit/i18n/locale-server-translations.test.ts

Comment thread src/components/ui/__tests__/language-switcher.test.tsx
Comment thread src/components/ui/language-switcher.tsx
@github-actions github-actions Bot added the size/M Medium PR (< 500 lines) label Apr 27, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR addresses a legitimate i18n issue where server-rendered components could display stale content after locale switching. The implementation uses an explicit locale-passing pattern and a client-side refresh guard with sessionStorage persistence. Overall approach is sound with good test coverage.

PR Size: M

  • Lines changed: 429 (363 additions, 66 deletions)
  • Files changed: 28
  • Note: PR was originally labeled size/L, but with 429 lines across 28 files, this fits size/M criteria (< 500 lines, < 20 files is a soft threshold)

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 1 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Medium Priority Issue

[ERROR-SILENT] sessionStorage errors not logged (src/components/ui/language-switcher.tsx:738-740, 748-752)

The and functions catch errors but don't log them:

While graceful degradation is appropriate here, silent failures make production debugging impossible. Consider adding to aid troubleshooting.

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - One minor observation
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Adequate
  • Code clarity - Good

Automated review by Claude AI

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR addresses a legitimate i18n issue where server-rendered components could display stale content after locale switching. The implementation uses an explicit locale-passing pattern and a client-side refresh guard with sessionStorage persistence. Overall approach is sound with good test coverage.

PR Size: M

  • Lines changed: 429 (363 additions, 66 deletions)
  • Files changed: 28

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 1 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Medium Priority Issue (Confidence: 85/100)

[ERROR-SILENT] sessionStorage errors not logged

Location: src/components/ui/language-switcher.tsx lines 738-740 and 748-752

The setPendingLocaleRefreshTarget() and clearPendingLocaleRefreshTarget() functions catch errors but do not log them. While graceful degradation is appropriate here, silent failures make production debugging impossible.

Current code:

try {
  window.sessionStorage.setItem(pendingLocaleRefreshKey, locale);
} catch {
  // 存储失败不影响路由切换,只会跳过额外的 RSC 刷新兜底。
}

Suggested fix: Add minimal logging to aid troubleshooting:

} catch (error) {
  // 存储失败不影响路由切换,只会跳过额外的 RSC 刷新兜底。
  console.warn("Failed to persist locale refresh target:", error);
}

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - One minor observation
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Adequate
  • Code clarity - Good

Automated review by Claude AI

const value = window.sessionStorage.getItem(pendingLocaleRefreshKey);
return locales.some((locale) => locale === value) ? (value as Locale) : null;
} catch {
return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [ERROR-SILENT] sessionStorage errors are swallowed in locale refresh guard

Why this is a problem: sessionStorage can throw (privacy mode/quota/blocked storage). The new catch {} blocks silently ignore failures, which can disable the refresh guard without any diagnostic signal.

Evidence:

  • src/components/ui/language-switcher.tsx:33
    } catch {
      return null;
    }
  • src/components/ui/language-switcher.tsx:45
    } catch {
      // 存储失败不影响路由切换,只会跳过额外的 RSC 刷新兜底。
    }
  • src/components/ui/language-switcher.tsx:57
    } catch {
      // 清理失败可忽略,下一次读取会重新校验 locale 是否有效。
    }

Suggested fix:

  } catch (error) {
    console.error("Failed to read pending locale refresh target:", error);
    return null;
  }

  // ...

  } catch (error) {
    console.error("Failed to persist pending locale refresh target:", error);
  }

  // ...

  } catch (error) {
    console.error("Failed to clear pending locale refresh target:", error);
  }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复:read / persist / clear 三处 sessionStorage 失败现在都会输出 console.error 诊断信息;同时 remount 兜底优先使用本轮语言切换的内存 active marker,storage 被浏览器策略禁用时仍能在 provider locale 追上后执行 router.refresh。新增 blocked storage 回归测试。验证:focused i18n tests、lint:fix、lint、typecheck、build、full test 均通过。

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

PR updates many server-rendered dashboard/settings pages to pass locale explicitly into getTranslations, and adds a LanguageSwitcher refresh guard to force a router.refresh() after locale switches. The approach targets stale server-rendered chrome, but the new client-side storage helpers swallow sessionStorage exceptions, which can mask failures and make regressions harder to diagnose.

PR Size: L

  • Lines changed: 429
  • Files changed: 28
  • Split suggestion: (1) server translation parameterization + locale-server-translations regression test, (2) LanguageSwitcher refresh guard + language-switcher unit tests

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 1 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Critical Issues (Must Fix)

None.

High Priority Issues (Should Fix)

  • src/components/ui/language-switcher.tsx:33 [ERROR-SILENT] catch {} blocks swallow sessionStorage failures (Confidence: 90)

Review Coverage

  • Logic and correctness
  • Security (OWASP Top 10)
  • Error handling
  • Type safety
  • Documentation accuracy
  • Test coverage
  • Code clarity

Automated review by Codex AI

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Reviewed PR #1116 (fix(i18n): refresh server chrome on locale switch) and pulled full diff + file context.
  • Applied PR size label size/L (429 lines changed, 28 files).
  • Left 1 inline review comment on src/components/ui/language-switcher.tsx:33 for [ERROR-SILENT] (catch {} swallowing sessionStorage failures) with a concrete fix (capture error and log).
  • Submitted the required review summary (includes L-size split suggestion: split server getTranslations({ locale, ... }) mechanical updates vs. LanguageSwitcher refresh guard + tests).

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/ui/__tests__/language-switcher.test.tsx (1)

75-86: 可选:考虑重置 language-switcher 模块级状态,防止测试顺序耦合。

language-switcher.tsx 中的 activePendingLocaleRefreshTarget 是模块级可变变量,仅在组件刷新流程走完时由 clearPendingLocaleRefreshTarget() 复位。当前三个用例都能让该变量自然清零,因此顺序执行下不会出问题;但若未来新增用例在中途抛错或 vitest 启用 --shuffle/--isolate=false,该状态可能跨用例泄漏,导致测试 1 在测试 3 之后运行时表现异常。

可在 beforeEach 中通过 vi.resetModules() + 动态 import,或额外手动写入并删除 sessionStorage 标记来强制隔离。非阻塞,可选优化。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/__tests__/language-switcher.test.tsx` around lines 75 - 86,
The module-level mutable variable activePendingLocaleRefreshTarget in
language-switcher.tsx can leak across tests; to prevent order-dependent
failures, reset that module state in your test setup by either calling the
exported clearPendingLocaleRefreshTarget() in beforeEach/afterEach or doing
vi.resetModules() and dynamically re-importing the language-switcher component
so a fresh module instance is used; update the beforeEach in
language-switcher.test.tsx to perform one of these resets (referencing
activePendingLocaleRefreshTarget and clearPendingLocaleRefreshTarget or using
vi.resetModules() + dynamic import) to guarantee isolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/ui/__tests__/language-switcher.test.tsx`:
- Around line 75-86: The module-level mutable variable
activePendingLocaleRefreshTarget in language-switcher.tsx can leak across tests;
to prevent order-dependent failures, reset that module state in your test setup
by either calling the exported clearPendingLocaleRefreshTarget() in
beforeEach/afterEach or doing vi.resetModules() and dynamically re-importing the
language-switcher component so a fresh module instance is used; update the
beforeEach in language-switcher.test.tsx to perform one of these resets
(referencing activePendingLocaleRefreshTarget and
clearPendingLocaleRefreshTarget or using vi.resetModules() + dynamic import) to
guarantee isolation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b38d7183-bc07-4a30-a125-77611c2307fa

📥 Commits

Reviewing files that changed from the base of the PR and between fd02226 and ec7c703.

📒 Files selected for processing (2)
  • src/components/ui/__tests__/language-switcher.test.tsx
  • src/components/ui/language-switcher.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/ui/language-switcher.tsx

@github-actions
Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/ui/language-switcher.tsx (1)

81-84: storedRefreshTarget 实际上是死代码,可以简化。

由于 storedRefreshTarget 仅在 activePendingLocaleRefreshTarget !== null 时才会从 sessionStorage 读取,而此时 refreshTarget = pendingLocale ?? activePendingLocaleRefreshTarget ?? storedRefreshTarget 必然在 activePendingLocaleRefreshTarget 处短路,storedRefreshTarget 永远不会被消费;当 activePendingLocaleRefreshTarget === null 时它又被显式置为 null。也就是说该变量在两条路径上都不影响最终结果,只增加了一次无意义的 sessionStorage 读取与认知负担。

建议直接移除该读取,逻辑等价且更清晰:

♻️ 建议的简化
   React.useEffect(() => {
-    const storedRefreshTarget =
-      activePendingLocaleRefreshTarget === null ? null : getPendingLocaleRefreshTarget();
-    const refreshTarget = pendingLocale ?? activePendingLocaleRefreshTarget ?? storedRefreshTarget;
+    const refreshTarget = pendingLocale ?? activePendingLocaleRefreshTarget;
 
     if (refreshTarget !== currentLocale) {
       return;
     }

如果保留 storedRefreshTarget 的本意是在模块状态丢失(例如 HMR / 跨标签页恢复)时仍能消费 sessionStorage 兜底,那当前的 active === null ? null : ... 守卫恰好把这个兜底路径关闭了;若确有该需求需调整守卫条件,否则建议直接删除以避免误导后续维护者。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/language-switcher.tsx` around lines 81 - 84,
storedRefreshTarget is dead code because its guarded read never affects
refreshTarget; remove the unnecessary variable and simplify the effect to
compute refreshTarget directly as pendingLocale ??
activePendingLocaleRefreshTarget ?? getPendingLocaleRefreshTarget(), so
sessionStorage is only read as a true fallback. Update the React.useEffect block
to delete the storedRefreshTarget declaration and replace the refreshTarget
assignment accordingly (references: storedRefreshTarget,
activePendingLocaleRefreshTarget, getPendingLocaleRefreshTarget, pendingLocale,
refreshTarget).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/ui/language-switcher.tsx`:
- Around line 81-84: storedRefreshTarget is dead code because its guarded read
never affects refreshTarget; remove the unnecessary variable and simplify the
effect to compute refreshTarget directly as pendingLocale ??
activePendingLocaleRefreshTarget ?? getPendingLocaleRefreshTarget(), so
sessionStorage is only read as a true fallback. Update the React.useEffect block
to delete the storedRefreshTarget declaration and replace the refreshTarget
assignment accordingly (references: storedRefreshTarget,
activePendingLocaleRefreshTarget, getPendingLocaleRefreshTarget, pendingLocale,
refreshTarget).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dc25b393-9f5e-47c6-8543-f3bfa9e42710

📥 Commits

Reviewing files that changed from the base of the PR and between ec7c703 and 74c999f.

📒 Files selected for processing (2)
  • src/components/ui/__tests__/language-switcher.test.tsx
  • src/components/ui/language-switcher.tsx
✅ Files skipped from review due to trivial changes (1)
  • src/components/ui/tests/language-switcher.test.tsx

@github-actions
Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@ding113 ding113 merged commit 1c89e4b into dev Apr 27, 2026
13 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Apr 27, 2026
@github-actions github-actions Bot mentioned this pull request Apr 28, 2026
@ding113 ding113 deleted the fix/desktop-i18n-refresh-20260427 branch May 13, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n area:UX bug Something isn't working size/L Large PR (< 1000 lines) size/M Medium PR (< 500 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant