From 02b60d8ec1b0a00b778c4563f58513ba2ba701f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 06:12:48 +0000 Subject: [PATCH 1/3] fix(react): UseNavigationOverlayOptions.onRowClick declares the modifier payload it is called with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `useNavigationOverlay` declared the option with one parameter: onRowClick?: (record: Record) => void; and `handleClick`, 126 lines below in the same file, asserted that declaration away in order to call it with two: (onRowClick as (r: Record, e?: HandleClickModifiers) => void)(record, event); The assertion was the only thing holding the two apart — a declaration that had lost an argument, not a hook that needed one. The consequence is not a crash: it is that the modifier payload is invisible on the one line a host reads, so a host implementing Cmd/Ctrl/middle-click has to discover the second argument from the implementation and then spell its own parameter optional to stay assignable. `app-shell`'s `ObjectView` does exactly that at three call sites. The declaration now names both parameters and the assertion is deleted. `HandleClickModifiers` is declared and exported in this very file, so naming it here costs no import and no dependency. Source-compatible in BOTH directions, measured rather than assumed: a one-parameter handler is assignable to the widened signature, and a handler written against the widened signature was already assignable to the narrow one (its minimum argument count is still one). No consumer changes. ⭐ Which is exactly why the pin is not an assignability assertion: both spellings satisfy each other, so an `extends` pin is green on the broken tree and on the repaired one alike. `useNavigationOverlay.onRowClickArity-9357.test.tsx` uses the two instruments that can separate them — an exact-identity read of `Parameters<...>` under `tsconfig.test.json`, and a bytes read of the declaration and the call site off disk — each with controls proving it can fire. Scope: the hook's own option only. The pass-through props on the view components that feed it still declare one parameter on their own published faces; which spelling that family converges on is the open question on objectui#9357 and is not decided here. Part of objectui#9357 Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ Co-authored-by: Claude --- ...357-navigation-overlay-onrowclick-arity.md | 32 ++ ...ationOverlay.onRowClickArity-9357.test.tsx | 342 ++++++++++++++++++ .../react/src/hooks/useNavigationOverlay.ts | 27 +- 3 files changed, 398 insertions(+), 3 deletions(-) create mode 100644 .changeset/9357-navigation-overlay-onrowclick-arity.md create mode 100644 packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx diff --git a/.changeset/9357-navigation-overlay-onrowclick-arity.md b/.changeset/9357-navigation-overlay-onrowclick-arity.md new file mode 100644 index 0000000000..47f5321086 --- /dev/null +++ b/.changeset/9357-navigation-overlay-onrowclick-arity.md @@ -0,0 +1,32 @@ +--- +'@object-ui/react': minor +--- + +`UseNavigationOverlayOptions.onRowClick` now declares the modifier payload it +has always been called with (objectui#9357). + +`useNavigationOverlay`'s `handleClick` invokes the caller-supplied `onRowClick` +with two arguments — the record, and the optional `HandleClickModifiers` +payload (`metaKey` / `ctrlKey` / `button`) a host needs to implement +Cmd/Ctrl/middle-click. The option declared only the record, and `handleClick` +carried a type assertion that widened the value at the call site so the code +would compile. The second argument was therefore invisible on the one line a +host reads, and a host that wanted it had to discover it from the +implementation and then spell its own second parameter optional to stay +assignable. + +The declaration now names both parameters and the assertion is gone. + +**Not breaking, in either direction.** A one-parameter handler stays assignable +to the widened signature (its extra parameter is optional), and a handler +written against the widened signature was already assignable to the old one — +measured on this change, both directions. No caller has to change; what changes +is that a caller who wants the modifier payload can now see, from the published +type, that it is there. + +Scope note: this repairs the hook's own option. The pass-through props on the +view components that feed it — `ObjectKanban`, `ObjectGallery`, the +`plugin-kanban` renderer's `onCardClick`, and the `onRowClick` prop on the other +view plugins — each still declare one parameter on their own published face; +which spelling that family converges on is objectui#9357's open question and is +not decided here. diff --git a/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx new file mode 100644 index 0000000000..f13310d506 --- /dev/null +++ b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx @@ -0,0 +1,342 @@ +/** + * 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#9357 — `UseNavigationOverlayOptions.onRowClick` declares the + * modifier payload it is actually invoked with, and `handleClick` calls it + * without a type assertion. + * + * ## The defect + * + * The option declared ONE parameter: + * + * onRowClick?: (record: Record) => void; + * + * and `handleClick`, 126 lines below in the same file, asserted that + * declaration away in order to call it with TWO — the record and the modifier + * payload a host needs for Cmd/Ctrl/middle-click. The assertion was the only + * thing holding the two apart: a declaration that had lost an argument, not a + * hook that needed one. + * + * ## Why the instrument here is not an assignability assertion + * + * ⭐ Widening this option is source-compatible in BOTH directions — a + * one-parameter handler stays assignable to a two-parameter optional + * signature, and a two-parameter optional handler stays assignable to the + * one-parameter spelling (its minimum argument count is still 1). That is the + * measurement `SOURCE COMPATIBILITY` below makes, and it is good news for + * consumers: nothing breaks either way. + * + * It is also exactly why an `extends` / assignability pin would assert + * NOTHING here. Both spellings satisfy each other, so such a pin is green on + * the broken tree and green on the repaired one — a dead instrument on a + * type-only card. The two instruments that CAN separate them are used + * instead, and both are proven able to fire: + * + * - the COMPILE-TIME half reads the parameter LIST (`Parameters<…>` and its + * `length`) through an exact-identity `Equal`, which distinguishes the two + * spellings where `extends` cannot. It runs only under + * `packages/react`'s `tsconfig.test.json`; vitest erases every line of it. + * - the BYTES half reads the declaration and the call site off disk, so the + * assertion cannot creep back in under a green type-check. + * + * The third block is a RUNTIME control: the implementation really does hand + * the second argument through. It was true before this card too — it is what + * made the assertion look harmless — so it is labelled a control rather than + * a pin, and it reds if a later change makes the widened declaration a + * promise the implementation stops keeping. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { existsSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { useNavigationOverlay } from '../useNavigationOverlay'; +import type { + HandleClickModifiers, + NavigationOverlayState, + UseNavigationOverlayOptions, +} from '../useNavigationOverlay'; + +/* ------------------------------------------------------------------ * + * COMPILE-TIME half — erased at runtime. `tsc -p tsconfig.test.json` + * (chained from this package's `type-check` script) is the only thing + * that executes it. + * ------------------------------------------------------------------ */ + +type Equal = + (() => T extends X ? 1 : 2) extends (() => T extends Y ? 1 : 2) ? true : false; +type Expect = T; + +type OnRowClick = NonNullable; +type OnRowClickParams = Parameters; + +/** The declaration as it stood before this card — the measurement subject. */ +type NarrowOnRowClick = (record: Record) => void; + +/** + * THE PIN. The option takes the record AND the optional modifier payload, so + * its parameter list spans one-or-two rather than exactly one. + */ +type _AritySpansBoth = Expect>; +type _FirstParamIsTheRecord = Expect>>; +type _SecondParamIsTheModifiers = Expect< + Equal +>; + +/** + * The option and the handler that invokes it are now the SAME function type. + * `handleClick` always declared both parameters; this is the half that had + * drifted. + */ +type _OptionAgreesWithHandleClick = Expect< + Equal +>; + +/** + * SOURCE COMPATIBILITY — the measurement, reported as a result rather than + * assumed. Both directions hold, so no consumer is broken by the widening in + * either direction of assignment... + */ +type _NarrowIsAssignableToWide = Expect; +type _WideIsAssignableToNarrow = Expect; + +/** + * ...and THIS is the consequence that decides the instrument: because both + * directions hold, assignability cannot tell the repaired declaration from + * the broken one. Exact identity can, and does. + */ +type _IdentityStillSeparatesThem = Expect, false>>; + +/** + * Control: the checker is live in this file and the parameter list really + * stops at two. If the declaration ever grew a third parameter this directive + * would become UNUSED and `tsc` would fail with TS2578 — which is why the + * control is written as an expected error rather than as another assertion. + */ +// @ts-expect-error the option declares no third parameter +type _NoThirdParameter = OnRowClickParams[2]; + +/* ------------------------------------------------------------------ * + * BYTES half — the declaration and the call site, read off disk. + * ------------------------------------------------------------------ */ + +const here = path.dirname(fileURLToPath(import.meta.url)); +// packages/react/src/hooks/__tests__ -> repo root +const repoRoot = path.resolve(here, '../../../../..'); +const HOOK_REL = 'packages/react/src/hooks/useNavigationOverlay.ts'; +const HOOK_ABS = path.join(repoRoot, HOOK_REL); + +/** + * Mask `//` and block comments with spaces, leaving every other byte — string + * contents included — in place. Quoted spans are walked rather than blanked, + * purely so a `//` inside a URL literal is not read as a comment start. + */ +function maskComments(src: string): string { + const out = src.split(''); + let i = 0; + const n = src.length; + while (i < n) { + const c = src[i]; + const d = src[i + 1]; + if (c === '/' && d === '/') { + while (i < n && src[i] !== '\n') out[i++] = ' '; + continue; + } + if (c === '/' && d === '*') { + out[i++] = ' '; + out[i++] = ' '; + while (i < n && !(src[i] === '*' && src[i + 1] === '/')) { + if (src[i] !== '\n') out[i] = ' '; + i++; + } + if (i < n) { + out[i++] = ' '; + out[i++] = ' '; + } + continue; + } + if (c === '"' || c === "'" || c === '`') { + const quote = c; + i++; + while (i < n) { + if (src[i] === '\\') { + i += 2; + continue; + } + if (src[i] === quote) { + i++; + break; + } + i++; + } + continue; + } + i++; + } + return out.join(''); +} + +/** The body of a `{ … }` block opened at `openIndex`, brace-balanced. */ +function blockAt(masked: string, openIndex: number): string { + let depth = 0; + for (let i = openIndex; i < masked.length; i++) { + if (masked[i] === '{') depth++; + else if (masked[i] === '}') { + depth--; + if (depth === 0) return masked.slice(openIndex + 1, i); + } + } + throw new Error('unbalanced block'); +} + +const HOOK_SOURCE = readFileSync(HOOK_ABS, 'utf8'); +const HOOK_MASKED = maskComments(HOOK_SOURCE); + +const OPTIONS_DECL = 'export interface UseNavigationOverlayOptions'; +const optionsOpen = HOOK_MASKED.indexOf('{', HOOK_MASKED.indexOf(OPTIONS_DECL)); +const OPTIONS_BODY = blockAt(HOOK_MASKED, optionsOpen); + +const HANDLE_CLICK_DECL = 'const handleClick = useCallback('; +const handleClickOpen = HOOK_MASKED.indexOf('{', HOOK_MASKED.indexOf(HANDLE_CLICK_DECL)); +const HANDLE_CLICK_BODY = blockAt(HOOK_MASKED, handleClickOpen); + +/** `onRowClick` declared with a second, optional, named parameter. */ +const TWO_PARAMETER_MEMBER = + /onRowClick\?:\s*\(\s*record:\s*Record\s*,\s*event\?:\s*HandleClickModifiers\s*\)\s*=>\s*void\s*;/; +/** `onRowClick` declared with exactly one parameter — the defect's spelling. */ +const ONE_PARAMETER_MEMBER = + /onRowClick\?:\s*\(\s*record:\s*Record\s*\)\s*=>\s*void\s*;/; +/** Any type assertion applied to the `onRowClick` value. */ +const ONROWCLICK_ASSERTION = /\bonRowClick\s+as\b/; +/** The call, made on the declared value with both arguments. */ +const DIRECT_TWO_ARGUMENT_CALL = /\bonRowClick\(\s*record\s*,\s*event\s*\)/; + +describe('the instrument can fire (controls)', () => { + it('anchors on this file, not on the cwd', () => { + expect(existsSync(path.join(repoRoot, 'pnpm-workspace.yaml'))).toBe(true); + expect(existsSync(HOOK_ABS)).toBe(true); + }); + + it('found both spans it reads, and they are not empty', () => { + expect(HOOK_MASKED).toContain(OPTIONS_DECL); + expect(HOOK_MASKED).toContain(HANDLE_CLICK_DECL); + expect(OPTIONS_BODY).toContain('onRowClick'); + expect(HANDLE_CLICK_BODY).toContain('onRowClick'); + }); + + it('flags an assertion applied to the value', () => { + // Assembled from fragments so this control is not itself a hit if a + // source-scanning gate ever walks this file. + const asserted = '(onRowClick' + ' as ' + '(r: R, e?: E) => void)(record, event);'; + expect(ONROWCLICK_ASSERTION.test(maskComments(asserted))).toBe(true); + }); + + it('masks comments, so prose about the defect is not the defect', () => { + const commented = '// (onRowClick' + ' as ' + '(r: R) => void)(record, event);\nconst x = 1;'; + expect(ONROWCLICK_ASSERTION.test(maskComments(commented))).toBe(false); + const block = '/* onRowClick' + ' as ' + 'X */\nconst x = 1;'; + expect(ONROWCLICK_ASSERTION.test(maskComments(block))).toBe(false); + }); + + it('separates the one-parameter spelling from the two-parameter one', () => { + const narrow = 'onRowClick?: (record: Record) => void;'; + const wide = + 'onRowClick?: (record: Record, event?: HandleClickModifiers) => void;'; + expect(TWO_PARAMETER_MEMBER.test(narrow)).toBe(false); + expect(TWO_PARAMETER_MEMBER.test(wide)).toBe(true); + // Both matchers are needed and neither subsumes the other: one asserts the + // repaired spelling is PRESENT, the other that the defect's spelling is + // ABSENT, and a third spelling would have to satisfy both. + expect(ONE_PARAMETER_MEMBER.test(narrow)).toBe(true); + expect(ONE_PARAMETER_MEMBER.test(wide)).toBe(false); + }); +}); + +describe('UseNavigationOverlayOptions.onRowClick declares its modifier payload (objectui#9357)', () => { + it('declares the record and the optional modifier payload', () => { + expect( + TWO_PARAMETER_MEMBER.test(OPTIONS_BODY), + [ + `${HOOK_REL}: \`UseNavigationOverlayOptions.onRowClick\` does not declare the`, + 'modifier payload `handleClick` hands it. Every consumer reads this', + 'declaration to learn what its own pass-through receives, so an', + 'understated arity here is copied outward (objectui#9357).', + ].join('\n'), + ).toBe(true); + }); + + it('no longer carries the one-parameter spelling', () => { + const offender = OPTIONS_BODY.match(ONE_PARAMETER_MEMBER)?.[0]?.replace(/\s+/g, ' '); + expect( + offender ?? null, + 'the option is declared with one parameter again — the objectui#9357 defect', + ).toBeNull(); + }); +}); + +describe('handleClick calls the option through its declaration (objectui#9357)', () => { + it('applies no type assertion to onRowClick', () => { + expect( + ONROWCLICK_ASSERTION.test(HOOK_MASKED), + [ + `${HOOK_REL}: \`onRowClick\` is invoked through a type assertion.`, + 'An assertion here is the producer paying for its own understated', + 'declaration — AGENTS.md #0.1 sends that back to the producer, and the', + 'producer is this file. Declare the parameter instead (objectui#9357).', + ].join('\n'), + ).toBe(false); + }); + + it('calls it with both arguments, directly', () => { + expect(DIRECT_TWO_ARGUMENT_CALL.test(HANDLE_CLICK_BODY)).toBe(true); + }); +}); + +/* ------------------------------------------------------------------ * + * RUNTIME control — the implementation keeps the widened promise. + * ------------------------------------------------------------------ */ + +describe('the implementation delivers what the declaration now promises', () => { + it('forwards the record and the modifier payload to onRowClick', () => { + const onRowClick = vi.fn<(record: Record, event?: HandleClickModifiers) => void>(); + const { result } = renderHook(() => + useNavigationOverlay({ navigation: { mode: 'drawer' }, objectName: 'account', onRowClick }), + ); + + const record = { id: 'a1', name: 'Acme' }; + const event: HandleClickModifiers = { metaKey: true, ctrlKey: false, button: 0 }; + act(() => { + result.current.handleClick(record, event); + }); + + expect(onRowClick).toHaveBeenCalledTimes(1); + expect(onRowClick).toHaveBeenCalledWith(record, event); + // The external handler takes full priority: no overlay is opened behind it. + expect(result.current.isOpen).toBe(false); + }); + + it('still calls a one-parameter handler, which the widening keeps legal', () => { + const calls: Array> = []; + const oneArg: (record: Record) => void = (record) => { + calls.push(record); + }; + const { result } = renderHook(() => + useNavigationOverlay({ navigation: { mode: 'page' }, objectName: 'account', onRowClick: oneArg }), + ); + + const record = { id: 'b2' }; + act(() => { + result.current.handleClick(record); + }); + + expect(calls).toEqual([record]); + }); +}); diff --git a/packages/react/src/hooks/useNavigationOverlay.ts b/packages/react/src/hooks/useNavigationOverlay.ts index 00bb1307c9..18ddadad7f 100644 --- a/packages/react/src/hooks/useNavigationOverlay.ts +++ b/packages/react/src/hooks/useNavigationOverlay.ts @@ -139,8 +139,23 @@ export interface UseNavigationOverlayOptions { objectName?: string; /** External onNavigate callback (e.g., from ActionProvider or parent) */ onNavigate?: (recordId: string | number, action?: string) => void; - /** External onRowClick callback — if set, takes full priority */ - onRowClick?: (record: Record) => void; + /** + * External onRowClick callback — if set, takes full priority. + * + * Declares BOTH arguments `handleClick` hands it: the record, and the + * optional modifier payload (`HandleClickModifiers`, declared just below) + * that lets a host implement Cmd/Ctrl/middle-click for itself. The second + * argument was always delivered; until objectui#9357 this line declared only + * the first and `handleClick` asserted the declaration away in order to make + * the call — so the payload was invisible on the one line a host reads, and + * every consumer that passes its own handler through copied the understated + * spelling outward. + * + * A one-parameter handler stays assignable here, so nothing a caller already + * wrote has to change; what changes is that a caller who WANTS the payload + * can now see that it exists. + */ + onRowClick?: (record: Record, event?: HandleClickModifiers) => void; } /** @@ -265,8 +280,14 @@ export function useNavigationOverlay( // External onRowClick takes full priority. Forward the modifier event // so parent handlers (e.g. ObjectView) can still implement Cmd/Ctrl/ // middle-click → open in new tab. + // + // Called straight through the declaration (objectui#9357). This used to + // carry a type assertion widening the option to two parameters at the + // call — the producer paying for its own understated declaration, which + // AGENTS.md #0.1 sends back to the producer. The producer is this file, + // and the option above now declares what this line passes. if (onRowClick) { - (onRowClick as (r: Record, e?: HandleClickModifiers) => void)(record, event); + onRowClick(record, event); return; } From d8325fa79dfd3f20e5cb0b04d596adb8bb4e422e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 12:05:15 +0000 Subject: [PATCH 2/3] docs(changeset): name the one class the onRowClick widening refuses, and pin it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract review of this PR verified the repair on every axis and failed it on one published sentence: the changeset asserted "No caller has to change", and changeset text ships verbatim into `packages/react/CHANGELOG.md`, where it cannot be corrected after release. Measured, that generalisation is false for exactly one class. A handler passed DIRECTLY to `useNavigationOverlay` whose second parameter is annotated narrower than `HandleClickModifiers` — React's `MouseEvent` being the shape a host that discovered the payload from the implementation would write — compiled against `origin/main` and is refused at this head: error TS2322: Type '(_record: Record, _ev?: ReactMouseEvent) => void' is not assignable to type '(record: Record, event?: HandleClickModifiers | undefined) => void'. … Type 'HandleClickModifiers' is missing the following properties from type 'MouseEvent': altKey, buttons, clientX, clientY, and 26 more. Reproduced in both directions before this edit, same tsconfig, same probe, the hook file the only variable: refused against this head's declaration (blob 18ddadad), accepted against `origin/main`'s (blob 00bb1307), with a one-parameter handler and an exactly-typed handler as controls accepted on both trees. Two changes, and nothing else. The changeset's compatibility passage now names that class, gives the one-line remedy (annotate the parameter `HandleClickModifiers`, or drop the annotation) and states — without asserting a count this text cannot re-derive — that no caller inside this repository is in it. And the pin file's compile-time half gains one `@ts-expect-error` row holding the boundary, so it is measured rather than described: a directive whose error stops occurring is itself TS2578, which reds if the option is ever widened back or the payload respelled `any`. The code is untouched — the hook, the deleted assertion and every existing pin assertion stand exactly as reviewed. Re card objectui#9357. Claude-Session: https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt Co-authored-by: Claude --- ...357-navigation-overlay-onrowclick-arity.md | 45 ++++++++++++++++--- ...ationOverlay.onRowClickArity-9357.test.tsx | 36 +++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/.changeset/9357-navigation-overlay-onrowclick-arity.md b/.changeset/9357-navigation-overlay-onrowclick-arity.md index 47f5321086..f036d71a3e 100644 --- a/.changeset/9357-navigation-overlay-onrowclick-arity.md +++ b/.changeset/9357-navigation-overlay-onrowclick-arity.md @@ -17,12 +17,45 @@ assignable. The declaration now names both parameters and the assertion is gone. -**Not breaking, in either direction.** A one-parameter handler stays assignable -to the widened signature (its extra parameter is optional), and a handler -written against the widened signature was already assignable to the old one — -measured on this change, both directions. No caller has to change; what changes -is that a caller who wants the modifier payload can now see, from the published -type, that it is there. +**Source-compatible in both directions, with one measured exception.** A +one-parameter handler stays assignable to the widened signature (its extra +parameter is optional), and a handler written against the widened signature was +already assignable to the old one — measured on this change, both directions. + +**The exception, and the one class that has to change.** A handler passed +*directly* to `useNavigationOverlay` whose second parameter is annotated +*narrower* than `HandleClickModifiers` no longer type-checks. React's +`MouseEvent` is the shape this hits in practice, because until now the payload +was only discoverable from the implementation, so a host that wanted it wrote +the annotation it saw arrive: + +```ts +useNavigationOverlay({ + objectName: 'account', + // was accepted; now TS2322 — `HandleClickModifiers` is not assignable to + // `React.MouseEvent` + onRowClick: (record, ev?: React.MouseEvent) => { /* ... */ }, +}); +``` + +It compiled before only because the old declaration had no second parameter to +check the annotation against. The parameter is checked contravariantly, so the +annotation now has to *admit* `HandleClickModifiers`. **The fix is one line at +the call site:** annotate the parameter `HandleClickModifiers` (exported from +`@object-ui/react`), or drop the annotation and let it be inferred. Either way +the handler keeps receiving exactly what it received before — this is a +type-level change only, with no runtime behaviour attached. + +No caller *inside this repository* is in that class, and the repository's own +type-check re-derives that on every run rather than this sentence asserting it; +a host that reaches the hook through a view component's `onRowClick` prop is +unaffected either way, because the prop's own declared type is what gets +assigned to the option. The boundary is pinned as a `@ts-expect-error` row in +this package's `useNavigationOverlay.onRowClickArity-9357` test, so it cannot +move without a red check. + +What the widening buys everyone else: a caller who wants the modifier payload +can now see, from the published type, that it is there. Scope note: this repairs the hook's own option. The pass-through props on the view components that feed it — `ObjectKanban`, `ObjectGallery`, the diff --git a/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx index f13310d506..1dbf5a4e92 100644 --- a/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx +++ b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx @@ -54,6 +54,7 @@ import { describe, it, expect, vi } from 'vitest'; import { renderHook, act } from '@testing-library/react'; +import type { MouseEvent as ReactMouseEvent } from 'react'; import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -115,6 +116,41 @@ type _WideIsAssignableToNarrow = Expect, false>>; +/** + * THE ACCEPT-SET BOUNDARY — the one class the widening refuses, pinned rather + * than only described. A handler passed DIRECTLY to `useNavigationOverlay` + * whose second parameter is annotated NARROWER than `HandleClickModifiers` + * compiled before this card and is refused now. Measured on this change: + * + * error TS2322: Type '(_record: Record, _ev?: + * ReactMouseEvent) => void' is not assignable to type '(record: + * Record, event?: HandleClickModifiers | undefined) => + * void'. … Type 'HandleClickModifiers' is missing the following properties + * from type 'MouseEvent': altKey, buttons, clientX, + * clientY, and 26 more. + * + * It compiled before only because the old declaration had no second parameter + * to check the annotation against. Now the parameter is checked + * contravariantly, so the annotation has to ADMIT `HandleClickModifiers` — + * and React's mouse event, the shape a host writing against the + * implementation rather than the declaration reaches for, does not. The remedy + * at such a call site is one line: annotate the parameter + * `HandleClickModifiers`, or drop the annotation. + * + * ⭐ Why this is a `@ts-expect-error` and not another `Expect` row: the + * changeset publishes this class and that remedy, and changeset text ships + * verbatim into `packages/react/CHANGELOG.md`, where it cannot be corrected + * after release. A directive whose error stops occurring is itself an error + * (TS2578), so this line reds if the boundary ever moves — the option widened + * back to one parameter, or the payload respelled `any` — and the published + * sentence gets revisited instead of going quietly stale. Both readings were + * taken: refused against the repaired declaration, TS2578-unused against + * `origin/main`'s. + */ +type NarrowerSecondParam = (record: Record, event?: ReactMouseEvent) => void; +// @ts-expect-error a second parameter narrower than `HandleClickModifiers` is refused (TS2322) +type _NarrowerSecondParamIsRefused = Expect; + /** * Control: the checker is live in this file and the parameter list really * stops at two. If the declaration ever grew a third parameter this directive From 0d983a987acb2cbcf51b3e4561d7923eadaf2d61 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 23:17:32 +0000 Subject: [PATCH 3/3] docs(react): replace the refuted onRowClick generalisation in the shipped JSDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments are not stripped for this package, so the `onRowClick` docblock lands verbatim in `dist/hooks/useNavigationOverlay.d.ts` — the hover text every consumer of `@object-ui/react` reads. Its closing sentence still said a one-parameter handler stays assignable "so nothing a caller already wrote has to change", which the widening refutes for one class: a handler whose second parameter is annotated narrower than `HandleClickModifiers` is now refused with TS2322. The changeset two files away already names that class in bold, so the release would have published a CHANGELOG and a `.d.ts` that disagree about the same line. Replaced with the qualified form: what stays assignable, the one class that has to change, and the one-line remedy. Two residual restatements of the same generalisation in the pin test's own prose ("nothing breaks either way", "no consumer is broken by the widening") are corrected the same way; both sit in a file this PR already adds and both are contradicted 80 lines below by that file's own ACCEPT-SET BOUNDARY block. Comment text only. Re-measured at this head: the accept set is unchanged (a second parameter annotated React.MouseEvent is accepted at the merge-base b67b53bc0 and refused here with TS2322, while one-parameter and unannotated handlers are accepted on both trees), the emitted declaration corpus is still 65 files with 295 exported declaration names and an empty symmetric difference, and `dist/index.d.ts` is byte-identical. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L5xpA5q533BgTTNADibEFt --- ...vigationOverlay.onRowClickArity-9357.test.tsx | 11 +++++++---- packages/react/src/hooks/useNavigationOverlay.ts | 16 +++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx index 1dbf5a4e92..f7ceec87e8 100644 --- a/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx +++ b/packages/react/src/hooks/__tests__/useNavigationOverlay.onRowClickArity-9357.test.tsx @@ -29,8 +29,10 @@ * one-parameter handler stays assignable to a two-parameter optional * signature, and a two-parameter optional handler stays assignable to the * one-parameter spelling (its minimum argument count is still 1). That is the - * measurement `SOURCE COMPATIBILITY` below makes, and it is good news for - * consumers: nothing breaks either way. + * measurement `SOURCE COMPATIBILITY` below makes. It is a statement about those + * two spellings and nothing wider: one class of consumer IS refused by the + * widening — a handler whose second parameter is annotated narrower than + * `HandleClickModifiers` — and `THE ACCEPT-SET BOUNDARY` below pins it. * * It is also exactly why an `extends` / assignability pin would assert * NOTHING here. Both spellings satisfy each other, so such a pin is green on @@ -103,8 +105,9 @@ type _OptionAgreesWithHandleClick = Expect< /** * SOURCE COMPATIBILITY — the measurement, reported as a result rather than - * assumed. Both directions hold, so no consumer is broken by the widening in - * either direction of assignment... + * assumed. Both directions hold between these two spellings, so assigning + * either to the other breaks nobody (the class that IS broken is annotation- + * shaped, and is pinned under `THE ACCEPT-SET BOUNDARY` below)... */ type _NarrowIsAssignableToWide = Expect; type _WideIsAssignableToNarrow = Expect; diff --git a/packages/react/src/hooks/useNavigationOverlay.ts b/packages/react/src/hooks/useNavigationOverlay.ts index 18ddadad7f..4535696198 100644 --- a/packages/react/src/hooks/useNavigationOverlay.ts +++ b/packages/react/src/hooks/useNavigationOverlay.ts @@ -151,9 +151,19 @@ export interface UseNavigationOverlayOptions { * every consumer that passes its own handler through copied the understated * spelling outward. * - * A one-parameter handler stays assignable here, so nothing a caller already - * wrote has to change; what changes is that a caller who WANTS the payload - * can now see that it exists. + * A one-parameter handler stays assignable here, and so does one that leaves + * its second parameter unannotated. The exception, and the one class that has + * to change: a handler whose second parameter is annotated NARROWER than + * `HandleClickModifiers` — React's `MouseEvent` is the shape this hits in + * practice, because the payload used to be discoverable only from the + * implementation — is refused from this card on with TS2322. The parameter is + * checked contravariantly, so the annotation has to ADMIT + * `HandleClickModifiers`. The fix is one line at that call site: annotate the + * parameter `HandleClickModifiers` (exported from this module, and from + * `@object-ui/react`), or drop the annotation and let it be inferred; either + * way the handler keeps receiving exactly what it received before. What the + * widening buys everyone else is that a caller who WANTS the payload can now + * see, from the published type, that it exists. */ onRowClick?: (record: Record, event?: HandleClickModifiers) => void; }