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
20 changes: 20 additions & 0 deletions .changeset/13075-nav-visible-envelope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@object-ui/types': minor
'@object-ui/layout': minor
'@object-ui/app-shell': patch
---

A navigation item's `visible` now takes `@objectstack/spec`'s `EvaluatedExpressionInputSchema` by reference, beside the boolean it already took (objectui#13075).

The spec types every navigation entry's `visible` as an evaluated expression input: a non-blank CEL string, or the `{ dialect, source }` envelope the spec's own parse produces. objectui's `NavigationItemSchema` typed it as `boolean | string`, so the two disagreed both ways. This changes what `NavigationItemSchema` (and so `objectui validate`) accepts:

- **Widened: the envelope is accepted.** `visible: { dialect: 'cel', source: "'org_admin' in current_user.positions" }` used to be refused. It parses now and is kept as written. The TypeScript type `NavigationItem['visible']` is `boolean | EvaluatedExpressionInput` (the spec's input type), so a typed literal can carry the envelope too.
- **Narrowed (breaking): a blank string is refused.** `visible: ''` and a whitespace-only string used to pass. They are refused now with the spec's own code (`invalid_union` at `visible`) and sentence. A blank `source` inside an envelope is refused at `visible.source`, as the spec refuses it. To show an entry to everyone, omit `visible`.

Unchanged: a non-blank string is still accepted and kept as a string (the validator does not rewrite it into an envelope), and `visible: true` / `false` are still accepted. The boolean is the one stated divergence from the spec on this key.

Code that reads `NavigationItem['visible']` and passes it on as `string | boolean` no longer compiles: widen that parameter to `NavigationItem['visible']`.

`@object-ui/layout` (minor): the exported `VisibilityEvaluator` type now takes `NavigationItem['visible']` instead of `string | boolean | undefined`, so an evaluator receives the envelope as authored. An evaluator a host passes in whose parameter is the old, narrower type no longer type-checks; widen it the same way. The runtime is unchanged: `NavigationRenderer` already handed the value through.

`@object-ui/app-shell` (patch): the shell's navigation gates (`useNavMemberGates` and the `nav:menu` block) take the widened type internally. No exported type changes, and the runtime is unchanged: `evaluateNavigationVisibility` already evaluated the envelope.
2 changes: 1 addition & 1 deletion packages/app-shell/src/hooks/useNavMemberGates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export interface NavMemberGates {
export function useNavMemberGates(): NavMemberGates {
const { evaluator, app } = useExpressionContext();
const evalVis = useCallback(
(expr: string | boolean | undefined) => evaluateNavigationVisibility(expr, evaluator),
(expr: NavigationItem['visible']) => evaluateNavigationVisibility(expr, evaluator),
[evaluator],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const OUTSIDER = { id: 'u_outsider', name: 'Otto', email: 'otto@example.com', po
function NavUnderProvider({ items, enablePinning }: { items: NavigationItem[]; enablePinning?: boolean }) {
const { evaluator } = useExpressionContext();
const evalVis = React.useCallback(
(expr: string | boolean | undefined) => evaluateVisibility(expr, evaluator),
(expr: NavigationItem['visible']) => evaluateVisibility(expr, evaluator),
[evaluator],
);
return (
Expand Down Expand Up @@ -276,7 +276,7 @@ describe('nav item `visible` is evaluated per item by the shell (objectui#10119)
children: [{ id: 'nav_secret', type: 'object', objectName: 'demo_thing', label: 'Secret', visible: false }],
},
];
const literal = (expr: string | boolean | undefined) => expr !== false && expr !== 'false';
const literal = (expr: NavigationItem['visible']) => expr !== false && expr !== 'false';

expect(hasVisibleNavigationItems(hiddenAncestor, { evaluateVisibility: literal })).toBe(false);
expect(hasVisibleNavigationItems(allChildrenHidden, { evaluateVisibility: literal })).toBe(false);
Expand Down
2 changes: 1 addition & 1 deletion packages/app-shell/src/views/nav-menu-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export const NavMenuRenderer: React.FC<NavMenuRendererProps> = ({

const { evaluator } = useExpressionContext();
const evalVis = useCallback(
(expr: string | boolean | undefined) => evaluateNavigationVisibility(expr, evaluator),
(expr: NavigationItem['visible']) => evaluateNavigationVisibility(expr, evaluator),
[evaluator],
);

Expand Down
6 changes: 5 additions & 1 deletion packages/layout/src/NavigationRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,13 @@ const HIDDEN_ICON_LABEL_NOT_FOCUSABLE =
* Callback to evaluate a visibility expression.
* Return `true` if the item should be visible.
* When not provided, all items default to visible.
*
* The parameter is the item's own `visible` type, so the spec's
* `{ dialect, source }` envelope reaches the evaluator as authored
* (objectui#13075).
*/
export type VisibilityEvaluator = (
expression: string | boolean | undefined,
expression: NavigationItem['visible'],
) => boolean;

/**
Expand Down
72 changes: 72 additions & 0 deletions packages/types/src/__tests__/navigation-spec-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,78 @@ describe('referencing the spec NavigationItemSchema would reject metadata object
});


// ─────────────────────────────────────────────────────────────────────────────
// `visible`: the spec's expression input, both ways (objectui#13075)
// ─────────────────────────────────────────────────────────────────────────────

/**
* The spec types every nav branch's `visible` as `EvaluatedExpressionInputSchema`:
* a non-blank predicate string, or the `{ dialect, source }` envelope its own
* parse produces. The mirror's slot was `boolean | string`, so it refused the
* envelope and accepted a blank string — a divergence both ways that no row
* above recorded. The mirror now takes the spec's schema by reference beside
* its boolean arm, so the boolean (the `visible: boolean` row above) is the one
* divergence left on this key.
*
* Each verdict is read off BOTH schemas, on an ordinary entry and on a `doc`
* entry (which the spec's `doc` arm judges), so an agreement cannot pass
* because one side refuses everything.
*/
describe('objectui#13075 — `visible` takes the spec\'s expression input', () => {
const ENTRIES: Array<[string, Record<string, unknown>]> = [
['a url entry', { id: 'nav_ai', type: 'url', label: 'AI', url: '/ai' }],
['a doc entry', { id: 'nav_help', type: 'doc', book: 'crm_manual' }],
];
/** An envelope that reads no `data`, so the spec's data-source rule does not apply. */
const ENVELOPE = { dialect: 'cel', source: "'org_admin' in current_user.positions" };

it.each(ENTRIES)('the `{ dialect, source }` envelope parses on both (%s), and the mirror keeps it as written', (_name, entry) => {
const input = { ...entry, visible: ENVELOPE };
expect(SpecNavigationItemSchema.safeParse(input).success).toBe(true);
expect(keep(input)).toEqual(input);
});

it.each(ENTRIES.flatMap(([name, entry]) => [
[name, '\'\'', { ...entry, visible: '' }],
[name, 'whitespace', { ...entry, visible: ' ' }],
] as Array<[string, string, Record<string, unknown>]>))(
'a blank string is refused by both (%s, %s), with the spec\'s code and sentence',
(_name, _blank, input) => {
const spec = SpecNavigationItemSchema.safeParse(input);
const mirror = NavigationItemSchema.safeParse(input);
expect(refusal(spec)).toEqual(['invalid_union @ visible']);
expect(refusal(mirror)).toEqual(['invalid_union @ visible']);
expect(!mirror.success && mirror.error.issues[0].message).toBe(!spec.success && spec.error.issues[0].message);
},
);

it('a blank `source` inside the envelope is refused by both, once, at `source`', () => {
const input = { id: 'nav_ai', type: 'url', label: 'AI', url: '/ai', visible: { dialect: 'cel', source: ' ' } };
expect(refusal(SpecNavigationItemSchema.safeParse(input))).toEqual(['custom @ visible.source']);
expect(refusal(NavigationItemSchema.safeParse(input))).toEqual(['custom @ visible.source']);
});

it('CONTROL — a bare non-blank string parses on both, and the mirror does not rewrite it into an envelope', () => {
const input = { id: 'nav_ai', type: 'url', label: 'AI', url: '/ai', visible: "'org_admin' in current_user.positions" };
expect(SpecNavigationItemSchema.safeParse(input).success).toBe(true);
expect(keep(input)).toEqual(input);
});

it('CONTROL — a boolean still parses on the mirror and stays the one divergence (the `visible: boolean` row)', () => {
for (const visible of [true, false]) {
const input = { id: 'nav_ai', type: 'url', label: 'AI', url: '/ai', visible };
expect(keep(input)).toEqual(input);
expect(SpecNavigationItemSchema.safeParse(input).success).toBe(false);
}
});

it('the TypeScript face admits the envelope (compile-time)', () => {
const entry: NavigationItem = { id: 'nav_ai', type: 'url', label: 'AI', url: '/ai', visible: { dialect: 'cel', source: 'true' } };
expect(keep(entry)).toEqual(entry);
});
});


// ─────────────────────────────────────────────────────────────────────────────
// The type vocabulary and the `doc` entry (objectui#11197)
// ─────────────────────────────────────────────────────────────────────────────
Expand Down
17 changes: 12 additions & 5 deletions packages/types/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import type {
DocNavItem as SpecDocNavItem,
App as SpecApp,
} from '@objectstack/spec/ui';
import type { EvaluatedExpressionInput } from '@objectstack/spec/shared';
import type { BaseSchema } from './base.js';
import type { z } from 'zod';
import type { APP_SPEC_EXCLUDED, AppContextSelectorSchema } from './zod/app.zod.js';
Expand Down Expand Up @@ -276,16 +277,22 @@ export interface NavigationEntryItem {
// -- Visibility & Permissions --

/**
* Visibility expression — boolean or expression string e.g. "${user.role === 'admin'}".
* Visibility predicate — a CEL string, the spec's `{ dialect, source }`
* envelope, or a boolean.
*
* 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,
* The expression half is the spec's input type, `EvaluatedExpressionInput`,
* by reference (objectui#13075) — the same type the zod twin
* (`NavigationItemSchema`) takes by reference. Until then this was
* `boolean | string`, which left out the envelope the spec's own parse
* produces.
*
* The `boolean` arm is NOT derived (#3177): the spec admits it at neither
* tier. `NavigationRenderer`'s `evalVis(item.visible)` honours the boolean,
* and `menuItemToNavigationItem` produces one when mapping legacy
* `AppMenuItem.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;
visible?: boolean | EvaluatedExpressionInput;

/** Required permissions to see/access this item */
requiredPermissions?: string[];
Expand Down
54 changes: 52 additions & 2 deletions packages/types/src/zod/app.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ import {
// to this paragraph.
objectNavTargetExclusivity,
} from '@objectstack/spec/ui';
import {
EvaluatedExpressionInputSchema as SpecEvaluatedExpressionInputSchema,
EvaluatedExpressionSchema as SpecEvaluatedExpressionSchema,
} from '@objectstack/spec/shared';
import { BaseSchema, specFieldsExcept } from './base.zod.js';
import { aliasKeyRefusal, retirementTombstone } from './tombstone.zod.js';
import type { AppMenuItem, NavigationItemType } from '../app.js';
Expand Down Expand Up @@ -196,6 +200,49 @@ function refuseEmptyNavLabel(item: { type: string; label?: unknown }, ctx: z.Ref
});
}

/**
* A navigation entry's `visible`: the spec's `EvaluatedExpressionInputSchema`,
* by reference, beside the one pinned objectui arm, `boolean` (objectui#13075).
*
* The spec types every nav branch's `visible` as `EvaluatedExpressionInputSchema`:
* a non-blank predicate string, or the `{ dialect, source }` envelope its own
* parse produces (`defineApp` returns the parsed app, so a string authored there
* comes back as the envelope). Until objectui#13075 this slot was
* `boolean | string`: it refused the envelope the spec accepts, and accepted a
* blank string the spec refuses. Both are the spec's verdicts now.
*
* Spelled the way `visibleWhen`'s wire is in `./base.zod.ts`, and for the same
* reason: the accept set is the spec's input type (a string, or the spec's own
* `EvaluatedExpressionSchema` read by reference), and every verdict on a
* non-boolean value is the spec's — the value is handed to
* `EvaluatedExpressionInputSchema` and its issues are the refusal, so a blank
* string is refused with the spec's code and sentence. The judge runs only on a
* value an arm already took, so one fault is never reported twice. What is NOT taken is the
* spec's transform, which rewrites a bare string into `{ dialect: 'cel', source }`:
* this mirror writes nothing into an author's document (decision batches #69 /
* #90), so a string stays a string.
*
* `boolean` is the stated divergence: the spec admits it at neither tier, and
* `menuItemToNavigationItem` manufactures one when it inverts legacy
* `AppMenuItem.hidden`. `../__tests__/navigation-spec-parity.test.ts` pins all
* three rows: the envelope and the blank string agree with the spec, the boolean
* does not.
*/
function specVisibleRefusal(visible: unknown): z.core.$ZodIssue[] {
const verdict = stripImportedDefaults(SpecEvaluatedExpressionInputSchema).safeParse(visible);
return verdict.success ? [] : verdict.error.issues;
}
const NavigationVisibleSchema = z
.union([z.boolean(), z.string(), stripImportedDefaults(SpecEvaluatedExpressionSchema)], {
// A value no arm takes (`{ dialect: 'cel' }`, a number) is refused with the
// spec's own sentence for it, not zod's generic one.
error: (issue) => specVisibleRefusal(issue.input)[0]?.message,
})
.superRefine((visible, ctx) => {
if (typeof visible === 'boolean') return;
for (const issue of specVisibleRefusal(visible)) ctx.addIssue({ ...issue, message: issue.message });
}, { when: (payload) => payload.issues.length === 0 });

/** `a`, `b` and `c` — the separator refusal's list of what a separator may carry. */
function codeList(keys: readonly string[]): string {
const quoted = keys.map((key) => `\`${key}\``);
Expand Down Expand Up @@ -253,7 +300,7 @@ const NavigationItemObject = z.object({
}).optional().describe('Action payload (type: action)'),

// Visibility & Permissions
visible: z.union([z.boolean(), z.string()]).optional().describe('Visibility expression'),
visible: NavigationVisibleSchema.optional().describe("Visibility predicate: a non-blank CEL string or the spec's `{ dialect, source }` envelope (`@objectstack/spec` `EvaluatedExpressionInputSchema`), or a boolean"),
// objectui#12971: the spec's shared nav-item base declares one API data source
// (objectstack#24332, `ElementApiDataSourceSchema` by reference), whose bound
// value an entry's `visible` reads as `data`. Taken by reference for the
Expand Down Expand Up @@ -435,7 +482,10 @@ export const NavigationItemSchema: z.ZodType<any> = z.lazy(() => NavigationItemA
* against spec 17.2.0, referencing the spec's schema would make `objectui
* validate` REJECT metadata this renderer accepts today: `pinned` and
* `defaultOpen` fail `unrecognized_keys`, `visible: boolean` fails
* `invalid_union`, and a one-character `id` fails `too_small`. Pinned by
* `invalid_union`, and a one-character `id` fails `too_small`. (`visible`'s
* other arms are the spec's own `EvaluatedExpressionInputSchema` since
* objectui#13075, so its envelope and its blank-string refusal agree; only
* the boolean parts.) Pinned by
* `__tests__/navigation-spec-parity.test.ts`. (A separator carrying `label`
* was a fifth until objectui#10867, which made the mirror refuse it too.)
*
Expand Down
Loading