|
| 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. |
0 commit comments