Skip to content

refactor(i18n): about locales#83

Open
tomerqodo wants to merge 11 commits intogreptile_combined-20260114-qodo-grep-copilot_base_refactori18n_about_locales_pr279from
greptile_combined-20260114-qodo-grep-copilot_head_refactori18n_about_locales_pr279
Open

refactor(i18n): about locales#83
tomerqodo wants to merge 11 commits intogreptile_combined-20260114-qodo-grep-copilot_base_refactori18n_about_locales_pr279from
greptile_combined-20260114-qodo-grep-copilot_head_refactori18n_about_locales_pr279

Conversation

@tomerqodo
Copy link

Benchmark PR from qodo-benchmark#279

@greptile-apps
Copy link

greptile-apps bot commented Jan 15, 2026

Greptile Summary

This PR refactors the i18n (internationalization) architecture from React Context API to Jotai atom-based state management.

Key Changes:

  • Replaced I18NContext with localeAtom using Jotai for client-side locale state
  • Removed the withI18N HOC and context provider wrapper
  • Added server-only-context.ts utility for request-scoped server-side caching using React's cache() API
  • Implemented singleton i18n instance with per-request caching in i18n-config/server.ts
  • Updated 70 components and test files to use useLocale() hook instead of useContext(I18n)
  • Tests updated to mock useLocale instead of wrapping components with context providers

Critical Issue Found:

  • The useDocLink function in web/context/i18n.ts was changed to use pathMap[docLanguage] instead of pathMap[locale]. This breaks existing usage where pathMap keys are locale strings like 'zh-Hans' but docLanguage returns lowercase values like 'zh-hans'. This will cause Chinese doc links to fail.

Confidence Score: 2/5

  • This PR contains a critical bug in the useDocLink function that will break documentation links for non-English locales.
  • Score reflects a well-architected refactoring with comprehensive test updates, but a critical logic error in pathMap lookup will cause runtime failures for internationalized doc links. The bug is localized and fixable, but must be addressed before merge.
  • Pay close attention to web/context/i18n.ts - the pathMap lookup logic must be reverted or all pathMap usage sites must be updated to use lowercase locale keys.

Important Files Changed

Filename Overview
web/context/i18n.ts Refactored from Context API to Jotai atoms for locale state management. Critical bug: pathMap now uses docLanguage instead of locale, breaking existing usage.
web/i18n-config/server.ts Added server-side caching for i18n instance and locale detection using serverOnlyContext. Improved performance by avoiding redundant instance creation.
web/utils/server-only-context.ts New utility for server-side request-scoped state using React cache. Clean implementation for managing per-request context.
web/app/components/i18n.tsx Replaced Context Provider with Jotai's useHydrateAtoms. Simplified component by removing unnecessary context wrapper.
web/package.json Added jotai@^2.16.1 dependency for atom-based state management.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant ServerComponent
    participant getLocaleOnServer
    participant serverOnlyContext
    participant getTranslation
    participant I18nInstance
    participant ClientComponent
    participant JotaiAtom

    Note over Browser,JotaiAtom: Server-Side Rendering Phase
    Browser->>ServerComponent: Request page
    ServerComponent->>getLocaleOnServer: Get locale for request
    getLocaleOnServer->>serverOnlyContext: Check cached locale
    alt No cache
        getLocaleOnServer->>getLocaleOnServer: Read cookies/headers
        getLocaleOnServer->>getLocaleOnServer: Match locale
        getLocaleOnServer->>serverOnlyContext: Cache locale
    end
    getLocaleOnServer-->>ServerComponent: Return locale

    ServerComponent->>getTranslation: Get translations (locale, namespace)
    getTranslation->>serverOnlyContext: Check cached i18n instance
    alt No instance
        getTranslation->>I18nInstance: Create shared instance (en-US)
        getTranslation->>serverOnlyContext: Cache instance
    end
    getTranslation->>I18nInstance: Load namespace if needed
    getTranslation->>I18nInstance: getFixedT(locale, namespace)
    getTranslation-->>ServerComponent: Return {t, i18n}
    
    ServerComponent-->>Browser: Render HTML with locale

    Note over Browser,JotaiAtom: Client-Side Hydration Phase
    Browser->>ClientComponent: Hydrate React app
    ClientComponent->>JotaiAtom: useHydrateAtoms(localeAtom, locale)
    ClientComponent->>ClientComponent: setLocaleOnClient(locale)
    
    Note over Browser,JotaiAtom: Client-Side Usage
    ClientComponent->>JotaiAtom: useLocale()
    JotaiAtom-->>ClientComponent: Return current locale
    ClientComponent->>ClientComponent: useTranslation() via react-i18next
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

69 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

return (path?: string, pathMap?: { [index: string]: string }): string => {
const pathUrl = path || ''
let targetPath = (pathMap) ? pathMap[locale] || pathUrl : pathUrl
let targetPath = (pathMap) ? pathMap[docLanguage] || pathUrl : pathUrl
Copy link

Choose a reason for hiding this comment

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

logic: Using docLanguage instead of locale breaks existing pathMap usage. The docLanguage returns lowercase values like 'zh-hans' while existing code uses locale keys like 'zh-Hans' (see history-panel.tsx:49).

Suggested change
let targetPath = (pathMap) ? pathMap[docLanguage] || pathUrl : pathUrl
let targetPath = (pathMap) ? pathMap[locale] || pathUrl : pathUrl
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/context/i18n.ts
Line: 29:29

Comment:
**logic:** Using `docLanguage` instead of `locale` breaks existing `pathMap` usage. The `docLanguage` returns lowercase values like `'zh-hans'` while existing code uses locale keys like `'zh-Hans'` (see `history-panel.tsx:49`).

```suggestion
    let targetPath = (pathMap) ? pathMap[locale] || pathUrl : pathUrl
```

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants