Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/layout/src/NavigationRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,7 @@ function NavigationItemRenderer({
// group — otherwise an auto-collapsed group hides the active item
// and the user loses orientation.
const explicitOpen = (() => {
const expanded = (item as any).expanded;
if (typeof expanded === 'boolean') return expanded;
if (typeof item.expanded === 'boolean') return item.expanded;
if (typeof item.defaultOpen === 'boolean') return item.defaultOpen;
return undefined;
})();
Expand Down
115 changes: 115 additions & 0 deletions packages/types/src/__tests__/navigation-spec-parity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* Navigation ↔ `@objectstack/spec` drift guard (objectstack#4115).
*
* `NavigationItemSchema` is the only navigation schema with a *runtime*
* consumer: `@object-ui/cli`'s published `objectui validate` command parses
* metadata through `AnyComponentSchema`, which reaches it via `AppSchema`.
* Renderers read plain objects and never parse, so every gap below was a CLI
* defect — and a strip-mode schema fails silently, which is why they survived.
*
* The gaps this pins, all measured against spec 17.0.0-rc.0:
*
* - `requiresObject` / `requiresService` — capability gates that
* `NavigationItem` has always declared and `NavigationRenderer` has always
* honoured. Only the schema lagged, so `objectui validate` dropped them.
* - `actionDef` — an action item's entire payload. Stripped, so an item that
* could never invoke anything validated clean.
* - `expanded` — the spec's spelling of group expansion. The renderer already
* read it (behind an `as any`, because the type did not declare it).
* - `badgeVariant: 'secondary'` — spec-valid, hard-rejected here.
* - `{ type: 'separator' }` — spec-valid, rejected for missing id/label.
*
* Deliberately NOT modelled: the spec expresses navigation as a discriminated
* union of nine variants, each with its target field required and a
* `superRefine` exclusivity rule. objectui keeps one flat, all-optional shape,
* so it accepts items the spec would reject (e.g. `type: 'object'` with no
* `objectName`). Converging on the union is a breaking change for every
* consumer that reads fields off `NavigationItem` without narrowing — tracked
* separately, not smuggled in here.
*/

import { describe, it, expect } from 'vitest';
import { NavigationItemSchema, NavigationAreaSchema } from '../zod/app.zod.js';

/** Parse and return the surviving object, so "accepted" cannot hide a strip. */
function keep(input: unknown): Record<string, unknown> | null {
const r = NavigationItemSchema.safeParse(input);
return r.success ? (r.data as Record<string, unknown>) : null;
}

describe('NavigationItemSchema keeps the spec vocabulary it used to drop', () => {
it('keeps the capability gates the renderer actually reads', () => {
const kept = keep({ id: 'apps', type: 'object', label: 'Apps', objectName: 'sys_app', requiresObject: 'sys_app' });
expect(kept).not.toBeNull();
expect(kept!.requiresObject).toBe('sys_app');

const svc = keep({ id: 'ai', type: 'url', label: 'AI', url: '/ai', requiresService: 'ai' });
expect(svc!.requiresService).toBe('ai');
});

it('keeps an action item\'s payload instead of validating an empty shell', () => {
const kept = keep({ id: 'run', type: 'action', label: 'Run', actionDef: { actionName: 'sync_now' } });
expect(kept).not.toBeNull();
expect(kept!.actionDef).toEqual({ actionName: 'sync_now' });
});

it('keeps the spec spelling of group expansion', () => {
const kept = keep({ id: 'grp', type: 'group', label: 'Group', children: [], expanded: true });
expect(kept!.expanded).toBe(true);
});

it('still accepts the legacy defaultOpen spelling', () => {
const kept = keep({ id: 'grp', type: 'group', label: 'Group', children: [], defaultOpen: true });
expect(kept!.defaultOpen).toBe(true);
});

it('accepts every spec badge variant, including secondary', () => {
for (const variant of ['default', 'secondary', 'destructive', 'outline']) {
const kept = keep({ id: 'x', type: 'url', label: 'X', url: '/x', badgeVariant: variant });
expect(kept, `badgeVariant '${variant}' was rejected`).not.toBeNull();
expect(kept!.badgeVariant).toBe(variant);
}
});

it('accepts a bare separator, which carries no identity or text by definition', () => {
expect(keep({ type: 'separator' })).not.toBeNull();
});

it('exempts ONLY the separator from needing id + label', () => {
// The separator exemption is implemented by declaring `id`/`label`
// optional, so without the refinement that re-imposes them this would
// also wave through an unidentifiable destination.
expect(keep({ type: 'object', label: 'No id', objectName: 'contact' })).toBeNull();
expect(keep({ id: 'no_label', type: 'object', objectName: 'contact' })).toBeNull();
expect(keep({ id: 'ok', type: 'object', label: 'Ok', objectName: 'contact' })).not.toBeNull();
});

it('still rejects an unknown item type', () => {
// The permissiveness above is scoped to spec vocabulary — it must not
// become "anything goes".
expect(keep({ id: 'x', type: 'wormhole', label: 'X' })).toBeNull();
});
});

describe('NavigationAreaSchema keeps the spec fields it used to drop', () => {
it('keeps order and description', () => {
const r = NavigationAreaSchema.safeParse({
id: 'sales',
label: 'Sales',
order: 2,
description: 'Pipeline and accounts',
navigation: [],
});
expect(r.success).toBe(true);
expect(r.success && r.data.order).toBe(2);
expect(r.success && r.data.description).toBe('Pipeline and accounts');
});
});
41 changes: 37 additions & 4 deletions packages/types/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
* configurations should use `NavigationItem` and the `navigation` / `areas` fields.
*/

// The spec's own `NavigationItem` erases to `any` (its schema is declared
// `z.ZodType<any>` 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';
import type { BaseSchema } from './base';

// ============================================================================
Expand Down Expand Up @@ -171,13 +176,35 @@ export interface NavigationItem {
/** Badge text or count */
badge?: string | number;

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

/** Whether group is expanded by default (for type: 'group') */
/**
* Whether a `type: 'group'` item starts expanded. This is the spec's
* field name and the one to author against.
*/
expanded?: boolean;

/**
* @deprecated Legacy objectui spelling of {@link expanded}. `NavigationRenderer`
* honours whichever is set (`expanded` wins), so existing app metadata keeps
* working; new metadata should use `expanded`.
*/
defaultOpen?: boolean;

/** Whether this item is pinned */
/**
* 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.
*/
actionDef?: { actionName: string; params?: Record<string, unknown> };

/** Whether this item is pinned. objectui-only; the spec has no counterpart. */
pinned?: boolean;

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

/** Sort order weight among areas (lower first) — spec field (objectstack#4115). */
order?: number;

/** Longer description of the area — spec field (objectstack#4115). */
description?: string;

/** Navigation items within this area */
navigation: NavigationItem[];

Expand Down
47 changes: 43 additions & 4 deletions packages/types/src/zod/app.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@ export const NavigationItemTypeSchema = z.enum([
* Navigation Item Schema — unified model aligned with @objectstack/spec.
*/
export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => z.object({
id: z.string().describe('Unique identifier'),
// Declared optional so a bare `{ type: 'separator' }` — which the spec
// accepts, and which carries no identity or text by definition — validates
// here too (objectstack#4115). Every OTHER type still requires both; that is
// re-imposed by the refinement below rather than by the field declarations,
// because this is one flat shape and not the spec's discriminated union.
id: z.string().optional().describe('Unique identifier'),
type: NavigationItemTypeSchema.describe('Navigation item type'),
label: z.string().describe('Display label'),
label: z.string().optional().describe('Display label'),
icon: z.string().optional().describe('Icon name (Lucide)'),

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

// Action payload (type: 'action'). Undeclared until objectstack#4115: the
// schema strips unknown keys, so an action item validated clean while its
// entire payload was thrown away.
actionDef: z.object({
actionName: z.string(),
params: z.record(z.string(), z.unknown()).optional(),
}).optional().describe('Action payload (type: action)'),

// Visibility & Permissions
visible: z.union([z.boolean(), z.string()]).optional().describe('Visibility expression'),
requiredPermissions: z.array(z.string()).optional().describe('Required permissions'),
// Runtime capability gates. `NavigationRenderer` has always honoured these
// and `NavigationItem` has always declared them — only this schema lagged,
// so `objectui validate` silently dropped them (objectstack#4115).
requiresObject: z.string().optional().describe('Object that must be registered for this entry to render'),
requiresService: z.string().optional().describe('Kernel service that must be registered for this entry to render'),

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

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