Skip to content

Commit 42eeb7d

Browse files
authored
fix(spec): type-check defineApp's navigation again — #4171 fixed only half the annotation (#3786) (#4221)
`NavigationItemSchema` is recursive and carries a hand-written annotation. #4171 fixed its Output half (it was `z.ZodType<any>`). `z.ZodType` takes `<Output, Input>` and Input DEFAULTS TO `unknown`, so naming only the first parameter left `z.input<typeof AppSchema>` resolving `navigation` to `unknown` — and `defineApp`, the documented authoring entry point, compiled `navigation: [{ totally: 'made up' }, 42, 'nonsense']` clean. Only the compile-time contract lied, which is why no runtime test noticed. Adds `NavigationItemInput` and names both parameters. The unions must stay distinct: `GroupNavItemSchema.expanded` and `UrlNavItemSchema.target` carry `.default()`, so those keys are required in the parsed output and omissible when authoring. `app.nav-type-assertions.ts` pins both unions AND the wiring to the schema — a correct input union that no schema references would leave every authoring path back at `unknown`. Mutation-tested: dropping the Input parameter, removing the `expanded` default, and leaking `children` onto a flat nav branch each turn the matching assertion red. Also corrects the PLATFORM_ALWAYS_ON_CAPABILITIES comment, which named `analytics` as a hosted-slate gap. Re-measured, it is force-mounted via cloud's `defaultRequires`; the earlier claim came from diffing an array with no production call site.
1 parent 8a341a4 commit 42eeb7d

5 files changed

Lines changed: 326 additions & 11 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
@@ -3352,6 +3352,7 @@
33523352
"NavigationContribution (type)",
33533353
"NavigationContributionSchema (const)",
33543354
"NavigationItem (type)",
3355+
"NavigationItemInput (type)",
33553356
"NavigationItemSchema (const)",
33563357
"NavigationModeSchema (const)",
33573358
"Notification (type)",

packages/spec/src/kernel/platform-capabilities.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,22 @@ export const PLATFORM_CAPABILITY_PROVIDERS: Readonly<Record<string, PlatformCapa
180180
* Published here rather than left on `Serve.ALWAYS_ON_CAPABILITIES` for the same
181181
* reason {@link PLATFORM_CAPABILITY_PROVIDERS} was: **more than one runtime
182182
* mounts this slate.** The CLI's `serve` builds one kernel per process; cloud's
183-
* objectos-runtime builds one per tenant environment and carried its own copy,
184-
* under a CLI comment that merely said such hosts "mirror this list". They had
185-
* already diverged — the hosted slate was missing `sms`, `messaging` and
186-
* `analytics` — so an app that worked under `objectstack serve` could lose
187-
* dataset previews and `notify` deliveries once hosted, silently, with no error
188-
* anywhere (cloud#925, framework#3786). A second list nobody checks is how that
189-
* happens; one exported list is how it stops.
183+
* objectos-runtime builds one per tenant environment, from its own wiring, under
184+
* a CLI comment that merely said such hosts "mirror this list" — a claim nothing
185+
* checked. An app that works under `objectstack serve` can therefore lose
186+
* capabilities once hosted, silently, with no error anywhere (cloud#925,
187+
* framework#3786). A second description nobody checks is how that happens; one
188+
* exported list is how it stops.
189+
*
190+
* On the shape of that divergence, since an earlier revision of this comment got
191+
* it wrong: cloud does not keep a rival slate ARRAY to diff against. Its hosted
192+
* set is assembled from the host's `defaultRequires` plus the capability
193+
* loader's dependency patches, so the comparison has to be against what actually
194+
* mounts, not against the nearest list-shaped thing. (The array this comment
195+
* once cited, `mountDefaultEnvironmentPlugins`' `ORDER`, has no production call
196+
* site — diffing it proved nothing.) Measured against the real wiring,
197+
* `analytics` IS force-mounted there and was never the gap; `messaging` loads
198+
* only when `audit` does; `sms` has no provider package in that image at all.
190199
*
191200
* This is the FLOOR, not the ceiling: a host may mount more (cloud adds
192201
* `observability`), and `objectstack serve --preset minimal` opts out entirely.
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 }] });

0 commit comments

Comments
 (0)