fix(hooks): stop calling translation hooks inside try/catch (#2879)

.changeset/no-try-catch-around-hook.md

@@ -0,0 +1,51 @@
+---
+"@object-ui/i18n": patch
+"@object-ui/plugin-detail": patch
+"@object-ui/plugin-timeline": patch
+"@object-ui/plugin-list": patch
+"@object-ui/plugin-calendar": patch
+"@object-ui/plugin-grid": patch
+"@object-ui/plugin-designer": patch
+"@object-ui/plugin-gantt": patch
+"@object-ui/plugin-view": patch
+"@object-ui/components": patch
+---
+
+fix(hooks): stop calling translation hooks inside try/catch (objectui#2879)
+
+Eleven call sites wrapped a React hook in `try`/`catch` to make it
+"provider-safe". `useObjectTranslation` and `useObjectLabel` already are — they
+read context optionally and fall back to react-i18next's global instance, and
+never throw. The `catch` bought nothing and cost correctness: a throw *after*
+the hook ran desyncs hook order on the next render, because React matches hooks
+positionally. objectui#2595/#2596 fixed exactly this in `@object-ui/i18n`'s
+`createSafeTranslation`; nine plugin-local re-implementations kept their own
+copy of the bug, and two more (`ObjectTimeline`, `ObjectView`) were found by the
+new lint rule below — `ObjectView` had even suppressed
+`react-hooks/rules-of-hooks` inline to keep it.
+
+- Six exact re-implementations now delegate to `createSafeTranslation`:
+  `plugin-detail`, `plugin-timeline`, `plugin-list`, `plugin-calendar`,
+  `plugin-grid`'s `ObjectGrid`, `plugin-designer`.
+- `components`' `data-table` also delegates; `createSafeTranslation` now
+  returns `language` alongside `t` so consumers that localize dates don't need
+  a second hook call. Purely additive.
+- `plugin-gantt` and `plugin-grid`'s `ImportWizard` keep their local hooks —
+  they fall back *per key*, which a single-probe factory cannot express and
+  which their comments justify (a host dictionary that covers common keys but
+  lags on newer ones). Only the `try`/`catch` is removed.
+- `ObjectTimeline` and `ObjectView` call the hook directly and probe the
+  returned value, mirroring `useSafeFieldLabel`.
+
+Adds `object-ui/no-try-catch-around-hook` (error) so a twelfth copy fails CI.
+It only matches `use*` names, accepts member calls solely on `React` (so
+`vi.useRealTimers()` is not a hook), and resets its try-depth inside nested
+functions (so `renderHook(() => useThing())` inside a `try` is fine) — both
+false positives were real code in this repo and are pinned in the rule's tests.
+
+`eslint-rules/**/*.test.js` matched no vitest project glob, so the local
+plugin's specs had never run in CI. They are now included; all three pass.
+
+`ObjectTimeline`'s test mock of `@object-ui/react` omitted `useObjectLabel` —
+the removed `try`/`catch` had been silently absorbing that gap. The mock is now
+complete.

eslint-rules/index.js

@@ -3,10 +3,12 @@
  */
 import noSyntheticEventTrigger from './no-synthetic-event-trigger.js';
 import noInlineSpecConfig from './no-inline-spec-config.js';
+import noTryCatchAroundHook from './no-try-catch-around-hook.js';
 
 export default {
   rules: {
     'no-synthetic-event-trigger': noSyntheticEventTrigger,
     'no-inline-spec-config': noInlineSpecConfig,
+    'no-try-catch-around-hook': noTryCatchAroundHook,
   },
 };

eslint-rules/no-try-catch-around-hook.js

@@ -0,0 +1,99 @@
+/**
+ * ObjectUI ESLint rule: no-try-catch-around-hook
+ *
+ * Bans wrapping a React hook call in `try`/`catch`.
+ *
+ * A `catch` that swallows a throw from a hook desyncs hook order on the next
+ * render: the hooks that already ran before the throw were registered, the
+ * ones after it were not, and React matches hooks positionally. The result is
+ * the classic "Rendered fewer hooks than expected" crash, or silently reading
+ * another hook's state.
+ *
+ * This exists because the pattern kept getting copy-pasted. objectui#2595 /
+ * #2596 fixed it in `@object-ui/i18n`'s `createSafeTranslation`, but nine
+ * plugin-local re-implementations kept their own `try { useObjectTranslation() }
+ * catch {}` (objectui#2879) — every one of them written to make a hook
+ * "provider-safe", which the hook already is.
+ *
+ * The correct pattern for "no provider mounted" is a VALUE probe after the
+ * hook has run (e.g. `t(testKey) === testKey` → use English defaults), not a
+ * `catch`. See `packages/i18n/src/useSafeTranslation.ts`.
+ *
+ * Scope, deliberately narrow to stay false-positive free:
+ *  - Only `use*` names, per React's own convention.
+ *  - Bare identifiers and `React.useX` only. `vi.useRealTimers()` and
+ *    `jest.useFakeTimers()` are test utilities, not hooks.
+ *  - The try-depth RESETS inside a nested function. A hook called in a
+ *    callback that merely happens to be written inside a try —
+ *    `renderHook(() => useCondition(...))` — runs during React's render of
+ *    that component, not in the try's synchronous flow, and is fine.
+ *
+ * @type {import('eslint').Rule.RuleModule}
+ */
+
+/** React's own convention: a hook is `use` followed by a non-lowercase char. */
+function isHookName(name) {
+  return typeof name === 'string' && /^use[A-Z0-9]/.test(name);
+}
+
+/**
+ * Resolve the called name, but only for shapes that can be a React hook.
+ * `vi.useRealTimers()` / `jest.useFakeTimers()` are namespaced test helpers
+ * that match the `use*` spelling by coincidence, so member calls are accepted
+ * only on `React`.
+ */
+function hookCalleeName(callee) {
+  if (callee.type === 'Identifier') return callee.name;
+  if (
+    callee.type === 'MemberExpression' &&
+    callee.object.type === 'Identifier' &&
+    callee.object.name === 'React' &&
+    callee.property.type === 'Identifier'
+  ) {
+    return callee.property.name;
+  }
+  return undefined;
+}
+
+export default {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Disallow calling a React hook inside try/catch — a caught throw desyncs hook order on the next render (objectui#2595/#2596/#2879).',
+      recommended: true,
+    },
+    schema: [],
+    messages: {
+      banned:
+        "Do not call the hook '{{name}}' inside try/catch — a caught throw desyncs hook order on the next render (objectui#2879). Call the hook unconditionally and probe its RETURN VALUE for the degraded case; see createSafeTranslation in packages/i18n/src/useSafeTranslation.ts.",
+    },
+  },
+  create(context) {
+    // Stack of try-depths, one frame per enclosing function. Entering a
+    // function pushes a fresh 0 so a callback written inside a try starts
+    // clean; leaving pops back to the outer count.
+    const frames = [0];
+    const bump = (n) => { frames[frames.length - 1] += n; };
+
+    const enterFn = () => { frames.push(0); };
+    const exitFn = () => { frames.pop(); };
+
+    return {
+      ':function': enterFn,
+      ':function:exit': exitFn,
+
+      'TryStatement > BlockStatement': () => bump(1),
+      'TryStatement > BlockStatement:exit': () => bump(-1),
+      'CatchClause > BlockStatement': () => bump(1),
+      'CatchClause > BlockStatement:exit': () => bump(-1),
+
+      CallExpression(node) {
+        if (frames[frames.length - 1] === 0) return;
+        const name = hookCalleeName(node.callee);
+        if (!isHookName(name)) return;
+        context.report({ node, messageId: 'banned', data: { name } });
+      },
+    };
+  },
+};

eslint-rules/no-try-catch-around-hook.test.js

@@ -0,0 +1,67 @@
+/**
+ * Pins `no-try-catch-around-hook` against the two false positives that the
+ * first draft of the rule produced when it was run across the repo:
+ *
+ *  - `vi.useRealTimers()` / `jest.useFakeTimers()` — test helpers that match
+ *    the `use*` spelling by coincidence and are not React hooks.
+ *  - `renderHook(() => useThing())` inside a try — the hook runs during
+ *    React's render of the harness component, not in the try's synchronous
+ *    flow, so hook order is unaffected.
+ *
+ * Both were real code in this repo (packages/auth/.../createAuthClient.test.ts
+ * and packages/react/.../useExpression.test.ts), which is why they are pinned
+ * rather than left to judgement.
+ */
+import { describe, it, afterAll } from 'vitest';
+import { RuleTester } from 'eslint';
+import rule from './no-try-catch-around-hook.js';
+
+RuleTester.afterAll = afterAll;
+RuleTester.it = it;
+RuleTester.describe = describe;
+
+const ruleTester = new RuleTester();
+
+ruleTester.run('no-try-catch-around-hook', rule, {
+  valid: [
+    // The correct shape: hook first, probe its return value.
+    `function useThing() {
+       const r = useObjectTranslation();
+       return r.t('k') === 'k' ? FALLBACK : r.t;
+     }`,
+    // Namespaced test helpers are not hooks.
+    `function t() { try { vi.useRealTimers(); } finally { done(); } }`,
+    `function t() { try { jest.useFakeTimers(); } catch { /* noop */ } }`,
+    // Hook inside a nested callback: runs under React's render, not the try.
+    `function t() {
+       try { renderHook(() => useCondition(SRC)); } finally { cleanup(); }
+     }`,
+    // try/catch with no hook at all.
+    `function t() { try { JSON.parse(s); } catch { return null; } }`,
+    // Lowercase `user*` must not be mistaken for a hook.
+    `function t() { try { username(); } catch { return null; } }`,
+  ],
+  invalid: [
+    {
+      code: `function useBad() {
+        try { const r = useObjectTranslation(); return { t: r.t }; }
+        catch { return { t: (k) => k }; }
+      }`,
+      errors: [{ messageId: 'banned' }],
+    },
+    {
+      code: `function useBad() { try { return useObjectLabel(); } catch { return null; } }`,
+      errors: [{ messageId: 'banned' }],
+    },
+    // `React.useX` member form is still a hook.
+    {
+      code: `function useBad() { try { return React.useMemo(f, []); } catch { return null; } }`,
+      errors: [{ messageId: 'banned' }],
+    },
+    // A hook in the catch body is equally order-desyncing.
+    {
+      code: `function useBad() { try { f(); } catch { return useObjectTranslation(); } }`,
+      errors: [{ messageId: 'banned' }],
+    },
+  ],
+});

eslint.config.js

@@ -43,6 +43,12 @@ export default tseslint.config({ ignores: ['**/dist', '**/.next', '**/node_modul
     // new violation fails CI; the existing surfaces were converted to direct
     // idempotent commands first, so this lints clean today.
     'object-ui/no-synthetic-event-trigger': 'error',
+    // objectui#2879 ratchet — a hook called inside try/catch desyncs hook order
+    // when the catch swallows a throw. #2595/#2596 fixed this in the canonical
+    // createSafeTranslation; nine plugin-local copies kept it until #2879.
+    // Error so a tenth copy fails CI; all known sites were converted first, so
+    // this lints clean today.
+    'object-ui/no-try-catch-around-hook': 'error',
   },
 }, {
   // Type-discipline ratchet, scoped to the canonical view-schema file: a

packages/components/src/renderers/complex/data-table.tsx

@@ -13,7 +13,8 @@ import { resolveIcon } from '../action/resolve-icon';
 import { useGridFieldAuthoring } from '../../context/gridFieldAuthoring';
 import { ComponentRegistry } from '@object-ui/core';
 import type { DataTableSchema } from '@object-ui/types';
-import { useObjectTranslation, useRowPredicate } from '@object-ui/react';
+import { useRowPredicate } from '@object-ui/react';
+import { createSafeTranslation } from '@object-ui/i18n';
 import { 
   Table, 
   TableHeader, 
@@ -155,41 +156,12 @@ const TABLE_DEFAULT_TRANSLATIONS: Record<string, string> = {
 /**
  * Safe wrapper for useObjectTranslation that falls back to English defaults
  * when I18nProvider is not available (e.g., standalone usage).
+ *
+ * Delegates to `@object-ui/i18n`'s `createSafeTranslation` (which also
+ * surfaces `language` for the date/number formatting below); the local copy
+ * this replaced wrapped the hook in try/catch (rules-of-hooks, objectui#2879).
  */
-function useTableTranslation() {
-  try {
-    const result = useObjectTranslation();
-    const testValue = result.t('table.rowsPerPage');
-    if (testValue === 'table.rowsPerPage') {
-      return {
-        t: (key: string, options?: Record<string, unknown>) => {
-          let value = TABLE_DEFAULT_TRANSLATIONS[key] || key;
-          if (options) {
-            for (const [k, v] of Object.entries(options)) {
-              value = value.replace(`{{${k}}}`, String(v));
-            }
-          }
-          return value;
-        },
-        language: result.language || 'en',
-      };
-    }
-    return { t: result.t, language: result.language || 'en' };
-  } catch {
-    return {
-      t: (key: string, options?: Record<string, unknown>) => {
-        let value = TABLE_DEFAULT_TRANSLATIONS[key] || key;
-        if (options) {
-          for (const [k, v] of Object.entries(options)) {
-            value = value.replace(`{{${k}}}`, String(v));
-          }
-        }
-        return value;
-      },
-      language: 'en',
-    };
-  }
-}
+const useTableTranslation = createSafeTranslation(TABLE_DEFAULT_TRANSLATIONS, 'table.rowsPerPage');
 
 /**
  * Pull the most useful human-readable message out of whatever the save path

packages/i18n/src/useSafeTranslation.ts

@@ -37,10 +37,15 @@ export function createSafeTranslation(
     // "translations not configured" fallback.
     const result = useObjectTranslation();
     const testValue = result.t(testKey);
+    // `language` is surfaced so consumers that localize dates/numbers
+    // alongside their copy (data-table, gantt) don't have to call
+    // `useObjectTranslation` a second time just to read it. With no provider
+    // it is whatever react-i18next reports, which callers may treat as
+    // "follow the runtime default".
     if (testValue === testKey) {
-      return { t: fallbackT };
+      return { t: fallbackT, language: result.language };
     }
-    return { t: result.t };
+    return { t: result.t, language: result.language };
   };
 }
 

packages/plugin-calendar/src/CalendarView.tsx

@@ -23,7 +23,7 @@ import {
   PopoverContent,
   PopoverTrigger
 } from "@object-ui/components"
-import { useObjectTranslation } from "@object-ui/i18n"
+import { createSafeTranslation } from "@object-ui/i18n"
 
 const DEFAULT_EVENT_COLOR = "bg-blue-100 text-blue-900 border border-blue-200"
 const STABLE_DEFAULT_DATE = new Date()
@@ -87,43 +87,11 @@ const DEFAULT_TRANSLATIONS: Record<string, string> = {
 /**
  * Safe wrapper for useObjectTranslation that falls back to English defaults
  * when I18nProvider is not available (e.g., standalone usage outside console).
+ *
+ * Delegates to `@object-ui/i18n`'s `createSafeTranslation`; the local copy this
+ * replaced wrapped the hook in try/catch (rules-of-hooks, objectui#2879).
  */
-function useCalendarTranslation() {
-  try {
-    const result = useObjectTranslation()
-    // Check if i18n is properly initialized by testing a known key
-    const testValue = result.t('calendar.today')
-    if (testValue === 'calendar.today') {
-      // i18n returned the key itself — not initialized
-      return {
-        t: (key: string, options?: Record<string, unknown>) => {
-          let value = DEFAULT_TRANSLATIONS[key] || key
-          if (options) {
-            for (const [k, v] of Object.entries(options)) {
-              value = value.replace(`{{${k}}}`, String(v))
-            }
-          }
-          return value
-        },
-        language: 'en',
-      }
-    }
-    return { t: result.t, language: result.language }
-  } catch {
-    return {
-      t: (key: string, options?: Record<string, unknown>) => {
-        let value = DEFAULT_TRANSLATIONS[key] || key
-        if (options) {
-          for (const [k, v] of Object.entries(options)) {
-            value = value.replace(`{{${k}}}`, String(v))
-          }
-        }
-        return value
-      },
-      language: 'en',
-    }
-  }
-}
+const useCalendarTranslation = createSafeTranslation(DEFAULT_TRANSLATIONS, 'calendar.today')
 
 export interface CalendarEvent {
   id: string | number