From 66bae554e6651e304811e0062817e786890573fe Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 10:24:21 +0000 Subject: [PATCH] fix(types): retarget the objectstack#4171 inverted pins at their real trigger (#3177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec 17.0.0-rc.1 typed NavigationItem, FormField and ConditionalValidation.then/.otherwise, so the IsAny / IsUnknown pins fired. Firing was supposed to mean the burn-down was due. Per-symbol triage found it was not: `any` was never the only blocker for any of the three, so "no longer any" was never the right admission question. No symbol is bound. Each probe now asks the condition that actually governs it, asserts today's state (green now), and stops compiling the day that blocker lifts: - NavigationItem: flat shape vs the spec's nine-variant union, plus three semantics absent at BOTH spec tiers (visible: boolean, pinned, defaultOpen) and a labelled separator. Four probes, one per blocker. - FormField: two layers, not two dialects — disjoint required keys (local `name` vs spec `field`), and `field` itself is a string on one side and the resolved metadata object on the other. - ConditionalValidation: the branches became BaseValidationRuleShape, `{ type: string; …; [key: string]: unknown }` — no literal discriminant and an index signature that absorbs anything. The spec says so itself and names objectstack#4075 as the remaining work; the probe now pins that condition. What did become derivable is derived: NavigationItemType comes off the spec's nav-item discriminant instead of a hand-written nine-member copy, and recordMode / filters / badge / target / params / actionDef each come off the spec branch that owns them, extending the badgeVariant precedent. No member changes today. Also corrects the stale premise in check-spec-symbol-derivation.mjs, which still named these three as "the spec export resolves to any" — the guard's own warning about a wrong canonical-claim being a planted premise for the next session. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- .changeset/inverted-pin-burndown-probes.md | 45 +++++ .../src/__tests__/spec-derived-unions.test.ts | 157 ++++++++++++++++-- .../validation-rule-spec-parity.test.ts | 79 +++++++-- packages/types/src/app.ts | 114 ++++++++++--- packages/types/src/data-protocol.ts | 35 +++- scripts/check-spec-symbol-derivation.mjs | 38 ++++- 6 files changed, 390 insertions(+), 78 deletions(-) create mode 100644 .changeset/inverted-pin-burndown-probes.md diff --git a/.changeset/inverted-pin-burndown-probes.md b/.changeset/inverted-pin-burndown-probes.md new file mode 100644 index 0000000000..5e6344628c --- /dev/null +++ b/.changeset/inverted-pin-burndown-probes.md @@ -0,0 +1,45 @@ +--- +"@object-ui/types": patch +--- + +Fix the admission probes behind objectstack#4171's three inverted pins, and +derive the `NavigationItem` keys that genuinely became derivable (objectui#3177). + +Spec 17.0.0-rc.1 typed `NavigationItem`, `FormField` and +`ConditionalValidation.then`/`.otherwise`, so the `IsAny` / `IsUnknown` pins +guarding them fired. Firing was supposed to mean "the burn-down is due". A +per-symbol triage found it did not: **`any` was never the only blocker for any +of the three**, so "no longer `any`" was never the right admission question. +Nothing was bound; the probes now ask the condition that actually governs each +symbol, and each still asserts today's state — so they pass now and stop +compiling the day their own blocker lifts. + +- `NavigationItem` — the spec models navigation as a nine-variant discriminated + union; objectui keeps one flat shape, and the spec has no counterpart at + either tier for `visible: boolean` (which `menuItemToNavigationItem` + manufactures when it inverts legacy `MenuItem.hidden`), `pinned` (backs + `useNavPins`), the legacy `defaultOpen` spelling, or a separator carrying a + `label`. Four probes, one per blocker. +- `FormField` — two concepts on two layers, not two dialects of one: the + required keys are disjoint (objectui `name` = the form data path; spec + `field` = an object-field reference, with no `name` at either tier), and the + shared `field` key is a string on one side and the resolved metadata object + on the other. Binding would also collapse the objectui#3090 disambiguation + that exports `SpecFormField` separately, and revert framework#4074's + `dependsOn` widening. +- `ConditionalValidation` — the branches went from `unknown` to + `BaseValidationRuleShape`, which is `{ type: string; …; [key: string]: + unknown }`. Better than `unknown`, still not derivable: `type` is not a + literal union so a branch cannot narrow by discriminant, and the index + signature waves through any member — a typo'd `type: 'formatt'` included. The + spec says so itself and names the remaining work as objectstack#4075. The + probe now pins "literal discriminant / no index signature", so it goes green + exactly when that lands. + +What DID become derivable is derived. `NavigationItemType` now comes off the +spec's own nav-item discriminant instead of a hand-written nine-member copy — +the objectstack#4115 failure class, and it also makes a future spec variant a +compile error at exhaustive consumers rather than a silent `default:`. Same for +`recordMode`, `filters`, `badge`, `target`, `params` and `actionDef`, each taken +from the spec branch that owns it, extending the existing `badgeVariant` +precedent. No member changes today, so no consumer is affected. diff --git a/packages/types/src/__tests__/spec-derived-unions.test.ts b/packages/types/src/__tests__/spec-derived-unions.test.ts index 28877663aa..262b7ff6a4 100644 --- a/packages/types/src/__tests__/spec-derived-unions.test.ts +++ b/packages/types/src/__tests__/spec-derived-unions.test.ts @@ -61,14 +61,18 @@ import { import { FieldType as SpecFieldType, } from '@objectstack/spec/data'; -// The objectstack#4171 inverted pin must import the banned name to probe its -// any-ness — this guard is a sanctioned importer (#3090 tripwire). +// The objectstack#4171 / #3177 pins must import the banned name to probe it — +// this guard is a sanctioned importer (#3090 tripwire). /* eslint-disable no-restricted-imports -- reported at the specifier line, out of -next-line reach */ import type { NavigationItem as SpecNavigationItem, + NavigationItemInput as SpecNavigationItemInput, FormField as SpecFormField, + FormFieldInput as SpecFormFieldInput, } from '@objectstack/spec/ui'; /* eslint-enable no-restricted-imports */ +import type { NavigationItem, NavigationItemType } from '../app'; +import type { FormField } from '../form'; import type { BreakpointName } from '../mobile'; import type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError } from '../data'; import { @@ -145,40 +149,157 @@ const _validationErrorShape: ValidationError = { field: 'name', message: 'requir // objectui query-AST vocabulary they have become. /** - * Inverted pins — the tripwire FIRED on spec 17.0.0-rc.1 (#3177). + * Admission probes for `NavigationItem` and `FormField` (#3177). + * + * ## What these used to be, and why they were replaced * * `NavigationItem`, `JoinNode` and `FormField` used to collide with a spec * export whose own declaration resolved to `any` (the spec annotated the * recursive schemas behind them as `z.ZodType`, and `z.infer` of that is * `any`). Binding objectui's local interface to that would have replaced a * precise, documented shape with `any` — a type-safety regression wearing a - * burn-down's clothes — so they stayed local, and these pins asserted the + * burn-down's clothes — so they stayed local, and two `IsAny` pins asserted the * premise held. Filed upstream as objectstack#4171. * - * Two of them are now typed properly upstream, so the assertions below are - * inverted to record that fact rather than the old one. **They are a record, - * not a resolution**: the burn-down each one asks for — deriving objectui's - * `NavigationItem` / `FormField` from the spec — touches widely-used public - * types and is deliberately NOT bundled into a version bump. Tracked in #3177. + * Spec 17.0.0-rc.1 typed both properly and the `IsAny` pins fired. The #3177 + * triage then measured what the burn-down they demanded would actually cost — + * and found that **`any` was never the only blocker for either symbol**, so + * "no longer `any`" was never the right admission question. `IsAny` going + * `false` proves the spec type is no longer EMPTY; it says nothing about + * whether it is PRECISE enough to bind, which is what the burn-down needs. + * + * So the probes below ask the real question instead, one blocker per line. Each + * asserts the CURRENT state (so this file is green today) and stops compiling + * the day that specific blocker lifts — at which point that line names exactly + * what became derivable. `JoinNode` needs none of this: spec 17.0.0 retired the + * symbol (framework#4286), so there is no collision left to reason about. * - * `JoinNode`'s pin is gone entirely: spec 17.0.0 retired the symbol - * (framework#4286), so there is no collision left to reason about. + * ## Why not simply compare the two types * - * Mutual assignability CANNOT distinguish the `any` case on its own: `any` - * answers every `extends` question affirmatively, so a naive probe reports such - * a symbol as "identical to the spec" and recommends exactly the wrong edit. - * That is why these are written as explicit `IsAny` probes. + * Mutual assignability lies here, in three separate ways, all of them present + * in this repo (the list is `scripts/check-spec-symbol-derivation.mjs`'s): + * `any` answers every `extends` question affirmatively; so does `unknown` on + * one side; and objectui's `FormField` carries `[key: string]: any`, which + * absorbs any member the spec has and makes the two compare equal while they + * accept wildly different objects. A structural `extends` ALSO silently permits + * excess properties, so it cannot see that the spec declares no `pinned`. Hence + * per-key, per-tier probes rather than one verdict. */ type IsAny = 0 extends 1 & T ? true : false; -const _specNavigationItemIsStillAny = false satisfies IsAny; -const _specFormFieldIsStillAny = false satisfies IsAny; + +/** Every key of every branch of a union (plain `keyof` on a union gives the intersection). */ +type KeysOfUnion = T extends unknown ? keyof T : never; +/** Does the spec declare this key on ANY nav branch, at EITHER tier? */ +type SpecNavDeclares = + K extends KeysOfUnion | KeysOfUnion ? true : false; + +// ── NavigationItem: the three blockers, none of which `any` ever caused ────── +// +// Umbrella verdict: still not bindable. The lines under it say why, and are the +// ones to act on — this one stays `false` while ANY blocker remains. +const _localNavIsNotYetTheSpecUnion = false satisfies [NavigationItem] extends [SpecNavigationItem] + ? true + : false; + +// 1. `visible: boolean`. The spec takes a CEL string (input) / Expression +// envelope (output); neither tier admits a boolean. `NavigationRenderer` +// evaluates one, and `menuItemToNavigationItem` MANUFACTURES one when it +// inverts legacy `MenuItem.hidden`. Measured: binding to the input tier +// fails with 3x TS2322 on exactly those lines. +type SpecNavVisible = + | NonNullable['visible']> + | NonNullable['visible']>; +const _specNavVisibleStillRejectsBoolean = false satisfies boolean extends SpecNavVisible + ? true + : false; + +// 2. Keys the spec has no counterpart for at either tier. `pinned` backs +// `useNavPins` + `FavoritesProvider`; `defaultOpen` is the legacy spelling +// `navigation-spec-parity.test.ts` keeps accepting for published metadata. +// If the spec ever claims either NAME, this fails and the two meanings must +// be reconciled rather than silently shadowed. +const _specNavStillHasNoPinned = false satisfies SpecNavDeclares<'pinned'>; +const _specNavStillHasNoDefaultOpen = false satisfies SpecNavDeclares<'defaultOpen'>; + +// 3. objectui's separator carries a `label`; the spec's separator branch +// declares only `type` / `id?` / `order?`. `menuItemToNavigationItem` emits +// one (measured: TS2353), so this is load-bearing, not decorative. +const _specSeparatorStillHasNoLabel = false satisfies 'label' extends keyof Extract< + SpecNavigationItem, + { type: 'separator' } +> + ? true + : false; + +// What IS derivable today is derived: `app.ts` now takes `NavigationItemType` +// off the spec's discriminant, and `recordMode` / `filters` / `badge` / +// `target` / `params` / `actionDef` / `badgeVariant` off the branch that owns +// each. This asserts the membership list really is the spec's — a restatement +// that drops a member (the objectstack#4115 failure class) fails here. +const _navTypeCoversSpec = null as unknown as SpecNavigationItem['type'] satisfies NavigationItemType; + +// ── FormField: not one concept in two dialects, but two concepts on two layers ─ +// +// `select-option-spec-parity.test.ts` states the distinction in its own header — +// "Unlike the FormField pair — two genuinely different concepts on two layers — +// a select option is ONE concept in two dialects" — and `index.ts` exports +// `SpecFormField` SEPARATELY as the disambiguation the #3090 tripwire exists to +// force. Binding would make `FormField === SpecFormField` and collapse that. +// +// The decisive, mechanical form of "two layers": the two types' REQUIRED keys +// are disjoint. objectui requires `name` (the form data path); the spec requires +// `field` (a reference to an object field) and has no `name` at either tier. +const _specFormFieldIsNoLongerAny = false satisfies IsAny; +const _specFormFieldStillHasNoName = false satisfies 'name' extends keyof SpecFormField + ? true + : false; +const _specFormFieldInputStillHasNoName = false satisfies 'name' extends keyof SpecFormFieldInput + ? true + : false; + +// The same key on both sides, meaning different things — the pun `form.ts` +// flags with a ⚠️. The spec's `field` is the referenced field's NAME; on a +// runtime `FormField` the slot holds the RESOLVED metadata object, and +// `normalizeSectionField` (@object-ui/plugin-form) is the only place the two +// layers meet. +const _specFieldSlotIsStillAName = true satisfies [SpecFormField['field']] extends [string] + ? true + : false; +const _localFieldSlotIsStillAnObject = false satisfies [NonNullable] extends [ + string, +] + ? true + : false; + +// framework#4074 widened objectui's `dependsOn` to match its runtime reader +// (`resolveCascadingOptions` has always accepted arrays and `{ field, param }` +// entries). The spec still says `string`, so binding would revert that fix. +const _specDependsOnStillTakesNoArray = false satisfies string[] extends NonNullable< + SpecFormFieldInput['dependsOn'] +> + ? true + : false; + +// ADR-0089 D2 folds `visibleOn` into `visibleWhen` at the spec's schema +// boundary, so it is absent from the OUTPUT type by construction — while +// objectui's #2212 wire contract keeps it. (The spec's input tier still +// accepts it; this asks the output tier on purpose.) +const _specOutputStillDropsVisibleOn = false satisfies 'visibleOn' extends keyof SpecFormField + ? true + : false; void _chartCovers; void _reportCovers; void _actionCovers; void _pageCovers; void _vizCovers; void _runnableCovers; void _componentCovers; void _paramFieldCovers; void _resolvableCovers; void _fieldBackedParam; void _minimalTypedParam; void _breakpointCovers; void _importModeCovers; void _importStatusCovers; void _exportStatusCovers; void _validationErrorShape; -void _specNavigationItemIsStillAny; void _specFormFieldIsStillAny; +void _localNavIsNotYetTheSpecUnion; void _specNavVisibleStillRejectsBoolean; +void _specNavStillHasNoPinned; void _specNavStillHasNoDefaultOpen; +void _specSeparatorStillHasNoLabel; void _navTypeCoversSpec; +void _specFormFieldIsNoLongerAny; void _specFormFieldStillHasNoName; +void _specFormFieldInputStillHasNoName; void _specFieldSlotIsStillAName; +void _localFieldSlotIsStillAnObject; void _specDependsOnStillTakesNoArray; +void _specOutputStillDropsVisibleOn; /** Read a spec enum's members, failing loudly if the shape ever changes. */ const optionsOf = (schema: unknown, name: string): string[] => { diff --git a/packages/types/src/__tests__/validation-rule-spec-parity.test.ts b/packages/types/src/__tests__/validation-rule-spec-parity.test.ts index 0fe4d7f133..7a675401be 100644 --- a/packages/types/src/__tests__/validation-rule-spec-parity.test.ts +++ b/packages/types/src/__tests__/validation-rule-spec-parity.test.ts @@ -104,19 +104,55 @@ const _conditionalNonBranchKeysAreSpec = true satisfies Equal< // …and the branches are precise here, not `unknown`. const _conditionalBranchIsTyped = false satisfies IsUnknown; -// INVERTED PIN (objectstack#4171) — FIRED on spec 17.0.0-rc.1 (#3177). +// ── The branch probes, retargeted (#3177) ─────────────────────────────────── // -// The divergence above was only justified while the spec's own -// `then`/`otherwise` erased to `unknown`. They no longer do, so the assertions -// are inverted to record that. **A record, not a resolution**: what the pin asks -// for — deleting the divergence and deriving `ConditionalValidation` whole — is -// deliberately not bundled into a version bump. Tracked in #3177. -const _specThenIsStillUnknown = false satisfies IsUnknown< - z.input['then'] ->; -const _specOtherwiseIsStillUnknown = false satisfies IsUnknown< - z.input['otherwise'] ->; +// These used to ask `IsUnknown<…>`, on the reasoning that the divergence was +// justified only while the spec's `then`/`otherwise` erased to `unknown`. Spec +// 17.0.0-rc.1 made that `false` and the pins fired — but the #3177 triage found +// the pins had asked too weak a question, and firing did NOT mean the branches +// had become derivable. +// +// What they actually became is `BaseValidationRuleShape`: +// +// interface BaseValidationRuleShape { +// type: string; name: string; message: string; …; [key: string]: unknown; +// } +// +// The spec says so itself, at that type's declaration: "it types the KNOWN keys +// and accepts any others. That is a real improvement over `unknown` (which types +// nothing) but **it is not strictness** — the discriminated union below is what +// actually rejects a malformed rule, at parse time. Removing the index signature +// is the #4075 family of work, not this change." +// +// So deriving wholesale today would trade a 9-member discriminated union for a +// bag: `then.type` degrades from a literal union to `string`, `then.condition` +// to `unknown`, and a typo'd `type: 'formatt'` starts compiling. That is the +// same regression the original pin existed to prevent, one notch weaker — and +// it lands where it costs most, because renderers read plain objects and never +// parse, so the authoring-time discriminant is the ONLY gate on these rules. +// +// The probes below therefore ask the pin's TRUE trigger — "does the spec type +// them PROPERLY", not "does it type them at all". Both assert today's state, so +// this file is green now; both stop compiling when objectstack#4075 removes the +// index signature and the branches become a real union, which is when the +// wholesale derivation genuinely comes due. +type SpecThen = z.input['then']; +type SpecOtherwise = NonNullable['otherwise']>; + +/** A discriminated union has a LITERAL `type`; a bag widens it to `string`. */ +type HasLiteralDiscriminant = string extends T['type'] ? false : true; +/** `[key: string]: unknown` absorbs any member, so nothing is rejected at authoring time. */ +type HasIndexSignature = string extends keyof T ? true : false; + +const _specThenStillCannotNarrow = false satisfies HasLiteralDiscriminant; +const _specOtherwiseStillCannotNarrow = false satisfies HasLiteralDiscriminant; +const _specThenStillCarriesAnIndexSignature = true satisfies HasIndexSignature; +const _specOtherwiseStillCarriesAnIndexSignature = true satisfies HasIndexSignature; + +// Kept as the record of WHY the predicate had to change: the branches are no +// longer `unknown`, so the original pin's own stated condition really did lapse +// — it was simply not the condition that governed the burn-down. +const _specThenIsNoLongerUnknown = false satisfies IsUnknown; /** * The spec ships these as `lazySchema()` thunks, so `.shape` is only reachable @@ -332,13 +368,20 @@ describe('predicates are on the ExpressionInput wire shape, not `string`', () => }); }); -describe('pinned divergence: `then` / `otherwise` (objectstack#4171)', () => { +describe('pinned divergence: `then` / `otherwise` (objectstack#4171, #4075)', () => { // The type half of this divergence is pinned at the top of this file - // (`_specThenIsStillUnknown` / `_conditionalBranchIsTyped`), because that is - // the half a runtime assertion cannot reach. What IS observable at runtime is - // that the spec's own parser still rejects a nonsense branch — so the erasure - // is purely a published-typing defect, not a loosened contract, and objectui - // re-typing the branch cannot admit anything the server would refuse. + // (`_specThenStillCannotNarrow` / `_specThenStillCarriesAnIndexSignature` / + // `_conditionalBranchIsTyped`), because that is the half a runtime assertion + // cannot reach. What IS observable at runtime is that the spec's own parser + // still rejects a nonsense branch — so the weak typing is purely a published- + // typing defect, not a loosened contract, and objectui re-typing the branch + // cannot admit anything the server would refuse. + // + // That asymmetry is the whole argument for keeping the divergence: the spec + // REJECTS this payload at parse time and DESCRIBES it as legal at compile + // time. objectui's renderers read plain objects and never parse, so only the + // compile-time half is ever consulted on the client — which is precisely why + // adopting the spec's weaker branch type would matter. it('is a typing gap only — the spec`s parser still rejects a nonsense branch', () => { expect(() => ConditionalValidationSchema.parse({ diff --git a/packages/types/src/app.ts b/packages/types/src/app.ts index 02b3f67d3f..c430e93c1b 100644 --- a/packages/types/src/app.ts +++ b/packages/types/src/app.ts @@ -19,11 +19,37 @@ * configurations should use `NavigationItem` and the `navigation` / `areas` fields. */ -// The spec's own `NavigationItem` erases to `any` (its schema is declared -// `z.ZodType` to carry the recursive group variant), so objectui keeps a -// real interface. Its per-variant types ARE properly typed, though, so the -// fields below are derived from one of them rather than restated. -import type { ObjectNavItem as SpecObjectNavItem } from '@objectstack/spec/ui'; +// Spec 17.0.0-rc.1 gave the spec's own `NavigationItem` a real type — it is no +// longer the `z.ZodType` erasure objectstack#4171 was filed about. That +// removed the ORIGINAL reason this stayed a local interface, but not the +// remaining ones (#3177 triage), and they were never caused by `any`: +// +// 1. Shape. The spec models navigation as a discriminated union of nine +// `$strict` variants with a `superRefine` exclusivity rule; this is one +// flat, all-optional shape. Converging is a breaking change for every +// consumer that reads a target field without narrowing — the verdict +// already recorded in `__tests__/navigation-spec-parity.test.ts`. +// 2. Three semantics the spec has at NEITHER tier: `visible: boolean` (the +// spec takes only a CEL string / Expression envelope, yet +// `menuItemToNavigationItem` below inverts legacy `MenuItem.hidden` into +// a boolean), `pinned` (backs `useNavPins` + `FavoritesProvider`), and the +// legacy `defaultOpen` spelling. Binding deletes all three from the type +// while their implementations keep running. +// 3. A separator carrying a `label`; the spec's separator branch declares none. +// +// So the symbol stays local and the KEYS come off the spec one by one — the +// `badgeVariant` precedent (objectstack#4115), widened here to every key with a +// precise spec counterpart. That is the part of the burn-down that is safe +// today: a restated enum or payload shape can drift, and now cannot. +// `spec-derived-unions.test.ts` pins the three blockers above, each written so +// it fails the day the spec closes it. +import type { + NavigationItem as SpecNavigationItem, + ObjectNavItem as SpecObjectNavItem, + UrlNavItem as SpecUrlNavItem, + ActionNavItem as SpecActionNavItem, + ComponentNavItem as SpecComponentNavItem, +} from '@objectstack/spec/ui'; import type { BaseSchema } from './base'; // ============================================================================ @@ -32,17 +58,19 @@ import type { BaseSchema } from './base'; /** * Navigation item type — determines the target and required fields. + * + * The MEMBERSHIP list is the spec's, read off the discriminant of its nav-item + * union rather than restated (#3177). A hand-written copy of a spec vocabulary + * is the objectstack#4115 failure class — `ChartType` carried 7 of 19 members, + * `ActionType` was missing `form` — and this one had drifted into an identical + * nine-member copy that nothing checked. + * + * Deriving also makes a future spec addition LOUD instead of silent: exhaustive + * consumers (`NAV_TYPE_META` in `plugin-designer`'s `NavigationDesigner` is a + * `Record< NavigationItemType, … >`) stop compiling until the new variant is + * handled, which is where a renderer gap belongs — not in a dead `default:`. */ -export type NavigationItemType = - | 'object' - | 'dashboard' - | 'page' - | 'report' - | 'url' - | 'component' - | 'group' - | 'separator' - | 'action'; +export type NavigationItemType = SpecNavigationItem['type']; /** * Unified Navigation Item @@ -93,8 +121,11 @@ export interface NavigationItem { /** * Record opening mode when `recordId` is set. Defaults to `'view'`. * Use `'edit'` to land directly on the edit form (e.g. "Edit my profile"). + * + * Derived from the spec's own object-nav variant (#3177) — a restated + * two-member enum is one spec release away from drifting. */ - recordMode?: 'view' | 'edit'; + recordMode?: NonNullable; /** * URL filter conditions (for type: 'object') — the entry targets the @@ -109,8 +140,10 @@ export interface NavigationItem { * be resolved are dropped from the URL. * * Precedence within `type: 'object'`: `recordId` → `filters` → `viewName`. + * + * Shape derived from the spec's object-nav variant (#3177). */ - filters?: Record; + filters?: SpecObjectNavItem['filters']; /** Target dashboard name (for type: 'dashboard') */ dashboardName?: string; @@ -124,8 +157,16 @@ export interface NavigationItem { /** Target URL (for type: 'url') */ url?: string; - /** Link target (for type: 'url') */ - target?: '_blank' | '_self'; + /** + * Link target (for type: 'url'). + * + * Derived from the spec's url-nav variant (#3177). The spec declares it + * `.default('_self')`, so it is REQUIRED in that schema's output and optional + * here — objectui reads a plain object and never parses, so the default is + * never applied and the key is genuinely absent. `NonNullable` takes the + * member list without importing that requiredness. + */ + target?: NonNullable; /** * Target component reference (for type: 'component') — a colon-joined @@ -140,8 +181,11 @@ export interface NavigationItem { * querystring so the same component/page can be reused across nav entries * with different inputs (e.g. `params: { type: 'object' }`). String values * support the same template variables as `recordId`. + * + * Shape derived from the spec's component-nav variant (#3177); the page + * variant declares the identical shape. */ - params?: Record; + params?: SpecComponentNavItem['params']; // -- Grouping -- @@ -150,7 +194,16 @@ export interface NavigationItem { // -- Visibility & Permissions -- - /** Visibility expression — boolean or expression string e.g. "${user.role === 'admin'}" */ + /** + * Visibility expression — boolean or expression string e.g. "${user.role === 'admin'}". + * + * NOT derived (#3177): the spec takes a CEL string on input and an Expression + * envelope `{ dialect, source }` on output — `boolean` is absent at BOTH + * tiers. `NavigationRenderer`'s `evalVis(item.visible)` honours the boolean, + * and `menuItemToNavigationItem` produces one when mapping legacy + * `MenuItem.hidden`. Pinned in `spec-derived-unions.test.ts`, which fails the + * day the spec accepts a boolean and this can come off it. + */ visible?: boolean | string; /** Required permissions to see/access this item */ @@ -173,8 +226,8 @@ export interface NavigationItem { // -- UX Enhancements -- - /** Badge text or count */ - badge?: string | number; + /** Badge text or count — derived from the spec's nav-item base (#3177). */ + badge?: NonNullable; /** * Badge visual variant — derived from the spec's own nav-item variant @@ -201,10 +254,21 @@ export interface NavigationItem { * Action payload for `type: 'action'` items. Without it the item names an * action it cannot invoke — and before this was declared, `objectui validate` * silently stripped it, so a broken action item validated clean. + * + * Derived from the spec's action-nav variant (#3177). This is a structured + * payload rather than a scalar, so a restatement is exactly the kind that + * goes stale unnoticed — which is how it came to be stripped in the first + * place (objectstack#4115). */ - actionDef?: { actionName: string; params?: Record }; + actionDef?: NonNullable; - /** Whether this item is pinned. objectui-only; the spec has no counterpart. */ + /** + * Whether this item is pinned. objectui-only; the spec has no counterpart at + * either tier, so NOT derived (#3177). `useNavPins` and `FavoritesProvider` + * are built on it. Pinned in `spec-derived-unions.test.ts`, which fails the + * day the spec claims the name — at which point the two meanings have to be + * reconciled rather than silently shadowed. + */ pinned?: boolean; /** Sort order weight (lower = higher) */ diff --git a/packages/types/src/data-protocol.ts b/packages/types/src/data-protocol.ts index 3dbdbe5556..ba0c35cdce 100644 --- a/packages/types/src/data-protocol.ts +++ b/packages/types/src/data-protocol.ts @@ -946,15 +946,32 @@ export type FormatValidation = z.input; /** * Conditional validation — evaluate `when`, then apply `then` or `otherwise`. * - * PINNED DIVERGENCE (objectstack#4171): every key comes from the spec except - * `then` / `otherwise`, which the spec's published types erase to `unknown` - * (its `ValidationRuleSchema` is annotated `z.ZodType`, - * an index-signature bag). Re-exporting that would replace a discriminated - * union with `unknown` — a type-safety regression wearing a burn-down's - * clothes. They are re-typed to objectui's union here, and - * `validation-rule-spec-parity.test.ts` carries an inverted pin that fails the - * day the spec types them properly, so this divergence cannot outlive its - * reason. + * PINNED DIVERGENCE (objectstack#4171, narrowed to objectstack#4075 by #3177): + * every key comes from the spec except `then` / `otherwise`. + * + * Those two used to erase to `unknown`. Spec 17.0.0-rc.1 typed them — as + * `BaseValidationRuleShape`, which is `{ type: string; name: string; message: + * string; …; [key: string]: unknown }`. That is better than `unknown` and still + * not enough to derive from: `type` is `string` rather than a literal union, so + * a branch cannot narrow by discriminant, `then.condition` reads back as + * `unknown`, and the index signature waves through any member at all — a typo'd + * `type: 'formatt'` included. The spec's own comment on `ValidationRuleSchema` + * says as much and names the remaining work: "it is not strictness … Removing + * the index signature is the #4075 family of work, not this change." + * + * So the branches stay re-typed to objectui's discriminated union here. What + * changed in #3177 is the tripwire, not the divergence: + * `validation-rule-spec-parity.test.ts` used to pin "the spec still says + * `unknown`" — too weak a question, which fired without the burn-down having + * become correct. It now pins the condition that actually governs (literal + * discriminant / no index signature), so this divergence still cannot outlive + * its reason, but the reason is stated accurately. + * + * Why this side of the trade matters more than it looks: renderers read plain + * objects and never parse, so the spec's parse-time union — the thing that DOES + * reject a malformed rule — is not on the client path at all. The compile-time + * discriminant here is the only gate an authored (or AI-generated) rule meets + * before it runs. */ export type ConditionalValidation = Omit< z.input, diff --git a/scripts/check-spec-symbol-derivation.mjs b/scripts/check-spec-symbol-derivation.mjs index a9c8d5aa91..d81a35cf2a 100644 --- a/scripts/check-spec-symbol-derivation.mjs +++ b/scripts/check-spec-symbol-derivation.mjs @@ -191,14 +191,36 @@ const ALLOW = { // annotated `z.ZodType` (`FilterConditionSchema`, // `NavigationItemSchema`). `any` answers every assignability question // affirmatively. Detect: `0 extends (1 & Local) ? true : false`. -// 2. The SPEC export resolves to `any` (`NavigationItem`, `JoinNode`, -// `FormField`). Re-exporting these REPLACES a precise local interface with -// `any` — a type-safety regression wearing a burn-down's clothes. These -// cannot be burned down here at all; the fix belongs upstream in the spec, -// filed as objectstack#4171. `spec-derived-unions.test.ts` carries an -// inverted pin that fails the day the spec types one of them properly. -// Detect: the same `0 extends (1 & Spec)` probe — but see the variant -// below, which that probe does NOT catch. +// 2. The SPEC export resolves to `any`. Re-exporting such a symbol REPLACES a +// precise local interface with `any` — a type-safety regression wearing a +// burn-down's clothes. Detect: the same `0 extends (1 & Spec)` probe — but +// see 2b and 2c, which that probe does NOT catch. +// +// History worth keeping, because it is the reason this whole list exists: +// `NavigationItem`, `JoinNode` and `FormField` were the instances, filed +// upstream as objectstack#4171. All three are resolved as premises now — +// `JoinNode` was retired with `query.joins` (framework#4286) and the other +// two were typed properly in spec 17.0.0-rc.1 — and NEITHER became +// derivable as a result (objectui#3177). Do not read "no longer `any`" as +// "safe to bind"; see 2c. +// 2c. The SPEC export is typed but NOT PRECISE, which the `any` and `unknown` +// probes both report as clean. Two live instances, each pinned with a probe +// that asks its real blocker (objectui#3177): +// - `ConditionalValidation.then` / `.otherwise` are +// `BaseValidationRuleShape` — `type: string` plus `[key: string]: +// unknown`, i.e. case 3 on the SPEC side. Deriving swaps a +// discriminated union for a bag. Blocked on objectstack#4075. +// Detect: `string extends Spec['type']`, and the case-3 probe. +// - `NavigationItem` / `FormField` are precise but describe a DIFFERENT +// shape (a nine-variant union vs objectui's flat one) or a different +// LAYER (spec `field` = an object-field reference; objectui `name` = +// the form data path, with disjoint required keys). Precision is not +// equivalence. Detect: per-key, per-tier probes — a structural +// `extends` permits excess properties and so cannot see a key the spec +// does not declare. +// Both sets live in `spec-derived-unions.test.ts` / +// `validation-rule-spec-parity.test.ts`, written to fail the day the +// blocker they name lifts. // 2b. The SPEC export resolves to `unknown` (`JoinedReportBlock`, whose // `JoinedReportBlockSchema` the spec declares as `z.ZodTypeAny`). Just as // empty as case 2 and just as unburnable, but the `any` probe reports