refactor: share binding utilities across react-core and gt-vue via gt-i18n#1784
refactor: share binding utilities across react-core and gt-vue via gt-i18n#1784ErnestM1234 wants to merge 1 commit into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (inFlightTranslations.has(key)) return; | ||
| inFlightTranslations.add(key); | ||
| void Promise.resolve() | ||
| .then(() => | ||
| getI18nCache().lookupTranslationWithFallback( | ||
| lookup.locale, | ||
| lookup.message, | ||
| lookup.options as Parameters< | ||
| ReturnType<typeof getI18nCache>['lookupTranslationWithFallback'] | ||
| >[2] | ||
| ) | ||
| ) | ||
| .catch(() => undefined) | ||
| .then(() => onSettled()); |
There was a problem hiding this comment.
Once a key is added to inFlightTranslations, it remains there permanently — including after the promise rejects. If a translation fetch fails transiently in dev (e.g., a momentary network error or a bad API key), the onSettled callback fires and triggers a re-render, but subsequent renders silently skip re-queuing because the key is still in the Set. The translation stays missing for the rest of the session with no retry. This was the same behaviour in the old reactivity.ts, so it's not a regression introduced here, but now that the logic is shared and more prominent it's worth a deliberate inFlightTranslations.delete(key) after settling (at least on the failure path) if retries in dev are desirable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/i18n/src/binding/runtimeTranslationQueue.ts
Line: 41-54
Comment:
**In-flight keys never removed after settling**
Once a key is added to `inFlightTranslations`, it remains there permanently — including after the promise rejects. If a translation fetch fails transiently in dev (e.g., a momentary network error or a bad API key), the `onSettled` callback fires and triggers a re-render, but subsequent renders silently skip re-queuing because the key is still in the Set. The translation stays missing for the rest of the session with no retry. This was the same behaviour in the old `reactivity.ts`, so it's not a regression introduced here, but now that the logic is shared and more prominent it's worth a deliberate `inFlightTranslations.delete(key)` after settling (at least on the failure path) if retries in dev are desirable.
How can I resolve this? If you propose a fix, please make it concise.
| .find((row) => row.startsWith(`${cookieName}=`)) | ||
| ?.split('=')[1]; | ||
| return rawCookieValue; |
There was a problem hiding this comment.
=
?.split('=')[1] silently drops everything after the first = in the value. For locale tags this is harmless today, but the function is named getCookieValue generically and could be called with cookies whose values are base64-encoded or contain query-string-style data (both of which legally contain =). A safer parse is row.slice(cookieName.length + 1) (after the matched cookieName= prefix), or split('=').slice(1).join('='). This bug was present in both the gt-react and gt-vue originals, so it's not new here, but consolidating it into a shared utility is a good time to fix it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/i18n/src/binding/browserCookies.ts
Line: 44-46
Comment:
**Cookie value truncated at first `=`**
`?.split('=')[1]` silently drops everything after the first `=` in the value. For locale tags this is harmless today, but the function is named `getCookieValue` generically and could be called with cookies whose values are base64-encoded or contain query-string-style data (both of which legally contain `=`). A safer parse is `row.slice(cookieName.length + 1)` (after the matched `cookieName=` prefix), or `split('=').slice(1).join('=')`. This bug was present in both the gt-react and gt-vue originals, so it's not new here, but consolidating it into a shared utility is a good time to fix it.
How can I resolve this? If you propose a fix, please make it concise.
13dd724 to
06aff4d
Compare
…-i18n Adds a framework-agnostic binding layer to gt-i18n's internal entrypoint (packages/i18n/src/binding/): plural branch selection, variable naming and formatting cores (Num/Currency/DateTime/RelativeTime), format-locale resolution, the dev hot reload runtime translation queue, msg() resolution, dictionary translation resolution, and browser cookie helpers. react-core's copies become re-exports/thin adapters (public surface and behavior unchanged), gt-react's cookie modules re-export, and gt-vue consumes the shared implementations directly.
c15c36e to
6b1ded2
Compare
1 participant
Summary
Follow-up to #1774 (stacked on
e/odysseus/gt-vue). gt-vue's initial implementation duplicated a chunk of framework-free logic that already existed in react-core. This PR extracts that logic intogt-i18n's internal entrypoint (packages/i18n/src/binding/) — the natural home, since both bindings already depend on it — and points both bindings at the shared implementations.Now shared via
gt-i18n/internal:getPluralBranch— plural-form branch selectiongetFormatLocales/getShouldTranslate— format-locale chain resolutiongetVariableName+baseVariablePrefix— wire-format variable namingcomputeNum/computeCurrency/computeDateTime/computeRelativeTime— the variable formatting corescreateRuntimeTranslationQueue— the dev hot reload primitive (deduped runtime translation requests + settle notification)createMFunction—msg()resolutionresolveDictionaryEntryTranslation/resolveDictionaryObjectTranslation— dictionary resolution (binding supplies its resolvers + miss handling)getCookieValue/setCookieValue/readBrowserLocaleConsumers:
children,_locale) onto the shared signatures — public surface and behavior unchanged, no serialization/tagging code touched (translation hashes unaffected)defaultLocaleCookieNameetc. preserved)Deliberately not shared here: the tree pipeline (tag → serialize → reconcile). It's the same algorithm over different element models and would need an element-adapter abstraction plus a react-core refactor with hash-stability testing — deferred until a third binding needs it.
Testing
vite-vuetest app against this branch: dev 16/16 (locale selector switching en→fr→zh with no reload,<T>/useGT()/t()/<Plural>translated on the fly through the shared hot reload queue) and prod 15/15 (translations from local files)BrowserConditionStore.test.ts's fullgt-i18n/internalmock now spreadsimportOriginal(the module gained members)Notes
gt-i18n,@generaltranslation/react-core,gt-react,gt-vuesrc/binding/directory but ship through the existing/internalentrypoint — no export-map, lint-boundary, or size-limit changes needed; they can be split into a dedicatedgt-i18n/bindingentrypoint later if preferredGreptile Summary
This PR extracts duplicated framework-agnostic binding logic from
react-coreandgt-vueinto a newpackages/i18n/src/binding/directory, exposed throughgt-i18n/internal. Both bindings now share plural-branch selection, variable formatting cores, format-locale resolution, the dev hot-reload queue,msg()resolution, dictionary translation helpers, and browser cookie utilities instead of maintaining separate copies.gt-i18n/src/binding/and are re-exported fromgt-i18n/internal; react-core files become thin re-exports or one-call adapters with no public surface change.gt-vuelocal copies are deleted and replaced with direct imports fromgt-i18n/internal;gt-react's cookie and cookie-name modules become re-exports.BrowserConditionStoreis updated to spreadimportOriginalso that the new exports don't break the mock.Confidence Score: 4/5
Safe to merge; the refactor is a clean deduplication with no changes to public API surfaces, serialization formats, or translation hashes.
All public behaviour is preserved — react-core and gt-vue are thin wrappers over the shared implementations and the logic is equivalent. The two observations (in-flight Set cleanup and cookie-value truncation) are pre-existing bugs being consolidated rather than new defects, and neither affects locale tag values in practice. Tests across the full affected graph pass.
runtimeTranslationQueue.ts and browserCookies.ts carry the pre-existing limitations noted in comments; everything else in the changeset is straightforward.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph gt-i18n/internal B1[getPluralBranch] B2[getFormatLocales / getShouldTranslate] B3[getVariableName / baseVariablePrefix] B4[computeNum / computeCurrency\ncomputeDateTime / computeRelativeTime] B5[createRuntimeTranslationQueue] B6[createMFunction] B7[resolveDictionaryEntryTranslation\nresolveDictionaryObjectTranslation] B8[getCookieValue / setCookieValue\nreadBrowserLocale / cookie names] end subgraph react-core RC1[getPluralBranch.ts - re-export] RC2[getFormatLocales.ts - re-export] RC3[getVariableName.ts - thin adapter] RC4[Currency/Num/DateTime/RelativeTime.shared.ts] RC5[useMessages.ts] RC6[useTranslations.ts] end subgraph gt-react GR1[cookie-names.ts - re-export] GR2[cookies.ts - re-export] GR3[readBrowserLocale.ts - re-export] end subgraph gt-vue GV1[branches.ts] GV2[variables.ts] GV3[dictionary.ts] GV4[translate.ts] GV5[condition-store.ts] GV6[reactivity.ts] end B1 --> RC1 B2 --> RC2 B3 --> RC3 B4 --> RC4 B6 --> RC5 B7 --> RC6 B8 --> GR1 B8 --> GR2 B8 --> GR3 B1 --> GV1 B2 --> GV1 B4 --> GV2 B7 --> GV3 B6 --> GV4 B2 --> GV4 B8 --> GV5 B5 --> GV6%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD subgraph gt-i18n/internal B1[getPluralBranch] B2[getFormatLocales / getShouldTranslate] B3[getVariableName / baseVariablePrefix] B4[computeNum / computeCurrency\ncomputeDateTime / computeRelativeTime] B5[createRuntimeTranslationQueue] B6[createMFunction] B7[resolveDictionaryEntryTranslation\nresolveDictionaryObjectTranslation] B8[getCookieValue / setCookieValue\nreadBrowserLocale / cookie names] end subgraph react-core RC1[getPluralBranch.ts - re-export] RC2[getFormatLocales.ts - re-export] RC3[getVariableName.ts - thin adapter] RC4[Currency/Num/DateTime/RelativeTime.shared.ts] RC5[useMessages.ts] RC6[useTranslations.ts] end subgraph gt-react GR1[cookie-names.ts - re-export] GR2[cookies.ts - re-export] GR3[readBrowserLocale.ts - re-export] end subgraph gt-vue GV1[branches.ts] GV2[variables.ts] GV3[dictionary.ts] GV4[translate.ts] GV5[condition-store.ts] GV6[reactivity.ts] end B1 --> RC1 B2 --> RC2 B3 --> RC3 B4 --> RC4 B6 --> RC5 B7 --> RC6 B8 --> GR1 B8 --> GR2 B8 --> GR3 B1 --> GV1 B2 --> GV1 B4 --> GV2 B7 --> GV3 B6 --> GV4 B2 --> GV4 B8 --> GV5 B5 --> GV6Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "refactor: share binding utilities across..." | Re-trigger Greptile