Uh oh!
There was an error while loading. Please reload this page.
fix(i18n): hold useObjectLabel's memo when no i18next instance is bound - #5585
Merged
Merged
Conversation
react-i18next rebuilds its return value from a fresh `{}` every render when it
has no instance to bind to, so the `i18n` object `useObjectLabel` keyed its memo
on churned on every render — the memo never held on exactly the no-provider path
`useSafeFieldLabel` advertises (4 distinct objects in 4 renders, against 1 with
an instance), re-keying every consumer memo it feeds.
Pin both memo dependencies to module-level constants while no instance is bound.
The substitution is unobservable: every `t()` call in the module sits inside a
`for (… of getAppNamespaces())` loop, and `getAppNamespaces()` returns `[]` under
exactly the same predicate. When an instance appears the dependencies become the
live values again, so a provider mounting late recomputes exactly once.
Fixes#5564Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 15:39
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5564
Verified at
b492c9796(the final commit on this branch; every gate below ran on that tree).The defect
useSafeFieldLabel()delegates touseObjectLabel(), which memoizes its whole returned object on react-i18next's[t, i18n]. With no i18next instance bound,useTranslationwarnsNO_I18NEXT_INSTANCEand rebuilds its return value out of a fresh empty object every render (const finalI18n = i18n || {}, which then feeds that hook's ownuseMemodeps) — soi18narrives with a new identity each render and the memo never holds.Re-measured here by ablation: 4 distinct returned objects across 4 renders with no instance, 1 with one. The protection is present exactly where it is not needed and absent exactly where
useSafeFieldLabel's docstring says it exists to serve.One refinement to the card's account: only
i18nchurns.tis already stable when unbound — react-i18next returns a module-levelnotReadyTfrom a module-levelnotReadySnapshot. The single churning dependency isi18n.Option 2 was chosen, and why not option 1
Option 1 does not fix the consumer the memo's own docstring names as its reason to exist.
ListView.filterFieldsreaches the hook throughuseListFieldLabel, which callsuseObjectLabel()directly (packages/plugin-list/src/ListView.tsx:683), notuseSafeFieldLabel(). Option 1 only changes the wrapper, soListViewkeeps the defect — along with 20+ other directuseObjectLabel()call sites acrossapp-shell,plugin-dashboard,componentsandreact.packages/plugin-timeline/src/ObjectTimeline.tsx:37even carries its own hand-rolleduseObjectLabel() ?? OBJECT_LABEL_FALLBACKwrapper, which option 1 would miss too.The 27-vs-5 counts from the dispatch were re-verified independently and are exact: the memo returns 27 members,
SAFE_FIELD_LABEL_FALLBACKhas 5. Option 1 would therefore also cost a hand-maintained 27-member parallel shape plus a pin test to stop it drifting. Option 2 needs no parallel shape at all, because there is only one code path.Option 2 also preserves today's return values exactly and changes only identity, which is precisely the defect.
The change
Both memo dependencies are pinned to module-level constants while no instance is bound. The substitution is unobservable rather than merely convenient, and that is the correctness argument:
t()call in the module sits inside afor (... of getAppNamespaces())loop;getAppNamespaces()returns the empty array under exactly the same "is there a usable instance" predicate;tori18nat all.The predicate now has one definition (
hasUsableI18nInstance) used by both the memo key andgetAppNamespaces, so the two cannot drift.NO_INSTANCE_Tstill mirrors react-i18next's own not-readytso behaviour would not change silently if that reachability argument ever stopped holding.When an instance appears the dependencies become the live values again, so a provider mounting after first render recomputes exactly once. On the bound path the dependency list is literally
[t, i18n]as before — no behaviour change where the memo already held.The now-false comment on
useSafeFieldLabel("It already returns a stable memoized object") is corrected.No API change: no new exports, no signature changes, identical returned surface on both paths.
Tests
packages/i18n/src/__tests__/useObjectLabel-identity-5564.test.tsx, 5 cases, all asserting on identity / recompute counts rather than rendered output — nothing renders wrong today, so a rendering assertion is green against the broken code.fieldLabelidentity)useSafeFieldLabelobjectLabelis callable unbound — this guardsrecord:reference-rail, which doesuseSafeFieldLabel().objectLabel(...)and would get "objectLabel is not a function" if the no-instance path were ever narrowed to the 5-member fallbackThe tests deliberately detach react-i18next's process-global instance and assert that precondition, because
createI18nregisters itself globally viainitReactI18next; without that, a probe meant to run unbound would silently bind to an instance from an earlier test and assert nothing.Reverse-verification
Fix committed first, then reverted (source only) under a
trap ... EXIT INT TERMrestore. Mutation confirmed on disk with anchored counts in both directions (fix markers 0, pre-fix gate and destructure 1 each) plusgit diff --stat.Ablated result —
VERDICT command-exit 1, 3 failed / 2 passed:The first two reproduce the card's measurement exactly. The two that stayed green are the right two — the with-instance guard and the shape pin — which is what makes this an ablation rather than a blanket break. Restored and re-run green;
git status --porcelainempty, so the tree is byte-identical.No dist is in the loop: the test imports the ablation target by relative path from source, so there is no build step whose staleness could fake the result.
Gates
All at
b492c9796, exit codes captured before any pipe:pnpm --filter '@object-ui/i18n^...' buildpackages/core build: Done)pnpm --filter @object-ui/i18n type-checktsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/i18n lint34 problems (0 errors, 34 warnings)— all warnings pre-existinganyusages outside this diff; the new test file contributes nonepnpm exec vitest run packages/i18n/Test Files 50 passed (50)/Tests 878 passed (878)pnpm exec vitest run packages/plugin-detail/Test Files 90 passed (90)/Tests 848 passed (848)pnpm exec vitest run packages/plugin-dashboard/Test Files 72 passed (72)/Tests 653 passed (653)node scripts/check-control-bytes.mjscheck-control-bytes: OK (scanned 4638 tracked text file(s))node scripts/check-changeset-presence.mjs2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjsNo changeset declares a major bump.node scripts/check-changeset-fixed.mjsAll workspace packages are in the changeset fixed group.Both consumer packages were run deliberately:
plugin-detailholds the two whole-objectuseSafeFieldLabel()consumers the dispatch flagged (record-reference-railcallingi18n.objectLabel(...), andrecord-related-list), so it is the package a shape change would break first;plugin-dashboardholdsObjectDataTable, the consumer whosederivedColumnsrecomputation the card measured.Declared narrowing
pnpm lintwas not run;pnpm --filter @object-ui/i18n lintwas. This cannot hide a failure: the diff touches onlypackages/i18n, andeslint.config.jsenables no type-aware linting (noprojectService, noparserOptions.project), so this change cannot move the verdict on any file it does not contain.plugin-list,plugin-grid,plugin-form,app-shell,plugin-charts,plugin-timeline,components,react,fields,plugin-report,plugin-kanbanorplugin-tree. CI runs the full farm.Notes
packages/plugin-charts/src/ObjectChart.tsx:268-271carries a hand-rolled ref workaround for this exact defect ("the i18n hook returns a fresh function reference on every render ... trigger an infinite refetch loop"). This fix makes that workaround removable —fieldOptionLabelis now identity-stable with or without a provider. Not removed here; it is outside this surface and wants its own card. The same applies toplugin-timeline'suseSafeObjectLabel.packages/i18n/src/useObjectLabel.ts, its tests, and one changeset. No consumer package was modified.Generated by Claude Code