Skip to content

Commit 5d93390

Browse files
Claudeclaude
Claude
and
claude
committed
fix(spec): type-check defineApp's navigation again — #4171 fixed only half the annotation (#3786)
`NavigationItemSchema` is recursive, so it cannot infer its own type and carries a hand-written annotation. #4171 fixed that annotation's Output half (it was `z.ZodType<any>`, making the exported `NavigationItem` `any` for everyone). `z.ZodType` takes `<Output, Input>` and Input DEFAULTS TO `unknown`. Naming only the first parameter left the input half at that default, so `z.input<typeof AppSchema>` resolved `navigation` to `unknown` — and `unknown` accepts everything. `defineApp(config: z.input<typeof AppSchema>)`, the documented authoring entry point, compiled `navigation: [{ totally: 'made up' }, 42, 'nonsense']` clean. Parsing was never affected; only the compile-time contract lied, which is why no runtime test noticed. #4171 was verified through `z.infer` and nobody re-measured `z.input` — half a fix looks identical to a whole one. Adds `NavigationItemInput` and names both parameters. The two unions must stay distinct: `GroupNavItemSchema.expanded` and `UrlNavItemSchema.target` carry `.default()`, so those keys are required in the parsed output and omissible when authoring; reusing `NavigationItem` for input would force authors to write values the schema exists to supply. `app.nav-type-assertions.ts` is the mechanism, not a comment: a non-test src module (the package's `tsc --noEmit` gate excludes test files) pinning both unions AND the wiring between them and the schema. A correct `NavigationItemInput` that no schema references would leave every authoring path back at `unknown`, so the load-bearing probes go through `AppInput`. Mutation-tested — dropping the Input parameter, removing the `expanded` default, and leaking `children` onto a flat nav branch each turn the matching assertion red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb
1 parent 86a71d1 commit 5d93390

4 files changed

Lines changed: 310 additions & 4 deletions

File tree

