fix(react): NavigationConfig.mode is optional — the type says what the hook does (#4550)

.changeset/navigation-config-mode-optional-4550.md

@@ -0,0 +1,22 @@
+---
+'@object-ui/react': minor
+'@object-ui/plugin-list': patch
+---
+
+`NavigationConfig.mode` is optional — the type now says what the hook does
+
+`@object-ui/react` published a `NavigationConfig` that required `mode`, in front of a `useNavigationOverlay` that has always defaulted it. The declaration took the spec's authored config, `Omit`ted `mode`, and re-added it as `NonNullable< … >`; 140 lines below, the hook read `navigation?.mode ?? 'page'`. The type was strictly stricter than the implementation it fronted, and `'page'` is meaningful behaviour rather than a placeholder.
+
+The spec never asked for that. `NavigationConfigSchema` declares `mode: NavigationModeSchema.default('page')`, and a `.default()` lands on the authoring side as `| undefined` — so `navigation: { view: 'summary_view' }` is legal authored metadata that lets the mode default. `@object-ui/types` already re-exported the spec's own `NavigationConfig` unchanged, which meant one monorepo shipped two published types of the same name that disagreed about whether `mode` could be omitted.
+
+The alias is now the spec's authored config verbatim, with no divergence of its own:
+
+```ts
+export type NavigationConfig = SpecAuthoredInput< typeof NavigationConfigSchema >;
+```
+
+The cost of the old spelling was paid by callers. `ListView` carried `schema.navigation as NavigationConfig | undefined` for no reason except to get a valid spec-shaped value past the declaration; that assertion is deleted here, not replaced. A type in front of an implementation must not be stricter than the implementation — when it is, every caller pays in casts, and a cast is exactly the renderer-side workaround that belongs back at the producer.
+
+**Nothing changes at runtime.** `navigation?.mode ?? 'page'` is untouched, and the default is now pinned as observable behaviour (`useNavigationOverlay.modeDefault.test.tsx`) rather than only as a comment — the explicit modes, the `preventNavigation` and `none` short-circuits, the `onRowClick` priority, and the Cmd/Ctrl/middle-click and `new_window` branches are all pinned alongside it.
+
+**Why minor rather than patch**, from the measured `.d.ts`. Optional-izing a property is looser for writers and narrower for readers, so the grade turns on which role the published surface actually plays. In this package `NavigationConfig` occurs only in input positions — `useNavigationOverlay`'s `navigation?:` option and `resolveOverlayWidth`'s parameter — and never in a return type; the package consumes these values and never hands one back. For consumers the change is therefore purely permissive: every call that compiled before still compiles, and spec-shaped configs that previously needed an assertion now compile without one. That gained input shape is a real capability rather than an internal repair, which is more than a patch describes. The reader-side narrowing is real but secondary: code that imports the bare type, annotates its own value with it and reads `.mode` now sees `NavigationMode | undefined`. The in-repo census found exactly one such importer — `ListView` — and it imported the type only to write the assertion this change removes.

packages/plugin-list/src/ListView.tsx

@@ -15,7 +15,6 @@ import { ViewSwitcherDropdown, ViewType } from './ViewSwitcher';
 import { ViewSettingsPopover } from './components/ViewSettingsPopover';
 import { UserFilters } from './UserFilters';
 import { SchemaRenderer, useNavigationOverlay } from '@object-ui/react';
-import type { NavigationConfig } from '@object-ui/react';
 import { useDensityMode } from '@object-ui/react';
 import type { ListViewSchema } from '@object-ui/types';
 import { detectStatusField } from '@object-ui/types';
@@ -1727,15 +1726,21 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
   }, [onSearchChange]);
 
   // --- NavigationConfig support ---
-  // The assertion bridges two spellings of ONE spec object and changes no
-  // value: `@object-ui/react`'s `NavigationConfig` alias re-declares `mode` as
-  // NON-optional, while the spec-derived `ListViewSchema['navigation']` leaves
-  // it optional. The hook's own body defaults it (`navigation?.mode ?? 'page'`),
-  // so a spec-shaped value is valid input and only the alias is tighter than
-  // its implementation. Surfaced by objectui#4528: this call used to type-check
-  // for the wrong reason, because the erased props type made `schema` `any`.
+  // No assertion, deliberately. `schema.navigation` is the spec-derived
+  // `ListViewSchema['navigation']` and the hook's `NavigationConfig` is now the
+  // spec's authored config verbatim — `mode` optional and all (objectui#4550).
+  // Two spellings of one spec object, so they simply agree.
+  //
+  // This call carried `as NavigationConfig | undefined` from objectui#4528
+  // until then. That cast bridged nothing real: the alias re-declared `mode` as
+  // required while the hook it fronts defaults it (`navigation?.mode ?? 'page'`),
+  // so the cast's only job was to get a valid value past an over-tight type.
+  // objectui#4550 fixed that at the producer, which deleted the reason for the
+  // cast — and a cast kept past its reason is how the next reader learns the
+  // wrong thing about the contract. Neither the cast nor its removal touches
+  // the runtime value.
   const navigation = useNavigationOverlay({
-    navigation: schema.navigation as NavigationConfig | undefined,
+    navigation: schema.navigation,
     objectName: schema.objectName,
     onNavigate: schema.onNavigate,
     onRowClick,

packages/react/src/hooks/__tests__/offline-nav-performance-spec-parity.test.ts

@@ -233,7 +233,7 @@ describe('OfflineCacheConfig keeps its defaulted keys authorable', () => {
   });
 });
 
