Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/i18n-global-reset-4514.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
---
---

Test harness only, no published behaviour change (objectui#4514).

Mounting an `I18nProvider` in a test file left react-i18next's global
default-instance pointer on that provider's instance, so every later
provider-less render in the same file resolved through it — a failure that
surfaced hundreds of lines away, in a test nobody had touched, and passed when
run alone. `vitest.setup.base.ts` now restores the pointer after every test.

`I18nProvider` and `useObjectTranslation()` are unchanged: the global fallback
that makes the hook provider-safe stays exactly as designed. The five source
files in this change are all `*.test.*` plus their harness.
1 change: 1 addition & 0 deletions package.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,7 @@
"playwright": "^1.62.1",
"react": "19.2.8",
"react-dom": "19.2.8",
"react-i18next": "^17.0.11",
"react-router-dom": "^7.18.2",
"rollup-plugin-visualizer": "^7.1.1",
"tailwindcss": "^4.3.3",
Expand Down
15 changes: 9 additions & 6 deletions packages/fields/src/__tests__/date-locale-channel.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -291,12 +291,15 @@ describe('channel precedence is unchanged (green both sides)', () => {
* The third step of the precedence — the provider-less `'en'` last resort —
* is pinned in `DateCellRenderer.test.tsx`, deliberately NOT here.
*
* It cannot be measured in this file: `useObjectTranslation()` outside a
* provider reports `i18n.language` from react-i18next's GLOBAL instance, and
* every `I18nProvider` mounted above leaves that global on the language it
* was given. So a provider-less render placed after these cases resolves
* `'zh'` — which is the state react-i18next is in, not the fallback under
* test. Written here it would assert a fact about test ordering.
* It USED to be unmeasurable in this file: `useObjectTranslation()` outside
* a provider reports `i18n.language` from react-i18next's GLOBAL instance,
* and every `I18nProvider` mounted above left that global on its own
* instance. So a provider-less render placed after these cases resolved
* `'zh'` — the state react-i18next was in, not the fallback under test.
*
* objectui#4514 closed that: `installI18nGlobalReset()` in
* `vitest.setup.base.ts` restores the global after every test. The split
* below is now organisation, not a workaround.
*
* `DateCellRenderer.test.tsx` mounts no provider at all, so its
* `6 days ago` / `Overdue 6d` / `Tomorrow` cases ARE that pin, and they hold
Expand Down
14 changes: 10 additions & 4 deletions packages/fields/src/widgets/GridField.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -332,10 +332,16 @@ describe('GridField / LineItemsField — editable line items', () => {

// The `zh` half of this — the sub-grid following the SESSION locale
// (objectui#4468) — lives in `__tests__/date-locale-channel.test.tsx`,
// not here. Mounting an `I18nProvider` anywhere in THIS file changes what
// the provider-less renders further down resolve, and the file-columns
// chip test reads a translated `aria-label`; keeping the provider in its
// own file keeps that coupling out of the way.
// grouped with the rest of the locale cases.
//
// It used to be a HAZARD rather than a preference: mounting an
// `I18nProvider` anywhere in this file left react-i18next's global on
// that provider's instance, so the file-columns chip test ~200 lines
// down read a Chinese `aria-label` and went red. objectui#4514 fixed
// that in the harness — `installI18nGlobalReset()` in
// `vitest.setup.base.ts` restores the global after every test, and
// `packages/i18n/src/__tests__/global-instance-reset.test.tsx` fails if
// it stops. A provider is safe to mount here now.
});
});

Expand Down
153 changes: 153 additions & 0 deletions packages/i18n/src/__tests__/global-instance-reset.test.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#4514 — mounting an `I18nProvider` must not change what a LATER
* provider-less render in the same file resolves.
*
* ── Why this file exists rather than a comment ───────────────────────────
* The trap was "documented where tests are written" three separate times —
* `packages/fields/src/widgets/GridField.test.tsx`,
* `packages/fields/src/__tests__/date-locale-channel.test.tsx` and
* `packages/plugin-timeline/src/__tests__/timeline-scale-vocabulary-defaults.test.ts`
* each carry a paragraph telling the next author to keep providers in a
* separate file. It still recurred, because a comment cannot fail. The
* mechanism is `installI18nGlobalReset()` in `vitest.setup.base.ts`; THIS file
* is what fails if it is removed or stops working.
*
* ── The ordering is the assertion ────────────────────────────────────────
* Every case below is written provider-LESS *after* a case that mounted a
* provider, which is exactly the arrangement that used to be unsafe.
*
* Reverse-verified by ablating the `installI18nGlobalReset()` call in
* `vitest.setup.base.ts` and re-running this file together with its `unit`
* sibling: 5 failed / 7 passed (12), against 12 passed with the call in place.
* The five are the four cases marked "THE PIN" plus the provider-safe-fallback
* case — which is the point: without the reset that fallback resolves whatever
* the previous case installed, not `'en'`.
*/

import { describe, it, expect } from 'vitest';
import React from 'react';
import { render, screen } from '@testing-library/react';
import { getI18n } from 'react-i18next';
import { I18nProvider, useObjectTranslation } from '../index';

/**
* Renders what it resolved instead of assigning to a module variable — writing
* to one during render is a side effect in render, which `react-hooks/globals`
* rejects (and would be a real hazard the moment React re-rendered this twice).
*/
function Probe() {
const { t, language } = useObjectTranslation();
return (
<dl>
<dd data-testid="probe-language">{language}</dd>
<dd data-testid="probe-save">{t('common.save')}</dd>
{/* A key no pack defines: pins the `defaultValue` path, which is what a
provider-less host actually renders (FileField's `aria-label` etc.). */}
<dd data-testid="probe-missing">
{t('probe.no.such.key', { defaultValue: 'INLINE-DEFAULT' })}
</dd>
</dl>
);
}

function readProbe() {
return {
language: screen.getByTestId('probe-language').textContent,
save: screen.getByTestId('probe-save').textContent,
missing: screen.getByTestId('probe-missing').textContent,
};
}

/** The value a provider-less render resolves in a pristine file. */
const PRISTINE = { language: 'en', save: 'common.save', missing: 'INLINE-DEFAULT' };

function renderZhProvider(node: React.ReactNode) {
return render(
<I18nProvider
config={{ defaultLanguage: 'zh', detectBrowserLanguage: false }}
persistLanguage={false}
>
{node}
</I18nProvider>,
);
}

describe('objectui#4514 — the react-i18next global does not leak between tests', () => {
it('baseline: with no provider anywhere, the global is unset', () => {
expect(getI18n()).toBeUndefined();
render(<Probe />);
expect(readProbe()).toEqual(PRISTINE);
});

it('a mounted zh provider translates inside its own subtree', () => {
renderZhProvider(<Probe />);
expect(readProbe()).toEqual({ language: 'zh', save: '保存', missing: 'INLINE-DEFAULT' });
// ...and it really did install itself as the global while mounted. This is
// the deliberate design (direction 2 of the card, NOT taken): the global
// fallback is what makes `useObjectTranslation()` provider-safe.
expect(getI18n()).toBeDefined();
expect(getI18n().language).toBe('zh');
});

it('THE PIN: a provider-less render after that zh case still resolves pristine', () => {
// Before objectui#4514 this resolved `zh` / `保存` — the state react-i18next
// was left in by the case above, ~1 screen up. That is the whole defect.
expect(getI18n()).toBeUndefined();
render(<Probe />);
expect(readProbe()).toEqual(PRISTINE);
});

it('THE PIN: more than the language is restored', () => {
render(
<I18nProvider
config={{
defaultLanguage: 'ar',
detectBrowserLanguage: false,
resources: { ar: { 'probe.custom': 'FROM-THE-PROVIDER' } },
}}
persistLanguage={false}
>
<Probe />
</I18nProvider>,
);
const mounted = getI18n();
// While mounted, the provider's own resources and RTL direction are on the
// global — this is the part a `changeLanguage('en')` reset would NOT undo.
expect(mounted.t('probe.custom')).toBe('FROM-THE-PROVIDER');
expect(mounted.dir()).toBe('rtl');
});

it('THE PIN: the provider above left no resources, no direction, no instance', () => {
// A language-only reset would leave `probe.custom` reachable here (measured:
// it resolves through the instance's `en` resources instead of vanishing).
// Restoring the POINTER is what makes the leak total rather than partial.
expect(getI18n()).toBeUndefined();
render(<Probe />);
expect(readProbe()).toEqual(PRISTINE);
});

it('the provider-safe fallback itself still works (must-not-break)', () => {
// `useObjectTranslation()` outside a provider must not throw, must report a
// language, and must serve `defaultValue` — that is the deliberate design at
// packages/i18n/src/provider.tsx (`context?.language || i18n.language || 'en'`).
// The reset must not destroy it, only make it deterministic.
expect(() => render(<Probe />)).not.toThrow();
expect(readProbe().language).toBe('en');
expect(readProbe().missing).toBe('INLINE-DEFAULT');
});

it('a provider still works when mounted AFTER all of the above', () => {
// The reset restores a pointer; it must not leave react-i18next in a state
// where a later provider cannot install itself.
renderZhProvider(<Probe />);
expect(readProbe()).toEqual({ language: 'zh', save: '保存', missing: 'INLINE-DEFAULT' });
});
});
58 changes: 58 additions & 0 deletions packages/i18n/src/__tests__/global-instance-reset.unit.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#4514, the `unit` project's half.
*
* The sibling `global-instance-reset.test.tsx` pins the DOM projects. This one
* exists because the `unit` project is where the same leak is WORSE, and it is
* easy to assume node-env tests are out of reach of a React-flavoured bug:
*
* - `createI18n()` installs react-i18next's global from plain node code — no
* React, no render, no provider needed. `instance.use(initReactI18next)`
* is what does it, and `packages/i18n/src/__tests__/i18n.test.ts` calls
* `createI18n()` a dozen times.
* - the `unit` project runs `isolate: false` (vitest.config.mts), so its
* module graph — react-i18next's module-level pointer included — is shared
* across FILES in a worker. Uncontained, an instance installed by one file
* is still the global for the next file that worker picks up.
*
* A cross-file assertion would depend on worker assignment and file ordering,
* so this pins the per-test guarantee that makes the cross-file one hold.
*/

import { describe, it, expect } from 'vitest';
import { getI18n } from 'react-i18next';
import { createI18n } from '../i18n';

describe('objectui#4514 — createI18n does not leave a global behind (unit project)', () => {
it('baseline: no global before anything in this file runs', () => {
expect(getI18n()).toBeUndefined();
});

it('createI18n installs itself as the react-i18next global', () => {
const instance = createI18n({ defaultLanguage: 'zh', detectBrowserLanguage: false });
// Not incidental — this is `initReactI18next`'s documented job, and the
// reason `useObjectTranslation()` is provider-safe.
expect(getI18n()).toBe(instance);
expect(getI18n().language).toBe('zh');
});

it('THE PIN: the next test sees no global at all', () => {
expect(getI18n()).toBeUndefined();
});

it('THE PIN: holds for a second instance too', () => {
createI18n({ defaultLanguage: 'ja', detectBrowserLanguage: false });
expect(getI18n().language).toBe('ja');
});

it('THE PIN: and again after that one', () => {
expect(getI18n()).toBeUndefined();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,13 +12,16 @@
*
* ── Why this file mounts nothing ─────────────────────────────────────────
* There is no React here at all — no `render`, no provider, not even an import
* of one. That is deliberate (objectui#4514): the sibling
* of one. That began as a workaround: the sibling
* `timeline-scale-vocabulary.test.tsx` mounts an `I18nProvider` in every case,
* and every mounted provider leaves react-i18next's GLOBAL instance on the
* language it was given, so a provider-less assertion sharing that file would
* resolve `'zh'` and pin test ordering instead of the fallback. Keeping the
* pure-function cases in their own file makes the separation structural rather
* than a comment someone has to remember.
* and a mounted provider used to leave react-i18next's GLOBAL instance on its
* own instance, so a provider-less assertion sharing that file resolved `'zh'`
* and pinned test ordering instead of the fallback.
*
* objectui#4514 fixed the harness — `installI18nGlobalReset()` in
* `vitest.setup.base.ts` restores the global after every test — so the split is
* no longer load-bearing. It stays because a pure-function file that imports no
* React is worth having on its own merits.
*
* ── What the seam is ─────────────────────────────────────────────────────
* `generateTimeScaleHeaders` is a pure exported function, so it cannot host a
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions vitest.setup.base.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,28 @@
*/

import { vi } from 'vitest';
import { installI18nGlobalReset } from './vitest.setup.i18n-global';

// objectui#4514 — put react-i18next's GLOBAL default-instance pointer back
// after every test, so a provider-less render resolves the same way whether it
// sits above or below an `I18nProvider` in the same file.
//
// It lives HERE, in the one file every project's setup leads back to (`unit`
// directly; `dom` via vitest.setup.dom-light.tsx; `dom-heavy` and apps/console
// via vitest.setup.dom.tsx), rather than in the two DOM setups, for two
// measured reasons:
//
// 1. Both DOM setups already import this file, so one call covers them; two
// calls would be two places for a third DOM project to forget.
// 2. The `unit` project needs it MORE, not less. It runs `isolate: false`, so
// its module graph — including react-i18next's module-level pointer — is
// shared across FILES in a worker. `packages/i18n/src/__tests__/i18n.test.ts`
// alone calls `createI18n()` a dozen times, and each call installs a new
// global. There the leak outlives the file that caused it.
//
// Cost of the import here is bounded by the same `isolate: false`: the unit
// project pays react-i18next once per worker, not once per file.
installI18nGlobalReset();

// Polyfill ResizeObserver (Radix UI / Shadcn components reference it even in
// node-env unit tests that import components transitively).
Expand Down
Loading
Loading