fix(react): allow explicit locale for static server rendering - #3528
floze-the-genius wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough
ChangesServer locale rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The change adds explicit locale support for static server rendering while preserving existing behavior for callers without a locale, and the reported checks cover the affected package and SSG routes. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant StaticRoute
participant LocaleLayout
participant createServerInstance
participant Tolgee
StaticRoute->>LocaleLayout: provide locale route param
LocaleLayout->>createServerInstance: call getTolgee({ locale })
createServerInstance->>Tolgee: create or retrieve locale instance
Tolgee-->>LocaleLayout: return configured instance
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/src/createServerInstance.tsx (1)
38-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSupport explicit locale in the Server
<T>component.Since
getTranslatenow supports an explicitlocaleto bypassgetLocale()(and avoid request-boundheaders()usage that breaks static generation), the Server<T>component will still fail during static generation because it callsgetTranslate()without arguments.Consider adding an optional
localeprop to the server<T>component (e.g., by extendingTPropsor extracting it from the props) so users can safely use<T>in statically generated Server Components.🤖 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 `@packages/react/src/createServerInstance.tsx` around lines 38 - 41, Add an optional locale prop to the server T component’s props and pass it to getTranslate, preserving the existing locale-resolution behavior when the prop is omitted so static generation can bypass request-bound locale detection.
🤖 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.
Outside diff comments:
In `@packages/react/src/createServerInstance.tsx`:
- Around line 38-41: Add an optional locale prop to the server T component’s
props and pass it to getTranslate, preserving the existing locale-resolution
behavior when the prop is omitted so static generation can bypass request-bound
locale detection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: afd64097-7f98-4935-9361-73c3ebb1deb4
📒 Files selected for processing (5)
packages/react/src/createServerInstance.spec.tsxpackages/react/src/createServerInstance.tsxtestapps/next-app-intl/README.mdtestapps/next-app-intl/src/app/[locale]/layout.tsxtestapps/next-app-intl/src/app/[locale]/ssg/page.tsx
4a96ccc to
9432a00
Compare
|
@Barush I refreshed the #3526 fix onto current The GitHub |
| const getTolgee = async () => { | ||
| const locale = await getLocale(); | ||
| const tolgee = await getTolgeeInstance(locale); | ||
| const getTolgee = async (locale?: string) => { |
There was a problem hiding this comment.
I would pass the locale as prop of parameter object, so when we add more optional params in the future, it's no breaking change.
I would recommend the same for the other methods. :)
|
Addressed the review in d5d9e43: getTolgee and getTranslate now take an optional { locale } parameter object, the server T path uses the same object form, and the test, README, and Next layout callers are updated consistently. Prettier, ESLint, @tolgee/react 39/39, generated public declarations, and the Next production build all pass; the example statically generates /en|cs|de|fr/ssg. CodeRabbit and Snyk are green. The refreshed Test workflow is action_required with zero jobs pending maintainer approval, not a code failure. |
2 participants
Problem
Next.js App Router applications that already know the locale from route params still had to invoke the request-bound locale resolver. That resolver calls
headers(), so otherwise deterministicgenerateStaticParamsroutes could not be rendered as SSG pages. This is the limitation confirmed in #3526.Solution
createServerInstancecallers to pass an explicit locale togetTolgeeandgetTranslateTcomponent to accept an explicit locale while preserving every existingTPropsfieldgenerateStaticParamsregression route and example guidanceThe API is additive; calls without a locale keep the existing resolver behavior.
Verification
pnpm --filter @tolgee/react... buildpnpm --filter @tolgee/react test: 38/38 passedpnpm --dir testapps/next-app-intl build: emitted/en/ssg,/cs/ssg,/de/ssg, and/fr/ssgas SSG routesgit diff --checkmainCloses #3526.
AI assistance
I used OpenAI Codex to assist with implementation, testing, and rebase validation. I reviewed and understand the changes.
Summary by CodeRabbit
New Features
Documentation
Tests