-describe('NavigationConfig derives from the spec, requiring only `mode`', () => {
+describe('NavigationConfig IS the spec\'s authored navigation config', () => {
   it('is pinned at compile time', () => {
     type SpecNavigationInput = SpecAuthoredInput<typeof NavigationConfigSchema>;
     type _SpecNotAny = Assert<Equal<IsAny<SpecNavigationInput>, false>>;
@@ -247,14 +247,35 @@ describe('NavigationConfig derives from the spec, requiring only `mode`', () =>
     type _NoLocalOnlyKeys = Assert<Equal<Exclude<keyof NavigationConfig, keyof SpecNavigationInput>, never>>;
     type _NoMissingKeys = Assert<Equal<Exclude<keyof SpecNavigationInput, keyof NavigationConfig>, never>>;
 
-    // `mode` is the ONE narrowing, and it is required here. If the spec ever
-    // makes `mode` required itself, `_StillNarrowed` fails and this alias
-    // should collapse to `SpecAuthoredInput<typeof NavigationConfigSchema>`.
-    type _ModeIsRequired = Assert<Equal<undefined extends NavigationConfig['mode'] ? true : false, false>>;
-    type _StillNarrowed = Assert<Equal<Extends<SpecNavigationInput, NavigationConfig>, false>>;
-    type _EverythingElseMatches = Assert<
-      Extends<Omit<SpecNavigationInput, 'mode'>, Omit<NavigationConfig, 'mode'>>
-    >;
+    // objectui#4550 — the narrowing is GONE, and these three pins are the
+    // inverted descendants of the ones that described it.
+    //
+    // The alias used to `Omit` `mode` and re-add it as `NonNullable<…>`, which
+    // made it strictly tighter than BOTH the spec that produces the value and
+    // the hook that consumes it (`navigation?.mode ?? 'page'`, 140 lines below
+    // the declaration). A required key in front of an implementation that
+    // defaults it is not a contract, and the cost was paid at every spec-typed
+    // caller: `ListView` had to write `schema.navigation as NavigationConfig`
+    // to hand the hook a value the hook was always happy to take.
+    //
+    // The previous spelling of this block predicted its own end — "if the spec
+    // ever makes `mode` required itself, `_StillNarrowed` fails and this alias
+    // should collapse to `SpecAuthoredInput<typeof NavigationConfigSchema>`."
+    // The collapse arrived from the other direction (the spec never moved; the
+    // alias was wrong all along), but it is the same collapse.
+    type _IsExactlyTheSpecInput = Assert<Equal<NavigationConfig, SpecNavigationInput>>;
+
+    // `mode` is OPTIONAL to author, because the spec defaults it
+    // (`packages/spec/src/ui/view.zod.ts` → `mode: NavigationModeSchema.default('page')`)
+    // and a `.default()` lands on the AUTHORING side as `| undefined`. This is
+    // the assertion that was false before objectui#4550.
+    type _ModeIsOptional = Assert<Equal<undefined extends NavigationConfig['mode'] ? true : false, true>>;
+
+    // Assignability now runs BOTH ways. `_LocalIsASpecConfig` above is the one
+    // direction that always held; this is the one the narrowing blocked, and
+    // it is the direction every caller actually needs — spec produces, alias
+    // consumes.
+    type _SpecShapedValueFits = Assert<Extends<SpecNavigationInput, NavigationConfig>>;
 
     // The overlay buckets the hook maps to pixel widths are the spec's, and
     // `size` is the key #2578 added — the deprecated `width` is still here too.
@@ -265,7 +286,7 @@ describe('NavigationConfig derives from the spec, requiring only `mode`', () =>
   });
 
   it('still carries every overlay mode the hook switches on', () => {
-    const all: NavigationConfig['mode'][] = [
+    const all: NonNullable<NavigationConfig['mode']>[] = [
       'page',
       'drawer',
       'modal',
@@ -284,19 +305,25 @@ describe('NavigationConfig derives from the spec, requiring only `mode`', () =>
     // spec's `NavigationMode` directly, and this is the pin the declaration
     // promises: the two spellings must stay the SAME type.
     //
-    // They can come apart in a way nothing else would catch. The equality holds
-    // today only because `NavigationConfig` above strips the `undefined` that
-    // `NavigationConfigSchema`'s `.default('page')` puts on `mode`'s authoring
-    // side. If the spec ever stops defaulting `mode`, or defaults it on a
-    // narrower union, `NavigationConfig['mode']` moves and the exported alias
-    // does not — and every call site keeps compiling, because the hook's
+    // They can come apart in a way nothing else would catch. If the spec ever
+    // stops defaulting `mode`, or defaults it on a narrower union,
+    // `NavigationConfig['mode']` moves and the exported alias does not — and
+    // every call site keeps compiling, because the hook's
     // `navigation?.mode ?? 'page'` would still be assignable either way.
     //
     // Both directions, deliberately: a one-way `extends` is satisfied by a
     // narrowing as well as by equality, and a narrowing is exactly the drift
     // that would delete a mode the hook switches on.
+    //
+    // `NonNullable` is objectui#4550's mark on this pin, and it is load-bearing
+    // rather than cosmetic. The alias no longer strips the `undefined` that
+    // `.default('page')` puts on the authoring side, so `NavigationConfig['mode']`
+    // is now `NavigationMode | undefined` — a bare `Equal` here would fail for
+    // a reason that has nothing to do with the drift this test exists to catch.
+    // What must stay pinned is the MEMBERSHIP: the seven modes the hook
+    // switches on are exactly the seven the exported union publishes.
     type _ModeIsSpecMode = Assert<Equal<NavigationMode, SpecNavigationMode>>;
-    type _ModeIsConfigMode = Assert<Equal<NavigationMode, NavigationConfig['mode']>>;
+    type _ModeIsConfigMode = Assert<Equal<NavigationMode, NonNullable<NavigationConfig['mode']>>>;
 
     // Runtime half — the type assertions above are erased, so this is what
     // fails visibly if the union ever loses a member.