-
Couldn't load subscription status.
- Fork 15
refactor(i18n): use typed i18n functions with explicit prop passing #983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ab4e2fc to
49f107a
Compare
49f107a to
3baa561
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
A refactor to improve type safety and explicit handling of translation functions to prevent hydration mismatches between server and client.
- Introduce a typed
TranslationObjectand auseTranslationObjecthook - Expand route-level
handle.i18nnamespaces to includeworkflows - Replace
useTranslationcalls withuseTranslationObjectand pass translation functions explicitly via props
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/app-builder/src/types/i18n.ts | Add TranslationObject type mapping namespaces to typed TFunction. |
| packages/app-builder/src/hooks/useTranslationObject.ts | Add useTranslationObject hook that returns named translation methods. |
| packages/app-builder/src/hooks/useTranslationObject.test.ts | Add comprehensive tests for the new hook. |
| packages/app-builder/src/routes/_builder+/scenarios+/$scenarioId+/home.tsx | Swap out useTranslation for useTranslationObject, update namespaces, and pass translations via translationObject props. |
| packages/app-builder/package.json | Add Testing Library dependencies for the new tests. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/app-builder/src/routes/_builder+/scenarios+/$scenarioId+/home.tsx:292
- The
QuickVersionAccesscomponent expects atranslationObjectprop but it’s not provided here. Pass it explicitly, e.g.:<QuickVersionAccess scenarioIteration={quickVersion} translationObject={{ tScenarios }} />.
{quickVersion ? <QuickVersionAccess scenarioIteration={quickVersion} /> : null}
packages/app-builder/src/routes/_builder+/scenarios+/$scenarioId+/home.tsx:293
- The
QuickVersionAccesscomponent expects atranslationObjectprop but it’s not provided here. Pass it explicitly, e.g.:<QuickVersionAccess scenarioIteration={quickDraft} translationObject={{ tScenarios }} />.
{quickDraft ? <QuickVersionAccess scenarioIteration={quickDraft} /> : null}
| <VersionSection | ||
| translationObject={{ tScenarios }} | ||
| scenarioIterations={scenarioIterations} | ||
| /> | ||
| <section className="flex flex-col gap-4"> | ||
| <h2 className="text-grey-00 text-m font-semibold">{t('scenarios:home.execution')}</h2> | ||
| <h2 className="text-grey-00 text-m font-semibold">{tScenarios('home.execution')}</h2> | ||
| <div className="grid max-w-[1000px] grid-cols-2 gap-8"> | ||
| <RealTimeSection | ||
| translationObject={{ tScenarios }} | ||
| scenarioId={currentScenario.id} | ||
| liveScenarioIteration={liveScenarioIteration} | ||
| /> | ||
| <BatchSection | ||
| translationObject={{ tScenarios }} |
Copilot
AI
Jul 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating the translationObject inline produces a new object on every render, which can trigger unnecessary re-renders of child components. Consider memoizing it with React.useMemo to keep a stable reference.
3baa561 to
4b7cee4
Compare
Purpose
This branch tries to fix one of current issues which are hydration errors due to mismatch of translation namespaces used on server and client.
How
This is actually working on 2 phases:
handle.i18nwhich is also used server sideuseTranslationObject(namespaces)