‎.changeset/nav-item-input-type.md‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
fix(spec): `defineApp` type-checked its navigation again — the #4171 fix only covered half the annotation (#3786)
6+
7+
`NavigationItemSchema` is recursive, so it cannot infer its own type and carries a
8+
hand-written annotation instead. #4171 fixed that annotation's **Output** half: it
9+
had been `z.ZodType<any>`, which made the exported `NavigationItem` `any` for every
10+
consumer — a type that constrains nothing, which reads exactly like a type that
11+
works.
12+
13+
`z.ZodType` takes two parameters, `<Output, Input>`, and **`Input` defaults to
14+
`unknown`**. Naming only the first left the input half at that default, so
15+
`z.input<typeof AppSchema>` resolved `navigation` to `unknown` — and `unknown`
16+
accepts everything. `defineApp(config: z.input<typeof AppSchema>)` is the documented
17+
authoring entry point, and it took
18+
19+
```ts
20+
defineApp({
21+
name: 'my_app',
22+
label: 'My App',
23+
navigation: [{ totally: 'made up' }, 42, 'nonsense'], // compiled clean
24+
});
25+
```
26+
27+
with no complaint. Every authoring path through the app schema — `AppInput`,
28+
`NavigationAreaSchema`, `NavigationContributionSchema` — was unchecked the same way.
29+
Parsing was never affected: the schema rejected all of the above at runtime. It was
30+
only the compile-time contract that lied, which is why nothing in the test suite
31+
noticed. #4171's fix was verified through `z.infer`; nobody re-measured `z.input`,
32+
and half a fix looks identical to a whole one.
33+
34+
**The fix.** A new exported `NavigationItemInput` describes the authoring side, and
35+
the annotation now names both parameters. The two unions genuinely differ and one
36+
cannot serve both: `GroupNavItemSchema.expanded` and `UrlNavItemSchema.target` carry
37+
`.default()`, so those keys are **required** in the parsed output and **omissible**
38+
when authoring. Reusing `NavigationItem` as the input type would force authors to
39+
write values the schema exists to supply.
40+
41+
**What this changes for you.** Nothing at runtime, and nothing for code that reads a
42+
parsed app. Code that *authors* navigation through `defineApp`, `AppInput`, or
43+
`z.input` of any schema embedding `NavigationItemSchema` is now type-checked where it
44+
previously was not, so genuinely malformed navigation that used to compile will now
45+
surface as a compile error — the errors are pre-existing bugs becoming visible, not
46+
new restrictions. Two notes on what the checked type says:
47+
48+
- Authoring types (`AppInput`, `NavigationItemInput`) let you omit `expanded` and
49+
`target`; the parsed types (`App`, `NavigationItem`) still guarantee both are
50+
present.
51+
- If you annotate a hand-written literal with the parsed type (`const APP: App = {…}`)
52+
rather than the authoring type, you must spell out every defaulted key. That was
53+
already true and is unchanged — this release just makes the authoring alternative
54+
actually check its contents.
55+
56+
**The mechanism, not a comment.** `src/ui/app.nav-type-assertions.ts` is a non-test
57+
`src` module (the package's `tsc --noEmit` CI gate excludes test files) holding
58+
compile-level probes for both unions and, critically, for the **wiring** between them
59+
and the schema. A correct `NavigationItemInput` that no schema references would leave
60+
every authoring path back at `unknown` — precisely the state being fixed — so the
61+
load-bearing assertions go through `AppInput` and fail the moment the annotation loses
62+
its second parameter. Each probe was mutation-tested: dropping the `Input` parameter,
63+
removing the `expanded` default, and leaking `children` onto a flat nav branch each
64+
turn the corresponding assertion red.

‎packages/spec/api-surface.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3345,6 +3345,7 @@
33453345
"NavigationContribution (type)",
33463346
"NavigationContributionSchema (const)",
33473347
"NavigationItem (type)",
3348+
"NavigationItemInput (type)",
33483349
"NavigationItemSchema (const)",
33493350
"NavigationModeSchema (const)",
33503351
"Notification (type)",

‎packages/spec/src/ui/app.nav-type-assertions.ts‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Compile-level pins for the hand-tied navigation types (#3786).
5+
*
6+
* `NavigationItem` / `NavigationItemInput` are the one part of the navigation
7+
* contract that does NOT derive from a schema — they are the annotation
8+
* `NavigationItemSchema`'s recursion needs, so they cannot be inferred back out
9+
* of it. Anything hand-written next to a schema is a second description of the
10+
* same key set, and a second description drifts.
11+
*
12+
* It already has, twice, both silently:
13+
*
14+
* 1. #4171 — the annotation was `z.ZodType<any>`, so the exported
15+
* `NavigationItem` was `any`. A consumer importing it got a type that
16+
* constrains nothing, which reads exactly like a type that works.
17+
* 2. The `Input` half of that same annotation stayed at its `unknown` default
18+
* even after #4171 typed the `Output` half — so `z.input<typeof AppSchema>`
19+
* left `navigation` unchecked, and `defineApp` (the documented authoring
20+
* entry point, `config: z.input<typeof AppSchema>`) accepted
21+
* `navigation: [{ totally: 'made up' }, 42, 'nonsense']` with a clean
22+
* compile. The #4171 fix was verified through `z.infer`; nobody re-measured
23+
* `z.input`, and half a fix looks identical to a whole one.
24+
*
25+
* Both failures share the shape this file exists to break: a type that accepts
26+
* too much reports nothing. A runtime test cannot catch either — the schema
27+
* parses correctly in both cases; it is the compile-time contract that lies.
28+
*
29+
* Why a plain src module and not a test: `packages/spec/tsconfig.json` EXCLUDES
30+
* every test file, and the CI gate for this package is `tsc --noEmit` over the
31+
* `src` tree (lint.yml), so probes must live in a non-test src file to be
32+
* checked at all. The file is referenced by no tsup entry and re-exported by no
33+
* barrel, so it adds nothing to any build; it exists purely to make
34+
* `tsc --noEmit` fail when the types and the schema drift apart again.
35+
*
36+
* The `@ts-expect-error` probes assert the NEGATIVE direction — if the type
37+
* ever starts accepting those shapes, the now-unused suppression becomes the
38+
* compile error. They are the half that catches "accepts too much", which is
39+
* the half both real failures landed in.
40+
*/
41+
42+
import type { AppInput, NavigationItem, NavigationItemInput } from './app.zod';
43+
44+
/* ────────────────────────────────────────────────────────────────────────────
45+
* Output side (`z.infer`) — what a parse RETURNS.
46+
* Defaulted keys are present, so they are required here.
47+
* ──────────────────────────────────────────────────────────────────────────── */
48+
49+
const asItem = (x: NavigationItem): NavigationItem => x;
50+
51+
/** A group holding children, including a group nested under a group. */
52+
export const groupWithChildren: NavigationItem = {
53+
id: 'group_probe',
54+
type: 'group',
55+
label: 'Probe',
56+
expanded: false,
57+
children: [
58+
{ id: 'child_page', type: 'page', label: 'Child', pageName: 'probe_page' },
59+
{ id: 'nested_group', type: 'group', label: 'Nested', expanded: true, children: [] },
60+
],
61+
};
62+
63+
/** An object item nesting its views — `children` stays OPTIONAL on this branch. */
64+
export const objectWithChildren: NavigationItem = {
65+
id: 'obj_probe',
66+
type: 'object',
67+
label: 'Probe',
68+
objectName: 'probe_object',
69+
children: [{ id: 'obj_view', type: 'object', label: 'View', objectName: 'probe_object' }],
70+
};
71+
72+
/** The same branch without children — legal, unlike `group`. */
73+
export const objectWithoutChildren: NavigationItem = {
74+
id: 'obj_bare',
75+
type: 'object',
76+
label: 'Bare',
77+
objectName: 'probe_object',
78+
};
79+
80+
/**
81+
* `expanded` is REQUIRED on the output side: `GroupNavItemSchema` declares it
82+
* `.default(false)`, so a parsed group always carries it. If this suppression
83+
* goes unused the default was dropped from the schema — and every consumer
84+
* reading `item.expanded` off a parsed app silently started getting
85+
* `undefined`.
86+
*/
87+
// @ts-expect-error — parsed groups always carry `expanded`
88+
asItem({ id: 'group_bare', type: 'group', label: 'Bare', children: [] });
89+
90+
/** Only the `object` and `group` branches nest. */
91+
// @ts-expect-error — `children` exists on no other branch
92+
asItem({ id: 'page_probe', type: 'page', label: 'Probe', pageName: 'probe_page', children: [] });
93+
94+
/* ────────────────────────────────────────────────────────────────────────────
95+
* Input side (`z.input`) — what an AUTHOR writes.
96+
* Defaulted keys are omissible; everything else is still checked.
97+
* ──────────────────────────────────────────────────────────────────────────── */
98+
99+
const asInput = (x: NavigationItemInput): NavigationItemInput => x;
100+
101+
/**
102+
* The authoring shape: defaulted keys omitted. This is what cloud's admin app
103+
* and every downstream `defineApp` caller writes. Under the `unknown` input
104+
* this compiled for the uninteresting reason — everything did.
105+
*/
106+
export const inputOmitsDefaults: NavigationItemInput = {
107+
id: 'group_probe',
108+
type: 'group',
109+
label: 'Probe',
110+
children: [
111+
// `target` is `.default('_self')`, so it is omissible here and required above.
112+
{ id: 'link', type: 'url', label: 'Link', url: 'https://example.com' },
113+
],
114+
};
115+
116+
/** Supplying a defaulted key explicitly stays legal. */
117+
export const inputStatesDefaults: NavigationItemInput = {
118+
id: 'group_probe2',
119+
type: 'group',
120+
label: 'Probe',
121+
expanded: true,
122+
children: [],
123+
};
124+
125+
/**
126+
* The load-bearing negatives. Each of these compiled clean while `Input` sat at
127+
* `unknown` — that is the whole bug, so these are the assertions that would
128+
* have caught it.
129+
*/
130+
// @ts-expect-error — a nav item is not an arbitrary record
131+
asInput({ totally: 'made up', not: 'a nav item' });
132+
133+
// @ts-expect-error — a nav item is not a number
134+
asInput(42);
135+
136+
// @ts-expect-error — a nav item is not a string
137+
asInput('nonsense');
138+
139+
// @ts-expect-error — `type` must be one of the nine declared discriminants
140+
asInput({ id: 'bad_type', type: 'not_a_variant', label: 'X' });
141+
142+
// @ts-expect-error — `group` requires `children` on both sides
143+
asInput({ id: 'group_bare', type: 'group', label: 'Bare' });
144+
145+
// @ts-expect-error — the members are `.strict()`; an undeclared key is not authoring
146+
asInput({ id: 'grp', type: 'group', label: 'G', children: [], defaultOpen: true });
147+
148+
// @ts-expect-error — only `object` and `group` nest, on this side too
149+
asInput({ id: 'page_probe', type: 'page', label: 'P', pageName: 'probe_page', children: [] });
150+
151+
/* ────────────────────────────────────────────────────────────────────────────
152+
* The halves must stay DIFFERENT.
153+
*
154+
* If someone "simplifies" by pointing both parameters at one union, the
155+
* defaulted keys go wrong in one direction or the other: authors are forced to
156+
* write `expanded`, or consumers stop being able to read it. These two pin the
157+
* asymmetry itself.
158+
* ──────────────────────────────────────────────────────────────────────────── */
159+
160+
/** Output is assignable to input — a parsed tree can be re-authored. */
161+
export const outputFeedsInput: NavigationItemInput = groupWithChildren;
162+
163+
/** Input is NOT assignable to output: the defaults have not been applied yet. */
164+
// @ts-expect-error — `NavigationItemInput` is the looser half, by design
165+
asItem(inputOmitsDefaults);
166+
167+
/* ────────────────────────────────────────────────────────────────────────────
168+
* The WIRING — the assertions that actually catch the #4171 half-fix.
169+
*
170+
* Everything above pins the two unions. None of it pins the thing that broke:
171+
* whether `NavigationItemSchema` is annotated with `NavigationItemInput` at
172+
* all. A perfectly-written `NavigationItemInput` that no schema references
173+
* leaves `z.input<typeof AppSchema>` at `unknown` and every authoring path
174+
* unchecked — which is precisely the state this file was written to end.
175+
*
176+
* These go through `AppInput`, the type `defineApp` takes, so they fail the
177+
* moment the annotation loses its second parameter (verified by mutation:
178+
* dropping it back to `z.ZodType<NavigationItem>` turns all three into unused
179+
* suppressions).
180+
* ──────────────────────────────────────────────────────────────────────────── */
181+
182+
const asAppInput = (x: AppInput): AppInput => x;
183+
184+
/** The shape every downstream author writes — defaulted keys omitted. */
185+
export const appInputOmitsDefaults: AppInput = {
186+
name: 'probe_app',
187+
label: 'Probe',
188+
navigation: [{ id: 'grp', type: 'group', label: 'G', children: [] }],
189+
};
190+
191+
// @ts-expect-error — nav entries are checked; `unknown` would swallow this
192+
asAppInput({ name: 'probe_app', label: 'Probe', navigation: [42] });
193+
194+
// @ts-expect-error — …and this
195+
asAppInput({ name: 'probe_app', label: 'Probe', navigation: [{ totally: 'made up' }] });
196+
197+
// NOTE: kept on one line — `@ts-expect-error` suppresses the NEXT LINE only, and
198+
// the excess-property error lands on the nav entry, not on the opening `asAppInput(`.
199+
// @ts-expect-error — …and this, the strictness the nav members declare
200+
asAppInput({ name: 'p', label: 'P', navigation: [{ id: 'grp', type: 'group', label: 'G', children: [], defaultOpen: true }] });

‎packages/spec/src/ui/app.zod.ts‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -442,25 +442,66 @@ export type NavigationItem =
442442
| SeparatorNavItem
443443
| GroupNavItem;
444444

445+
/**
446+
* The INPUT half of {@link NavigationItemSchema} — what an author writes, as
447+
* opposed to what a parse returns.
448+
*
449+
* `z.ZodType` takes TWO type parameters, `<Output, Input>`, and **`Input`
450+
* defaults to `unknown`**. #4171 fixed the annotation's Output half (it used to
451+
* be `z.ZodType<any>`, which made the exported `NavigationItem` `any` for every
452+
* consumer) but left `Input` at that default — so `z.input<typeof AppSchema>`
453+
* resolved `navigation` to `unknown`, and `unknown` accepts everything. The
454+
* documented authoring entry point, `defineApp(config: z.input<typeof AppSchema>)`,
455+
* therefore took `navigation: [{ totally: 'made up' }, 42, 'nonsense']` with a
456+
* clean compile. That is the same "a type that constrains nothing" failure
457+
* #4171 describes, surviving on the half nobody re-measured: the fix was
458+
* verified through `z.infer` and `z.input` was never checked.
459+
*
460+
* The two halves genuinely differ, which is why one union cannot serve both:
461+
* `GroupNavItemSchema.expanded` and `UrlNavItemSchema.target` carry
462+
* `.default()`, so they are REQUIRED in the output and OPTIONAL here. Reusing
463+
* {@link NavigationItem} as the input type would demand authors write values
464+
* the schema exists to supply.
465+
*
466+
* Both recursive branches tie their `children` knot inline. The output union
467+
* ties `object` inline but inherits `group`'s knot from the {@link GroupNavItem}
468+
* alias — an asymmetry kept only because that alias is public API.
469+
* `app.nav-type-assertions.ts` pins both unions at compile level.
470+
*/
471+
export type NavigationItemInput =
472+
| (z.input<typeof ObjectNavItemSchema> & { children?: NavigationItemInput[] })
473+
| z.input<typeof DashboardNavItemSchema>
474+
| z.input<typeof PageNavItemSchema>
475+
| z.input<typeof UrlNavItemSchema>
476+
| z.input<typeof ReportNavItemSchema>
477+
| z.input<typeof ActionNavItemSchema>
478+
| z.input<typeof ComponentNavItemSchema>
479+
| z.input<typeof SeparatorNavItemSchema>
480+
| (z.input<typeof GroupNavItemSchema> & { children: NavigationItemInput[] });
481+
445482
/**
446483
* Recursive Union of all navigation item types.
447484
* Allows constructing an unlimited-depth navigation tree.
448485
*
449-
* The trailing cast to `z.ZodType<NavigationItem>` is forced by the member-array
450-
* widening below: `discriminatedUnion` reports the output of a
486+
* The trailing cast to `z.ZodType<NavigationItem, NavigationItemInput>` is forced
487+
* by the member-array widening below: `discriminatedUnion` reports the output of a
451488
* `ZodObject<ZodRawShape>` member as `Record<string, unknown>`, so the union's
452489
* inferred output carries no branch shapes at all. That fits the `z.ZodType<any>`
453490
* this used to be annotated with — and nothing sharper, which is how the exported
454491
* `NavigationItem` came to be `any` for every consumer (#4171).
455492
*
493+
* BOTH type parameters are spelled out. Naming only the first leaves `Input` at
494+
* its `unknown` default, which silently un-checks every authoring path through
495+
* this schema — see {@link NavigationItemInput}.
496+
*
456497
* What the cast does NOT weaken: every branch of {@link NavigationItem} is
457498
* `z.infer<typeof XNavItemSchema>`, so a key added to any variant's schema still
458499
* flows into the exported type with no edit here. What it leaves unchecked is
459500
* only the MEMBERSHIP of the list — a branch added to the schema and not to the
460501
* type, or the reverse — which app.test.ts covers at runtime by parsing all nine
461502
* ("accepts every variant with its full declared payload").
462503
*/
463-
export const NavigationItemSchema: z.ZodType<NavigationItem> = z.lazy(() =>
504+
export const NavigationItemSchema: z.ZodType<NavigationItem, NavigationItemInput> = z.lazy(() =>
464505
// DISCRIMINATED on `type` (#4001 PR B). With `.strict()` members a plain
465506
// union would answer one unknown key with an `invalid_union` aggregate
466507
// listing all nine branches' failures; discriminating on `type` first means
@@ -484,7 +525,7 @@ export const NavigationItemSchema: z.ZodType<NavigationItem> = z.lazy(() =>
484525
// The members are lazySchema Proxies and a superRefine-wrapped variant, so
485526
// the array is widened for the discriminator-typed overload; runtime
486527
// discrimination works on all of them (asserted in app.test.ts).
487-
] as unknown as readonly [z.ZodObject<z.ZodRawShape>, ...z.ZodObject<z.ZodRawShape>[]]) as unknown as z.ZodType<NavigationItem>
528+
] as unknown as readonly [z.ZodObject<z.ZodRawShape>, ...z.ZodObject<z.ZodRawShape>[]]) as unknown as z.ZodType<NavigationItem, NavigationItemInput>
488529
);
489530

490531
/**

0 commit comments

Comments
 (0)