diff --git a/.changeset/dead-surface-deletions-batch3-4328.md b/.changeset/dead-surface-deletions-batch3-4328.md new file mode 100644 index 0000000000..c28fa5dd21 --- /dev/null +++ b/.changeset/dead-surface-deletions-batch3-4328.md @@ -0,0 +1,40 @@ +--- +'@object-ui/types': minor +'@object-ui/core': minor +'@object-ui/react': minor +'@object-ui/data-objectstack': patch +--- + +Retire four zero-consumer declared surfaces (dead-surface sweep batch 3, #4328). Each was +measured as declared-but-never-read at the branch point, and each is removed rather than +left as an authoring surface whose values nothing acts on. + +Breaking for anyone who typed against the removed declarations, marked `minor` per this +repository's version-alignment convention (the major tracks `@objectstack`, never an +API-break count): + +- `@object-ui/core` no longer exports `mergeViewsIntoObjects`. It was a second copy left + behind by the move of that step to the provider layer, and it had drifted: it ignored a + view container's default `list` and keyed views by the authored bare key instead of the + composer's `.` identity. The live implementation — `MetadataProvider`'s, in + `@object-ui/app-shell` — is unchanged and remains the only one. (#3775) +- `@object-ui/types`' `RoleDefinition` no longer declares `permissions`. A role's grants + live in `ObjectPermissionConfig.roles`, keyed by object; that is the only home any + consumer reads (`resolveRoles` walks `inherits` and matches on `name`). The removed + field was *required*, so five fixtures across three packages had been declaring an empty + array for a value nothing would ever look at. Role-attached grants are now a compile + error rather than silently ignored data. (#4288) +- `@object-ui/react`'s `RecordContextValue` no longer declares `loading` / `error`. Both + had zero producers and zero consumers — no host passed them, no `record:*` renderer read + them — and only the provider's memo dependency list still named them. Record-level + loading and error state stays where it is actually expressed: each renderer's own data + source. (#3773) + +No behaviour change, no request-count change: + +- `@object-ui/data-objectstack` drops five `metadataCache.invalidate('views:')` + calls across `updateViewConfig` / `createView` / `updateView` / `deleteView`. No read + path has ever populated that key — `listViews` fetches directly, uncached — so all five + were permanent no-ops. The invalidations of the keys that do have readers + (`view::` for `getView`, `view-overrides:` for + `listViewOverrides`) are untouched and now pinned. (#3778) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c97a087b6c..6dec0e32fc 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -31,7 +31,6 @@ export * from './data-scope/index.js'; export * from './errors/index.js'; export * from './utils/debug.js'; export * from './utils/debug-collector.js'; -export * from './utils/merge-views-into-objects.js'; export * from './utils/freeze-schema.js'; export * from './protocols/index.js'; export * from './styling/scoped-styles.js'; diff --git a/packages/core/src/utils/merge-views-into-objects.ts b/packages/core/src/utils/merge-views-into-objects.ts deleted file mode 100644 index 359ccd5449..0000000000 --- a/packages/core/src/utils/merge-views-into-objects.ts +++ /dev/null @@ -1,36 +0,0 @@ -/** - * 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. - */ - -/** - * Adapter: merge stack-level views into object definitions. - * - * Views are defined at the stack level (views[].listViews) but the runtime - * expects listViews on each object definition. This bridges the gap until - * the runtime/provider layer handles it natively. - * - * @param objects - Object definitions from composed stack - * @param views - View definitions containing listViews keyed by object name - * @returns Objects with listViews merged in from matching views - */ -export function mergeViewsIntoObjects(objects: any[], views: any[]): any[] { - const viewsByObject: Record> = {}; - for (const view of views) { - if (!view.listViews) continue; - for (const [viewName, listView] of Object.entries(view.listViews as Record)) { - const objectName = listView?.data?.object; - if (!objectName) continue; - if (!viewsByObject[objectName]) viewsByObject[objectName] = {}; - viewsByObject[objectName][viewName] = listView; - } - } - return objects.map((obj: any) => { - const v = viewsByObject[obj.name]; - if (!v) return obj; - return { ...obj, listViews: { ...(obj.listViews || {}), ...v } }; - }); -} diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index e26cfd3eec..6c94f01335 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -3019,7 +3019,6 @@ export class ObjectStackAdapter implements DataSource { this.metadataCache.invalidate?.(cacheKey); // Also invalidate the batch override map so listViewOverrides re-fetches this.metadataCache.invalidate?.(`view-overrides:${objectName}`); - this.metadataCache.invalidate?.(`views:${objectName}`); if (result && result.item) return result.item; return result ?? undefined; } @@ -3180,7 +3179,6 @@ export class ObjectStackAdapter implements DataSource { data: spec?.data || { provider: 'object', object: objectName }, }; const result: any = await this.client.meta.saveItem('view', name, fullSpec); - this.metadataCache.invalidate?.(`views:${objectName}`); if (result && result.item) return result.item; return fullSpec; } @@ -3240,7 +3238,6 @@ export class ObjectStackAdapter implements DataSource { if (draft) { const mergedDraft = mergeViewPatch(draft, partial, viewName, objectName); await metaClient.save('view', viewName, mergedDraft, { mode: 'draft' }); - this.metadataCache.invalidate?.(`views:${objectName}`); this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`); return mergedDraft; } @@ -3271,7 +3268,6 @@ export class ObjectStackAdapter implements DataSource { const merged = mergeViewPatch(current, partial, viewName, objectName); const result: any = await this.client.meta.saveItem('view', viewName, merged); - this.metadataCache.invalidate?.(`views:${objectName}`); this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`); if (result && result.item) return result.item; return merged; @@ -3288,7 +3284,6 @@ export class ObjectStackAdapter implements DataSource { ): Promise<{ deleted: boolean }> { await this.connect(); const result: any = await this.client.meta.deleteItem('view', viewName); - this.metadataCache.invalidate?.(`views:${objectName}`); this.metadataCache.invalidate?.(`view:${objectName}:${viewName}`); return { deleted: !!(result?.deleted ?? result?.reset ?? true) }; } diff --git a/packages/data-objectstack/src/viewCacheInvalidation.pin.test.ts b/packages/data-objectstack/src/viewCacheInvalidation.pin.test.ts new file mode 100644 index 0000000000..3550d08383 --- /dev/null +++ b/packages/data-objectstack/src/viewCacheInvalidation.pin.test.ts @@ -0,0 +1,198 @@ +/** + * 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. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackAdapter } from './index'; + +/** + * View metadata cache keys — invalidation matches the read keys (objectui#3778). + * + * `ObjectStackAdapter` caches exactly two view-shaped reads: + * + * | reader | cache key | + * |---------------------|------------------------------| + * | `getView` | `view:{object}:{viewId}` | + * | `listViewOverrides` | `view-overrides:{object}` | + * + * `listViews` is **not** one of them: it fetches `meta.getItems('view')` + * directly on every call, with no `metadataCache.get` wrapper. Five write + * paths nevertheless used to invalidate a `views:{object}` key that no read + * path has ever populated — five permanent no-ops. They are gone; these pins + * keep both halves of that honest: + * + * 1. the surviving invalidations still name the keys that DO have readers, so + * the deletion cannot be mistaken for "cache invalidation was dropped"; and + * 2. `listViews` keeps its uncached behavior — the deletion changed no + * request count, which is what makes it a pure dead-code removal rather + * than a caching change. (Whether `listViews` SHOULD be cached is a + * separate product question, deliberately not settled here.) + */ + +interface Harness { + ds: any; + /** Every key passed to `metadataCache.invalidate`, in order. */ + invalidated: string[]; + /** Every key passed to `metadataCache.get`, in order. */ + cacheReads: string[]; + getItems: ReturnType; + saveItem: ReturnType; + deleteItem: ReturnType; +} + +/** + * Adapter with a recording metadata cache. + * + * @param opts.items what `client.meta.getItems('view')` returns + * @param opts.published what `client.meta.getItem` answers (body, or an + * Error to throw — a 404 means "no published overlay") + * @param opts.draft body served at `GET /meta/view/:name?state=draft` + * (`null` → 404, i.e. nothing pending) + */ +function makeDS(opts: { + items?: any[]; + published?: any | Error; + draft?: any | null; +} = {}): Harness { + const invalidated: string[] = []; + const cacheReads: string[] = []; + + const fetchImpl = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); + if (url.includes('/meta/view/')) { + if ((init?.method ?? 'GET') === 'PUT') return json({ success: true, version: 2 }); + if (url.includes('state=draft')) { + if (opts.draft == null) return json({ error: 'not found' }, 404); + return json({ type: 'view', name: opts.draft.name, item: opts.draft }); + } + return json({ error: 'not found' }, 404); + } + return json({ success: true, data: { capabilities: {}, routes: {} } }); + }); + + const ds: any = new ObjectStackAdapter({ baseUrl: 'http://test.local', fetch: fetchImpl }); + ds.connected = true; + ds.connectionState = 'connected'; + + // Recording stand-in for the real MetadataCache: `get` records the key and + // always misses (runs the loader), `invalidate` records the key. + ds.metadataCache = { + get: async (key: string, loader: () => Promise) => { + cacheReads.push(key); + return loader(); + }, + invalidate: (key: string) => { + invalidated.push(key); + }, + getCachedSync: () => undefined, + getStats: () => ({}), + }; + + const getItems = vi.fn(async () => ({ items: opts.items ?? [] })); + const saveItem = vi.fn(async () => ({ success: true })); + const deleteItem = vi.fn(async () => ({ deleted: true })); + ds.client = { + meta: { + getItems, + saveItem, + deleteItem, + getItem: vi.fn(async () => { + if (opts.published instanceof Error) throw opts.published; + return { item: opts.published ?? { name: 'v1', object: 'account' } }; + }), + }, + }; + + return { ds, invalidated, cacheReads, getItems, saveItem, deleteItem }; +} + +/** A published read that 404s, decorated the way the SDK client decorates. */ +function notFound(): Error { + return Object.assign(new Error('Metadata item not found'), { httpStatus: 404 }); +} + +const VIEW = { + name: 'account.all', + object: 'account', + viewKind: 'list', + label: 'All Accounts', + config: { type: 'grid', data: { object: 'account' } }, +}; + +describe('view metadata cache — invalidation names only keys with readers (#3778)', () => { + it('listViews reads the transport on every call and consults no cache key', async () => { + const { ds, getItems, cacheReads } = makeDS({ items: [VIEW] }); + + await ds.listViews('account'); + await ds.listViews('account'); + + // Uncached: two calls, two round trips. This is the behavior the removed + // `views:{object}` invalidations pretended to manage. + expect(getItems).toHaveBeenCalledTimes(2); + expect(getItems).toHaveBeenCalledWith('view'); + expect(cacheReads).toEqual([]); + }); + + it('updateViewConfig invalidates exactly the two keys that have readers', async () => { + const { ds, invalidated } = makeDS(); + + await ds.updateViewConfig('account', 'v1', { label: 'Renamed' }); + + // `view:{object}:{viewId}` → getView; `view-overrides:{object}` → + // listViewOverrides. Nothing else is read back under a view-shaped key. + expect(invalidated).toEqual(['view:account:v1', 'view-overrides:account']); + }); + + it('updateView (published overlay) invalidates the getView key only', async () => { + const { ds, invalidated } = makeDS({ published: { name: 'v1', object: 'account' } }); + + await ds.updateView('account', 'v1', { label: 'Renamed' }); + + expect(invalidated).toEqual(['view:account:v1']); + }); + + it('updateView (pending draft) invalidates the getView key only', async () => { + const { ds, invalidated } = makeDS({ draft: VIEW, published: notFound() }); + + await ds.updateView('account', VIEW.name, { label: 'Renamed' }); + + expect(invalidated).toEqual([`view:account:${VIEW.name}`]); + }); + + it('deleteView invalidates the getView key only', async () => { + const { ds, invalidated } = makeDS(); + + await ds.deleteView('account', 'v1'); + + expect(invalidated).toEqual(['view:account:v1']); + }); + + it('no write path invalidates a `views:` key — nothing populates one', async () => { + // createView is the path whose ONLY invalidation was the dead key, so it + // now invalidates nothing. Asserted as "no `views:` key" rather than "no + // invalidation at all": whether it ought to invalidate the override map + // (`listViewOverrides` enumerates the same rows) is a separate question, + // filed on its own card — this pin must not freeze the answer. + const created = makeDS(); + await created.ds.createView('account', { name: 'account.mine', object: 'account' }); + + const config = makeDS(); + await config.ds.updateViewConfig('account', 'v1', { label: 'Renamed' }); + + const removed = makeDS(); + await removed.ds.deleteView('account', 'v1'); + + for (const { invalidated } of [created, config, removed]) { + expect(invalidated.filter((k) => k.startsWith('views:'))).toEqual([]); + } + }); +}); diff --git a/packages/permissions/src/__tests__/evaluator.test.ts b/packages/permissions/src/__tests__/evaluator.test.ts index 13e37c89c2..de5dfe9442 100644 --- a/packages/permissions/src/__tests__/evaluator.test.ts +++ b/packages/permissions/src/__tests__/evaluator.test.ts @@ -13,14 +13,14 @@ import type { ObjectPermissionConfig, } from '@object-ui/types'; -// `RoleDefinition.permissions` is required and carries a role's DIRECT object -// grants. These three roles grant nothing directly — every grant these cases -// exercise arrives through the `ObjectPermissionConfig[]` below, keyed by object -// — so the empty array is the accurate value, not padding. What the fixtures pin -// here is identity and inheritance, which is all `resolveRoles` reads. -const adminRole: RoleDefinition = { name: 'admin', label: 'Admin', permissions: [] }; -const editorRole: RoleDefinition = { name: 'editor', label: 'Editor', inherits: ['viewer'], permissions: [] }; -const viewerRole: RoleDefinition = { name: 'viewer', label: 'Viewer', permissions: [] }; +// Every grant these cases exercise arrives through the `ObjectPermissionConfig[]` +// below, keyed by object — the only wired home for a role's grants. What the +// role fixtures pin is identity and inheritance, which is all `resolveRoles` +// reads. (`RoleDefinition` used to require a second, never-read `permissions` +// array; retired in objectui#4288.) +const adminRole: RoleDefinition = { name: 'admin', label: 'Admin' }; +const editorRole: RoleDefinition = { name: 'editor', label: 'Editor', inherits: ['viewer'] }; +const viewerRole: RoleDefinition = { name: 'viewer', label: 'Viewer' }; const roles: RoleDefinition[] = [adminRole, editorRole, viewerRole]; diff --git a/packages/permissions/src/__tests__/store.test.ts b/packages/permissions/src/__tests__/store.test.ts index 9694ab17ed..d3eabdcac8 100644 --- a/packages/permissions/src/__tests__/store.test.ts +++ b/packages/permissions/src/__tests__/store.test.ts @@ -10,12 +10,12 @@ import { describe, it, expect } from 'vitest'; import { createPermissionStore } from '../store'; import type { RoleDefinition, ObjectPermissionConfig } from '@object-ui/types'; -// Empty `permissions` is the accurate value, not padding: a role's DIRECT object -// grants live there, and every grant these cases exercise arrives through the -// `ObjectPermissionConfig[]` below instead. +// A role carries identity and inheritance only; every grant these cases +// exercise arrives through the `ObjectPermissionConfig[]` below, which is the +// only wired home for role grants (objectui#4288). const roles: RoleDefinition[] = [ - { name: 'admin', label: 'Admin', permissions: [] }, - { name: 'viewer', label: 'Viewer', permissions: [] }, + { name: 'admin', label: 'Admin' }, + { name: 'viewer', label: 'Viewer' }, ]; const permissions: ObjectPermissionConfig[] = [ diff --git a/packages/plugin-detail/src/__tests__/DetailView.permissions.test.tsx b/packages/plugin-detail/src/__tests__/DetailView.permissions.test.tsx index a7c375811d..ebdccaf176 100644 --- a/packages/plugin-detail/src/__tests__/DetailView.permissions.test.tsx +++ b/packages/plugin-detail/src/__tests__/DetailView.permissions.test.tsx @@ -25,13 +25,11 @@ import type { ObjectPermissionConfig, RoleDefinition } from '@object-ui/types'; * so they're independent of the /auth/me endpoint. */ -// `permissions: []` is accurate and required: a role's DIRECT object grants live -// on `RoleDefinition.permissions`, and this role has none — every grant it uses -// comes from the `ObjectPermissionConfig` below. (That the field is required and -// read by nothing is the dormancy filed as #4288; this fixture states the truth -// for its own role rather than pre-judging that finding.) +// Every grant this role uses comes from the `ObjectPermissionConfig` below — +// the only wired home for role grants. (`RoleDefinition` used to require a +// second, never-read `permissions` array; retired in objectui#4288.) const roles: RoleDefinition[] = [ - { name: 'restricted', label: 'Restricted', description: 'denies one field', permissions: [] }, + { name: 'restricted', label: 'Restricted', description: 'denies one field' }, ]; function makeRestrictedConfig(deniedField: string): ObjectPermissionConfig { diff --git a/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.readgate.test.tsx b/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.readgate.test.tsx index 47b3b1b5b3..184ddc4a83 100644 --- a/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.readgate.test.tsx +++ b/packages/plugin-detail/src/__tests__/RecordRelatedListRenderer.readgate.test.tsx @@ -32,7 +32,7 @@ vi.mock('../RelatedList', () => ({ const ds = { find: vi.fn(async () => []) }; const roles: RoleDefinition[] = [ - { name: 'restricted', label: 'Restricted', permissions: [] }, + { name: 'restricted', label: 'Restricted' }, ]; function contactPerms(actions: Array<'read'>): ObjectPermissionConfig[] { diff --git a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx index a71762bf3a..8b6cb77742 100644 --- a/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.permissions.test.tsx @@ -29,12 +29,11 @@ import type { ObjectPermissionConfig, RoleDefinition } from '@object-ui/types'; * register table renderers), but the $select contract is invariant. */ -// `permissions: []` is accurate and required: a role's DIRECT object grants live -// on `RoleDefinition.permissions`, and this role has none — every grant it uses -// comes from the `ObjectPermissionConfig` below. (That the field is required and -// read by nothing is the dormancy filed as #4288.) +// Every grant this role uses comes from the `ObjectPermissionConfig` below — +// the only wired home for role grants. (`RoleDefinition` used to require a +// second, never-read `permissions` array; retired in objectui#4288.) const roles: RoleDefinition[] = [ - { name: 'restricted', label: 'Restricted', description: 'denies one field', permissions: [] }, + { name: 'restricted', label: 'Restricted', description: 'denies one field' }, ]; function makeRestrictedConfig(deniedField: string): ObjectPermissionConfig { diff --git a/packages/react/src/context/RecordContext.tsx b/packages/react/src/context/RecordContext.tsx index 46bb8749ee..270a9ae9bc 100644 --- a/packages/react/src/context/RecordContext.tsx +++ b/packages/react/src/context/RecordContext.tsx @@ -120,10 +120,6 @@ export interface RecordContextValue { data?: TData; /** Resolved object metadata schema (fields, label, etc.). */ objectSchema?: TObjectSchema; - /** True while the record is fetching. */ - loading?: boolean; - /** Last fetch error, if any. */ - error?: Error | null; /** Re-fetch the record from the source. */ refresh?: () => void | Promise; /** @@ -176,8 +172,6 @@ export const RecordContextProvider: React.FC = ({ value.dataSource, value.data, value.objectSchema, - value.loading, - value.error, value.refresh, value.embedded, value.headerSystemActions, diff --git a/packages/react/src/context/__tests__/RecordContext.valueShape.pin.test.tsx b/packages/react/src/context/__tests__/RecordContext.valueShape.pin.test.tsx new file mode 100644 index 0000000000..814959ddf1 --- /dev/null +++ b/packages/react/src/context/__tests__/RecordContext.valueShape.pin.test.tsx @@ -0,0 +1,137 @@ +/** + * 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. + */ + +/** + * `RecordContextValue` shape ↔ memo dep list, pinned together (objectui#3773). + * + * `RecordContextProvider` spreads all of its props into one object and hands + * consumers a `useMemo`'d reference. That makes the dep list a hand-maintained + * mirror of the interface: a key added to `RecordContextValue` and forgotten in + * the dep list gives consumers a STALE value with no type error — the drift + * shape that let `loading` / `error` sit in the dep list for months after they + * had zero producers and zero consumers (they were retired in #3773). + * + * Two halves, deliberately joined: + * + * - a compile-time assertion that `MEMO_DEP_KEYS` is exactly + * `keyof RecordContextValue` (adding a key to the interface without listing + * it here fails `tsc`); and + * - a runtime sweep proving every listed key really re-memoizes, i.e. is in + * the dep list (adding it here without wiring the dep list fails vitest). + * + * So a new context key cannot land half-wired, and a dead one cannot linger: + * removing it from the interface forces its removal from this list, which + * forces its removal from the dep list. + */ + +import * as React from 'react'; +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import { + RecordContextProvider, + useRecordContext, + type RecordContextValue, +} from '../RecordContext'; + +/** The keys `RecordContextProvider`'s `useMemo` dep list names, in its order. */ +const MEMO_DEP_KEYS = [ + 'objectName', + 'recordId', + 'dataSource', + 'data', + 'objectSchema', + 'refresh', + 'embedded', + 'headerSystemActions', + 'isFavorite', + 'onToggleFavorite', +] as const; + +type Equal = + (() => T extends X ? 1 : 2) extends (() => T extends Y ? 1 : 2) ? true : false; +type Expect = T; + +/** + * Compile-time half. Red when `RecordContextValue` gains or loses a key + * without this list following — including the retired `loading` / `error`, + * which cannot come back silently. + */ +type _EveryContextKeyIsDepListed = Expect< + Equal +>; + +/** A complete, valid value for every key — the "before" of each mutation. */ +const BASE: RecordContextValue = { + objectName: 'account', + recordId: 'rec_1', + dataSource: 'ds_primary', + data: { id: 'rec_1', name: 'Acme' }, + objectSchema: { name: 'account' }, + refresh: () => {}, + embedded: false, + headerSystemActions: [], + isFavorite: false, + onToggleFavorite: () => {}, +}; + +/** A different value for every key — the "after" of each mutation. */ +const CHANGED: RecordContextValue = { + objectName: 'contact', + recordId: 'rec_2', + dataSource: 'ds_secondary', + data: { id: 'rec_2', name: 'Globex' }, + objectSchema: { name: 'contact' }, + refresh: () => {}, + embedded: true, + headerSystemActions: [{ name: 'edit' }], + isFavorite: true, + onToggleFavorite: () => {}, +}; + +/** Render the provider and capture the context object handed to consumers. */ +function renderWithProbe(value: RecordContextValue) { + const seen: Array = []; + const Probe: React.FC = () => { + seen.push(useRecordContext()); + return null; + }; + const ui = (v: RecordContextValue) => ( + + + + ); + const { rerender } = render(ui(value)); + return { seen, rerender: (v: RecordContextValue) => rerender(ui(v)) }; +} + +describe('RecordContextProvider — value shape and memo dep list (#3773)', () => { + it('hands consumers exactly the declared keys', () => { + const { seen } = renderWithProbe(BASE); + expect(Object.keys(seen[0]!).sort()).toEqual([...MEMO_DEP_KEYS].sort()); + }); + + it('keeps one reference when nothing changes', () => { + const { seen, rerender } = renderWithProbe(BASE); + rerender({ ...BASE }); + expect(seen.length).toBeGreaterThan(1); + expect(seen[seen.length - 1]).toBe(seen[0]); + }); + + it.each(MEMO_DEP_KEYS)('re-memoizes when `%s` changes', (key) => { + const { seen, rerender } = renderWithProbe(BASE); + const before = seen[0]!; + rerender({ ...BASE, [key]: CHANGED[key] }); + const after = seen[seen.length - 1]!; + + // A new reference AND the new value visible: a key missing from the dep + // list fails the first expectation, a key dropped from the spread the + // second. + expect(after).not.toBe(before); + expect(after[key]).toBe(CHANGED[key]); + }); +}); diff --git a/packages/types/src/permissions.ts b/packages/types/src/permissions.ts index b2e7c725ed..d018ca7f1c 100644 --- a/packages/types/src/permissions.ts +++ b/packages/types/src/permissions.ts @@ -34,7 +34,16 @@ export type { PermissionAction }; /** Permission effect */ export type PermissionEffect = 'allow' | 'deny'; -/** Role definition for RBAC */ +/** + * Role definition for RBAC — identity and inheritance only. + * + * A role's actual grants live in {@link ObjectPermissionConfig.roles}, keyed by + * object; that is the single wired home for "what a role may do". This type + * used to declare a second one (`permissions: ObjectLevelPermission[]`, + * required) that no consumer ever read — the evaluator family walks `inherits` + * and matches on `name` — so it was retired rather than left as an authoring + * surface whose values are silently ignored (objectui#4288). + */ export interface RoleDefinition { /** Unique role identifier */ name: string; @@ -46,8 +55,6 @@ export interface RoleDefinition { inherits?: string[]; /** Whether this is a system role */ system?: boolean; - /** Object-level permissions */ - permissions: ObjectLevelPermission[]; } /** Object-level permission assignment */