Skip to content

Commit 15209ff

Browse files
os-zhuangclaude
andauthored
fix(types,layout): navigation metadata stops losing the spec fields the renderer already honours (objectstack#4115) (#3088)
`objectui validate` — a published CLI command — is the ONLY runtime consumer of objectui's zod schemas; renderers read plain objects and never parse. So every gap below was a CLI defect, and because the schema strips unknown keys instead of erroring, all of them failed silently. Measured against spec 17.0.0-rc.0, `NavigationItemSchema` was: - dropping `requiresObject` / `requiresService`. These are capability gates that `NavigationItem` has always declared and `NavigationRenderer` has always enforced — only the schema lagged, so validating an app stripped the gates from its own nav metadata. - dropping `actionDef`, i.e. an action item's entire payload. An item that could never invoke anything validated clean. - dropping `expanded`, the spec's spelling of group expansion. The renderer already read it — behind `(item as any).expanded`, because the TS type did not declare it. That cast is now gone. - rejecting `badgeVariant: 'secondary'`, which the spec accepts. The union is now derived from the spec's own `ObjectNavItem` instead of restated. - rejecting `{ type: 'separator' }`, which the spec accepts and which carries no identity or text by definition. `NavigationArea` lost `order` and `description` the same way. The separator fix needed a second pass: declaring `id`/`label` optional let a bare separator through but also waved through an id-less `type: 'object'` item — caught by two existing assertions in navigation-model.test.ts. A `superRefine` now re-imposes both for every non-separator type, and a new assertion pins that boundary so the exemption cannot be widened silently. NOT changed: the spec models navigation as a discriminated union of nine variants with per-variant required targets and an exclusivity refinement; objectui keeps one flat all-optional shape, so it still accepts items the spec would reject. Converging is a breaking change for every consumer that reads fields off `NavigationItem` without narrowing — tracked separately. The four ledger entries stay: spec's `NavigationItemSchema` is declared `z.ZodType<any>` (its recursive group variant erases the union), so a re-export would delete all navigation typing. This fixes the drift, not the name collision. Mutation-tested: dropping each of the four schema fixes fails its assertion by name, and disabling the separator refinement fails three. 78/78 type-check; full suite 8932 assertions green. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent bebaebd commit 15209ff

4 files changed

Lines changed: 196 additions & 10 deletions

File tree

packages/layout/src/NavigationRenderer.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,7 @@ function NavigationItemRenderer({
866866
// group — otherwise an auto-collapsed group hides the active item
867867
// and the user loses orientation.
868868
const explicitOpen = (() => {
869-
const expanded = (item as any).expanded;
870-
if (typeof expanded === 'boolean') return expanded;
869+
if (typeof item.expanded === 'boolean') return item.expanded;
871870
if (typeof item.defaultOpen === 'boolean') return item.defaultOpen;
872871
return undefined;
873872
})();
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Navigation ↔ `@objectstack/spec` drift guard (objectstack#4115).
11+
*
12+
* `NavigationItemSchema` is the only navigation schema with a *runtime*
13+
* consumer: `@object-ui/cli`'s published `objectui validate` command parses
14+
* metadata through `AnyComponentSchema`, which reaches it via `AppSchema`.
15+
* Renderers read plain objects and never parse, so every gap below was a CLI
16+
* defect — and a strip-mode schema fails silently, which is why they survived.
17+
*
18+
* The gaps this pins, all measured against spec 17.0.0-rc.0:
19+
*
20+
* - `requiresObject` / `requiresService` — capability gates that
21+
* `NavigationItem` has always declared and `NavigationRenderer` has always
22+
* honoured. Only the schema lagged, so `objectui validate` dropped them.
23+
* - `actionDef` — an action item's entire payload. Stripped, so an item that
24+
* could never invoke anything validated clean.
25+
* - `expanded` — the spec's spelling of group expansion. The renderer already
26+
* read it (behind an `as any`, because the type did not declare it).
27+
* - `badgeVariant: 'secondary'` — spec-valid, hard-rejected here.
28+
* - `{ type: 'separator' }` — spec-valid, rejected for missing id/label.
29+
*
30+
* Deliberately NOT modelled: the spec expresses navigation as a discriminated
31+
* union of nine variants, each with its target field required and a
32+
* `superRefine` exclusivity rule. objectui keeps one flat, all-optional shape,
33+
* so it accepts items the spec would reject (e.g. `type: 'object'` with no
34+
* `objectName`). Converging on the union is a breaking change for every
35+
* consumer that reads fields off `NavigationItem` without narrowing — tracked
36+
* separately, not smuggled in here.
37+
*/
38+
39+
import { describe, it, expect } from 'vitest';
40+
import { NavigationItemSchema, NavigationAreaSchema } from '../zod/app.zod.js';
41+
42+
/** Parse and return the surviving object, so "accepted" cannot hide a strip. */
43+
function keep(input: unknown): Record<string, unknown> | null {
44+
const r = NavigationItemSchema.safeParse(input);
45+
return r.success ? (r.data as Record<string, unknown>) : null;
46+
}
47+
48+
describe('NavigationItemSchema keeps the spec vocabulary it used to drop', () => {
49+
it('keeps the capability gates the renderer actually reads', () => {
50+
const kept = keep({ id: 'apps', type: 'object', label: 'Apps', objectName: 'sys_app', requiresObject: 'sys_app' });
51+
expect(kept).not.toBeNull();
52+
expect(kept!.requiresObject).toBe('sys_app');
53+
54+
const svc = keep({ id: 'ai', type: 'url', label: 'AI', url: '/ai', requiresService: 'ai' });
55+
expect(svc!.requiresService).toBe('ai');
56+
});
57+
58+
it('keeps an action item\'s payload instead of validating an empty shell', () => {
59+
const kept = keep({ id: 'run', type: 'action', label: 'Run', actionDef: { actionName: 'sync_now' } });
60+
expect(kept).not.toBeNull();
61+
expect(kept!.actionDef).toEqual({ actionName: 'sync_now' });
62+
});
63+
64+
it('keeps the spec spelling of group expansion', () => {
65+
const kept = keep({ id: 'grp', type: 'group', label: 'Group', children: [], expanded: true });
66+
expect(kept!.expanded).toBe(true);
67+
});
68+
69+
it('still accepts the legacy defaultOpen spelling', () => {
70+
const kept = keep({ id: 'grp', type: 'group', label: 'Group', children: [], defaultOpen: true });
71+
expect(kept!.defaultOpen).toBe(true);
72+
});
73+
74+
it('accepts every spec badge variant, including secondary', () => {
75+
for (const variant of ['default', 'secondary', 'destructive', 'outline']) {
76+
const kept = keep({ id: 'x', type: 'url', label: 'X', url: '/x', badgeVariant: variant });
77+
expect(kept, `badgeVariant '${variant}' was rejected`).not.toBeNull();
78+
expect(kept!.badgeVariant).toBe(variant);
79+
}
80+
});
81+
82+
it('accepts a bare separator, which carries no identity or text by definition', () => {
83+
expect(keep({ type: 'separator' })).not.toBeNull();
84+
});
85+
86+
it('exempts ONLY the separator from needing id + label', () => {
87+
// The separator exemption is implemented by declaring `id`/`label`
88+
// optional, so without the refinement that re-imposes them this would
89+
// also wave through an unidentifiable destination.
90+
expect(keep({ type: 'object', label: 'No id', objectName: 'contact' })).toBeNull();
91+
expect(keep({ id: 'no_label', type: 'object', objectName: 'contact' })).toBeNull();
92+
expect(keep({ id: 'ok', type: 'object', label: 'Ok', objectName: 'contact' })).not.toBeNull();
93+
});
94+
95+
it('still rejects an unknown item type', () => {
96+
// The permissiveness above is scoped to spec vocabulary — it must not
97+
// become "anything goes".
98+
expect(keep({ id: 'x', type: 'wormhole', label: 'X' })).toBeNull();
99+
});
100+
});
101+
102+
describe('NavigationAreaSchema keeps the spec fields it used to drop', () => {
103+
it('keeps order and description', () => {
104+
const r = NavigationAreaSchema.safeParse({
105+
id: 'sales',
106+
label: 'Sales',
107+
order: 2,
108+
description: 'Pipeline and accounts',
109+
navigation: [],
110+
});
111+
expect(r.success).toBe(true);
112+
expect(r.success && r.data.order).toBe(2);
113+
expect(r.success && r.data.description).toBe('Pipeline and accounts');
114+
});
115+
});

packages/types/src/app.ts

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
* configurations should use `NavigationItem` and the `navigation` / `areas` fields.
2020
*/
2121

22+
// The spec's own `NavigationItem` erases to `any` (its schema is declared
23+
// `z.ZodType<any>` to carry the recursive group variant), so objectui keeps a
24+
// real interface. Its per-variant types ARE properly typed, though, so the
25+
// fields below are derived from one of them rather than restated.
26+
import type { ObjectNavItem as SpecObjectNavItem } from '@objectstack/spec/ui';
2227
import type { BaseSchema } from './base';
2328

2429
// ============================================================================
@@ -171,13 +176,35 @@ export interface NavigationItem {
171176
/** Badge text or count */
172177
badge?: string | number;
173178

174-
/** Badge visual variant */
175-
badgeVariant?: 'default' | 'destructive' | 'outline';
179+
/**
180+
* Badge visual variant — derived from the spec's own nav-item variant
181+
* rather than restated (objectstack#4115). The hand-written union this
182+
* replaces was missing `'secondary'`, so a spec-valid badge was a type
183+
* error here and was rejected outright by `objectui validate`.
184+
*/
185+
badgeVariant?: NonNullable<SpecObjectNavItem['badgeVariant']>;
176186

177-
/** Whether group is expanded by default (for type: 'group') */
187+
/**
188+
* Whether a `type: 'group'` item starts expanded. This is the spec's
189+
* field name and the one to author against.
190+
*/
191+
expanded?: boolean;
192+
193+
/**
194+
* @deprecated Legacy objectui spelling of {@link expanded}. `NavigationRenderer`
195+
* honours whichever is set (`expanded` wins), so existing app metadata keeps
196+
* working; new metadata should use `expanded`.
197+
*/
178198
defaultOpen?: boolean;
179199

180-
/** Whether this item is pinned */
200+
/**
201+
* Action payload for `type: 'action'` items. Without it the item names an
202+
* action it cannot invoke — and before this was declared, `objectui validate`
203+
* silently stripped it, so a broken action item validated clean.
204+
*/
205+
actionDef?: { actionName: string; params?: Record<string, unknown> };
206+
207+
/** Whether this item is pinned. objectui-only; the spec has no counterpart. */
181208
pinned?: boolean;
182209

183210
/** Sort order weight (lower = higher) */
@@ -204,6 +231,12 @@ export interface NavigationArea {
204231
/** Icon name (Lucide) */
205232
icon?: string;
206233

234+
/** Sort order weight among areas (lower first) — spec field (objectstack#4115). */
235+
order?: number;
236+
237+
/** Longer description of the area — spec field (objectstack#4115). */
238+
description?: string;
239+
207240
/** Navigation items within this area */
208241
navigation: NavigationItem[];
209242

packages/types/src/zod/app.zod.ts

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,14 @@ export const NavigationItemTypeSchema = z.enum([
3535
* Navigation Item Schema — unified model aligned with @objectstack/spec.
3636
*/
3737
export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => z.object({
38-
id: z.string().describe('Unique identifier'),
38+
// Declared optional so a bare `{ type: 'separator' }` — which the spec
39+
// accepts, and which carries no identity or text by definition — validates
40+
// here too (objectstack#4115). Every OTHER type still requires both; that is
41+
// re-imposed by the refinement below rather than by the field declarations,
42+
// because this is one flat shape and not the spec's discriminated union.
43+
id: z.string().optional().describe('Unique identifier'),
3944
type: NavigationItemTypeSchema.describe('Navigation item type'),
40-
label: z.string().describe('Display label'),
45+
label: z.string().optional().describe('Display label'),
4146
icon: z.string().optional().describe('Icon name (Lucide)'),
4247

4348
// Type-specific target fields
@@ -57,16 +62,46 @@ export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => z.object({
5762
// Grouping
5863
children: z.array(z.lazy(() => NavigationItemSchema)).optional().describe('Child items (type: group)'),
5964

65+
// Action payload (type: 'action'). Undeclared until objectstack#4115: the
66+
// schema strips unknown keys, so an action item validated clean while its
67+
// entire payload was thrown away.
68+
actionDef: z.object({
69+
actionName: z.string(),
70+
params: z.record(z.string(), z.unknown()).optional(),
71+
}).optional().describe('Action payload (type: action)'),
72+
6073
// Visibility & Permissions
6174
visible: z.union([z.boolean(), z.string()]).optional().describe('Visibility expression'),
6275
requiredPermissions: z.array(z.string()).optional().describe('Required permissions'),
76+
// Runtime capability gates. `NavigationRenderer` has always honoured these
77+
// and `NavigationItem` has always declared them — only this schema lagged,
78+
// so `objectui validate` silently dropped them (objectstack#4115).
79+
requiresObject: z.string().optional().describe('Object that must be registered for this entry to render'),
80+
requiresService: z.string().optional().describe('Kernel service that must be registered for this entry to render'),
6381

6482
// UX Enhancements
6583
badge: z.union([z.string(), z.number()]).optional().describe('Badge text or count'),
66-
badgeVariant: z.enum(['default', 'destructive', 'outline']).optional().describe('Badge variant'),
67-
defaultOpen: z.boolean().optional().describe('Group default expanded state'),
84+
badgeVariant: z.enum(['default', 'secondary', 'destructive', 'outline']).optional().describe('Badge variant'),
85+
expanded: z.boolean().optional().describe('Group default expanded state (spec field name)'),
86+
/** @deprecated legacy objectui spelling of `expanded`; the renderer honours either. */
87+
defaultOpen: z.boolean().optional().describe('Group default expanded state (legacy alias of `expanded`)'),
6888
pinned: z.boolean().optional().describe('Pinned item'),
6989
order: z.number().optional().describe('Sort order weight'),
90+
}).superRefine((item, ctx) => {
91+
// Identity and text are required for every real destination; only the
92+
// separator — a rule, not an entry — is exempt. Declaring the fields
93+
// optional above is what lets `{ type: 'separator' }` through, so without
94+
// this an id-less `type: 'object'` item would validate too.
95+
if (item.type === 'separator') return;
96+
for (const key of ['id', 'label'] as const) {
97+
if (typeof item[key] !== 'string' || item[key] === '') {
98+
ctx.addIssue({
99+
code: 'custom',
100+
path: [key],
101+
message: `\`${key}\` is required for navigation items of type '${item.type}'`,
102+
});
103+
}
104+
}
70105
}));
71106

72107
/**
@@ -76,6 +111,10 @@ export const NavigationAreaSchema = z.object({
76111
id: z.string().describe('Unique identifier'),
77112
label: z.string().describe('Display label'),
78113
icon: z.string().optional().describe('Icon name (Lucide)'),
114+
// Spec fields this schema used to drop on the floor (objectstack#4115):
115+
// an area authored with a sort weight or a description lost both.
116+
order: z.number().optional().describe('Sort order weight among areas'),
117+
description: z.string().optional().describe('Longer description of the area'),
79118
navigation: z.array(NavigationItemSchema).describe('Navigation items within area'),
80119
visible: z.union([z.boolean(), z.string()]).optional().describe('Visibility expression'),
81120
requiredPermissions: z.array(z.string()).optional().describe('Required permissions'),

0 commit comments

Comments
 (0)