Uh oh!
There was an error while loading. Please reload this page.
fix(tests): restore react-i18next's global instance between tests - #5960
Merged
Conversation
) Mounting an `I18nProvider` in a test file silently changed what every later provider-less render in that file resolved. Measured three times in the repo, each time worked around by a comment telling the next author to keep providers in a separate file. What leaks is not the language: `createI18n()` calls `instance.use(initReactI18next)`, whose `init` runs react-i18next's `setI18n(instance)` — replacing the module-level default-instance POINTER, and with it the language, resources, namespaces and `dir()`. A `changeLanguage` reset would leave the test's instance installed as the global. `installI18nGlobalReset()` snapshots the pointer before each test file and puts it back after every test. It is called from `vitest.setup.base.ts`, the one file every project's setup leads back to — including the `unit` project, which runs `isolate: false` and therefore leaks across files, not just tests. `I18nProvider`'s runtime behaviour is unchanged; the global fallback that makes `useObjectTranslation()` provider-safe stays exactly as designed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ3NihCHE9LUtHoGxo6A9f
… changeset The comments in GridField.test.tsx, date-locale-channel.test.tsx and timeline-scale-vocabulary-defaults.test.ts each told the next author that mounting a provider changes what later provider-less renders resolve. That is no longer true, and a comment asserting a hazard that no longer exists is worse than none. Each now names the mechanism and the test that enforces it. The changeset has an empty frontmatter: every source file in this change is a test or the harness, so it releases nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ3NihCHE9LUtHoGxo6A9f
…e global `react-hooks/globals` (error) rejects reassigning a variable declared outside the component during render, and it is right to: it is a side effect in render that would misreport the moment React rendered the probe twice. The probe now renders its three resolved values and the assertions read them back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ3NihCHE9LUtHoGxo6A9f
Contributor
✅ 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-warren
marked this pull request as ready for review
August 24, 2026 07:24
Uh oh!
There was an error while loading. Please reload this page.
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#4514
Mounting an
I18nProviderin a test file silently changed what every later provider-less render in that file resolved. Measured three times in this repo, each time worked around by a comment telling the next author to keep providers in a separate file — which is exactly what failed to prevent the next occurrence.What actually leaks — and it is not "the language"
createI18n()(packages/i18n/src/i18n.ts:132) builds a fresh instance and callsinstance.use(initReactI18next).init(...). react-i18next'sinitReactI18nextis a3rdPartymodule whoseinitrunssetI18n(instance), andsetI18nis a plain assignment over a module-level binding:So a mounted provider does not mutate one shared instance's language — it replaces react-i18next's default-instance pointer, carrying that instance's language, resources, namespaces and
dir().Measured in one file, in order:
getI18n()t('common.save')undefinedcommon.savezhprovider保存zh保存← the bugchangeLanguage('en')enSaveThat fourth row is why a language-only reset is not the fix: it leaves the test's instance installed as the global, so a provider-less render that used to return a raw key now returns an
enpack value. Measured with a provider carrying custom resources, afterchangeLanguage('en')the provider's ownprobe.customkey was still reachable through the global, and this is the card's open question answered: more than language leaks.The fix restores the pointer to whatever the file inherited (
undefinedin this repo), which subsumes language, resources, namespaces and direction.Layer
vitest.setup.base.ts, not the two DOM setups. It is the one file every project's setup leads back to (unitdirectly;domviavitest.setup.dom-light.tsx;dom-heavyandapps/consoleviavitest.setup.dom.tsx), and theunitproject needs it more: it runsisolate: false, so react-i18next's module-level pointer is shared across files in a worker.That is measured, not argued. With the reset ablated, the pin file's very first test —
baseline: no global before anything in this file runs— passes when the file runs alone but fails in the fullunitproject, because an earlier file in the same worker (packages/i18n/src/__tests__/i18n.test.tscallscreateI18n()a dozen times) had already installed a global.I18nProvider's runtime behaviour is deliberately unchanged — the card's direction 2 is not taken here. The global fallback is what makesuseObjectTranslation()provider-safe, and that is a product decision, not a harness one.Reproduced before fixed
Injected one
I18nProviderzhcase intopackages/fields/src/widgets/GridField.test.tsxat the hazard-comment site, ~200 lines above the chip test, on today's tree:Same injection, same file, opposite outcome. The injection was reverted both times (it is not in this diff).
Root
package.json/pnpm-lock.yamlAdding
react-i18next@^17.0.11as a root devDependency is required, not incidental:vitest.setup.i18n-global.tsimports it directly and the repo root declares no@object-ui/*dependency, so nothing links it into rootnode_modules— the first run failed withCannot find package 'react-i18next' imported from vitest.setup.i18n-global.ts. This follows existing precedent:react,react-dom,@testing-library/reactand@testing-library/jest-domare already root devDependencies for exactly the same reason (the root setup files import them). The lockfile delta is 3 added lines — it reuses the already-resolved 17.0.11. The type import was dropped in favour ofReturnType<typeof getI18n>so this needs one new dependency rather than two.Enforcement, not documentation
packages/i18n/src/__tests__/global-instance-reset.test.tsx(dom) and…/global-instance-reset.unit.test.ts(unit). Every case is written provider-less after a case that mounted a provider — the arrangement that used to be unsafe. Reverse-verified by ablating theinstallI18nGlobalReset()call: 5 failed / 7 passed (12) ablated vs 12 passed with it in place, reproduced twice.The three hand-written workaround notes (
GridField.test.tsx,date-locale-channel.test.tsx,timeline-scale-vocabulary-defaults.test.ts) each asserted a hazard that no longer exists, so each now names the mechanism and the test that enforces it instead.Gates, each by name, at
238e2fdvitest --project unitTest Files 658 passed (658)·Tests 10053 passed | 1 skippedvitest --project dom-heavyTest Files 30 passed (30)·Tests 310 passed (310)vitest --project domTest Files 1154 passed (1154)·Tests 12843 passed (12843)(at96b9d59)vitest --project @object-ui/consoleTest Files 72 passed (72)·Tests 805 passed (805)(at96b9d59)pnpm type-check:vitest-setuppnpm lint:root26 problems (0 errors, 26 warnings)node scripts/check-lint-coverage.mjslint coverage: 46/46 packages linted, 0 with outstanding errorsnode scripts/check-changeset-presence.mjs5 source file(s) … declares 1 changeset(s)— empty frontmatter, releases nothingnode scripts/check-changeset-no-major.mjsNo changeset declares a major bumpbaac3f4for the same files)All four projects were green at
96b9d59; the only delta to238e2fdis the pin test file itself, re-run green there (12 passed), andunit+dom-heavywere re-run in full at238e2fd. Runs are from the repo root, the canonical invocation the config's guard requires.Cost (the reset runs under every DOM test):
packages/i18n.test.tsx+date-locale-channel.test.tsx, 909 tests, 3 runs each): total 17.20s with vs 17.16s ablated — inside the ±0.9s run-to-run spread. Setup phase +0.89s across the slice.unitproject (658 files): setup 3.46s with vs 2.07s ablated (~2ms/file); wall clock 98.09s vs 100.55s, i.e. unmeasurable. Bounded byisolate: false— the import is paid once per worker, not per file.Generated by Claude Code
Generated by Claude Code