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