diff --git a/.changeset/13075-nav-visible-envelope.md b/.changeset/13075-nav-visible-envelope.md new file mode 100644 index 0000000000..abf8ccad57 --- /dev/null +++ b/.changeset/13075-nav-visible-envelope.md @@ -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. diff --git a/packages/app-shell/src/hooks/useNavMemberGates.ts b/packages/app-shell/src/hooks/useNavMemberGates.ts index 1a15dafef1..8040a714fc 100644 --- a/packages/app-shell/src/hooks/useNavMemberGates.ts +++ b/packages/app-shell/src/hooks/useNavMemberGates.ts @@ -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], ); diff --git a/packages/app-shell/src/layout/__tests__/navItemVisibleEvaluated.test.tsx b/packages/app-shell/src/layout/__tests__/navItemVisibleEvaluated.test.tsx index 2e8ea70f01..09dec6a0f8 100644 --- a/packages/app-shell/src/layout/__tests__/navItemVisibleEvaluated.test.tsx +++ b/packages/app-shell/src/layout/__tests__/navItemVisibleEvaluated.test.tsx @@ -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 ( @@ -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); diff --git a/packages/app-shell/src/views/nav-menu-renderer.tsx b/packages/app-shell/src/views/nav-menu-renderer.tsx index 086c1e1450..88ee6b899d 100644 --- a/packages/app-shell/src/views/nav-menu-renderer.tsx +++ b/packages/app-shell/src/views/nav-menu-renderer.tsx @@ -185,7 +185,7 @@ export const NavMenuRenderer: React.FC = ({ const { evaluator } = useExpressionContext(); const evalVis = useCallback( - (expr: string | boolean | undefined) => evaluateNavigationVisibility(expr, evaluator), + (expr: NavigationItem['visible']) => evaluateNavigationVisibility(expr, evaluator), [evaluator], ); diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index c268da2c38..6dc9a2c1b0 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -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; /** diff --git a/packages/types/src/__tests__/navigation-spec-parity.test.ts b/packages/types/src/__tests__/navigation-spec-parity.test.ts index 27f3d49a2b..c4c7986ccc 100644 --- a/packages/types/src/__tests__/navigation-spec-parity.test.ts +++ b/packages/types/src/__tests__/navigation-spec-parity.test.ts @@ -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]> = [ + ['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]>))( + '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) // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/types/src/app.ts b/packages/types/src/app.ts index c55466fcca..bfef32fb0c 100644 --- a/packages/types/src/app.ts +++ b/packages/types/src/app.ts @@ -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'; @@ -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[]; diff --git a/packages/types/src/zod/app.zod.ts b/packages/types/src/zod/app.zod.ts index a8df21a2b8..9e64d7e864 100644 --- a/packages/types/src/zod/app.zod.ts +++ b/packages/types/src/zod/app.zod.ts @@ -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'; @@ -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}\``); @@ -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 @@ -435,7 +482,10 @@ export const NavigationItemSchema: z.ZodType = 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.) *