From 0924571056113ad79560714a50b99d63f8096f64 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 02:31:57 +0000 Subject: [PATCH 1/2] fix(plugin-grid): skip and diagnose an unusable `bulkActionDefs` member instead of crashing the selection bar (objectui#8730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bulkActions` and `bulkActionDefs` are one affordance in two vocabularies — bare action NAMES resolved against `objectDef.actions`, and full `BulkActionDef` OBJECTS used as authored — and nothing refused a member written in the other one: both keys are registered `type: 'array'` with no `of`, both spec rows are `z.array(z.unknown())`, and a JSON view is invisible to `tsc`. Writing a bare name into `bulkActionDefs` crashed the render. The string travelled into the authored list untouched, `BulkActionBar` rendered a button for it, and `def.label ?? formatActionLabel(def.name)` threw `TypeError: Cannot read properties of undefined (reading 'replace')` — so the author's first multi-row selection lost the whole selection bar, including its count, its Clear button and every well-formed sibling def. `key={def.name}` was `undefined` as well. `resolveBulkActions` now skips any member that is not an object carrying a non-empty string `name`. "Usable" is defined by what the renderer reads, not by a fresh opinion: `name` is both the React `key` and `formatActionLabel`'s argument, so one test covers the reported bare string and, identically, `null`, a number, `{}` and `{ name: '' }`. The guard sits at the single point where the authored array becomes the list the bar maps over, so the key and the label are read off the same validated def, and the clean case still returns the authored array by reference. The skip is not silent: `ObjectGrid` reports it once per authored array through the channel it already owns for "you declared it, the renderer dropped it", naming the block, the index, what was seen and what to write instead — including that a bare name belongs in `bulkActions`. Not a coercion, deliberately: `'approve'` is not lifted into `{ name: 'approve' }` and resolved the way `bulkActions` is. That would make the two vocabularies interchangeable, which is a product change to what a `bulkActionDefs` member means (objectui#3002 / objectui#3139), not a crash fix. The mirror direction (`bulkActions: [{ name: 'approve' }]`) keeps its existing silent skip. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../8730-bulk-action-defs-unusable-member.md | 55 +++ packages/plugin-grid/src/ObjectGrid.tsx | 26 +- ...bulkActionDefsUnusableMember-8730.test.tsx | 319 ++++++++++++++++++ .../plugin-grid/src/resolveBulkActions.ts | 144 +++++++- 4 files changed, 542 insertions(+), 2 deletions(-) create mode 100644 .changeset/8730-bulk-action-defs-unusable-member.md create mode 100644 packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx diff --git a/.changeset/8730-bulk-action-defs-unusable-member.md b/.changeset/8730-bulk-action-defs-unusable-member.md new file mode 100644 index 0000000000..7eeb38faf8 --- /dev/null +++ b/.changeset/8730-bulk-action-defs-unusable-member.md @@ -0,0 +1,55 @@ +--- +'@object-ui/plugin-grid': patch +--- + +`object-grid`: a `bulkActionDefs` member that is not a usable def is skipped and +diagnosed, instead of taking the whole selection bar down (objectui#8730). + +`bulkActions` and `bulkActionDefs` are one affordance authored in two vocabularies — +`bulkActions` members are bare action NAMES resolved against `objectDef.actions`, +`bulkActionDefs` members are full `BulkActionDef` OBJECTS used as authored — and +nothing refused a member written in the other one. Both keys are registered +`type: 'array'` with no `of`, both spec rows are `z.array(z.unknown())`, and a JSON +view is invisible to `tsc`. + +Writing a bare name into `bulkActionDefs` did not fail quietly, it crashed: +`Array.isArray(schema.bulkActionDefs)` is true, the string travelled into the authored +list untouched, `BulkActionBar` rendered a button for it, and +`def.label ?? formatActionLabel(def.name)` threw +`TypeError: Cannot read properties of undefined (reading 'replace')` **during render**. +The author's first multi-row selection lost the entire selection bar — count, Clear and +every well-formed sibling def with it. `key={def.name}` was `undefined` too, so React +logged a duplicate-key warning on the way down. + +`resolveBulkActions` now skips any member that is not an object carrying a non-empty +string `name`. "Usable" is defined by what the renderer actually reads: `name` is both +the React `key` and `formatActionLabel`'s argument, so that one test covers the reported +bare string and, identically, `null`, a number, `{}` and `{ name: '' }`. The guard sits +at the single point where the authored array becomes the list the bar maps over, so the +`key` and the label are read off the same validated def. + +The skip is not silent. `ObjectGrid` reports it once per authored array through the +channel it already uses for "you declared it, the renderer dropped it" — one +`console.warn` prefixed `[ObjectUI] ObjectGrid bulkActionDefs:` — naming the block, the +index, what was seen, and what to write instead: + +``` +[ObjectUI] ObjectGrid bulkActionDefs: object-grid (objectName: 'os_invoice') — 1 of 3 +authored bulk-action defs cannot be rendered and is skipped (2 still render). + • bulkActionDefs[0]: the entry is a string ('approve'), not a def object — this key's + members are full `BulkActionDef` objects, used as authored. Write + `{ name: 'approve', operation: 'custom' }` here, or move the bare name to + `bulkActions`, which resolves it against the object's declared actions and promotes + the match. +``` + +**Not a coercion, deliberately.** A bare `'approve'` is not lifted into +`{ name: 'approve' }` and resolved the way `bulkActions` is. That would make the two +vocabularies interchangeable — a product change to what a `bulkActionDefs` member means +(objectui#3002 / objectui#3139 made them distinct on purpose), not a crash fix. + +**Behaviour changes for authors** beyond the crash: a `{ name: '' }` member used to +render a nameless, unlabelled button with an empty React key, and now renders nothing. +Well-formed defs are untouched — a mixed list renders exactly its usable members, in +order, and a clean array is still returned by reference. The mirror direction +(`bulkActions: [{ name: 'approve' }]`) keeps its existing silent skip. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 323886ff5d..95d8fbbab5 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -47,7 +47,7 @@ import { resolveRowCrudAffordances, resolveRowRecordCrudAffordance } from './row import { useRecordCrudVerdicts } from './hooks/useRecordCrudVerdicts'; import { resolveLegacyRowActions } from './resolveLegacyRowActions'; import { applyRelationalMeta } from './relationalMetaKeys'; -import { resolveBulkActions } from './resolveBulkActions'; +import { resolveBulkActions, describeUnusableBulkActionDefs } from './resolveBulkActions'; import { partitionBulkRows } from './bulkEligibility'; import { resolvesToDataColumn, describeUnresolvedColumns } from './columnSpellingDiagnostics'; import { RowActionMenu, formatActionLabel } from './components/RowActionMenu'; @@ -2226,6 +2226,30 @@ export const ObjectGrid: React.FC = ({ if (message) console.warn(message); }, [schema.columns, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); + // [objectui#8730] The same channel, for the sibling failure on + // `bulkActionDefs`. A member that is not a usable def (a bare action name — + // the OTHER key's vocabulary — or `null`, a number, `{}`, `{ name: '' }`) is + // skipped by `resolveBulkActions`; before that guard it reached the bar and + // `formatActionLabel(undefined)` threw during render, so the author's first + // multi-row selection lost the whole selection bar. + // + // The skip alone would only relocate the failure into silence — which is what + // the mirror direction already does (`bulkActions: [{ name: 'approve' }]`, + // stepped over by the `typeof name !== 'string'` guard). Saying which member + // was dropped, and that a bare name belongs in `bulkActions`, is what makes + // this a diagnosis rather than a quieter version of the same defect. One + // `console.warn` per authored array, keyed on it — NOT a second guard: the + // predicate lives once, in `resolveBulkActions`, and this reads it. + const bulkDefsDiagnosticSlice = (schema as { bulkActionDefs?: unknown }).bulkActionDefs; + useEffect(() => { + const message = describeUnusableBulkActionDefs(bulkDefsDiagnosticSlice, { + blockType: columnDiagnosticBlockType, + objectName: schema.objectName, + label: columnDiagnosticLabel, + }); + if (message) console.warn(message); + }, [bulkDefsDiagnosticSlice, columnDiagnosticBlockType, schema.objectName, columnDiagnosticLabel]); + const generateColumns = useCallback((): ObjectGridColumnDraft[] => { // Map field type to column header icon (Airtable-style) const getTypeIcon = (fieldType: string | null): React.ReactNode => { diff --git a/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx new file mode 100644 index 0000000000..73624f03de --- /dev/null +++ b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx @@ -0,0 +1,319 @@ +/** + * 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. + */ + +/** + * `object-grid.bulkActionDefs` — a member that is not a usable def is SKIPPED + * and DIAGNOSED, never rendered (objectui#8730). + * + * ## What was wrong + * + * `bulkActions` and `bulkActionDefs` are one affordance authored in two + * vocabularies: `bulkActions` members are BARE ACTION NAMES resolved against + * `objectDef.actions`; `bulkActionDefs` members are FULL `BulkActionDef` + * OBJECTS used as authored. Nothing refuses a member written in the other + * vocabulary — both keys are registered `type: 'array'` with no `of`, and both + * spec rows are `z.array(z.unknown())` — so the read site is the whole member + * contract. + * + * Writing a bare name into `bulkActionDefs` did not fail quietly, it CRASHED: + * `Array.isArray(schema.bulkActionDefs)` is true, the string travelled into the + * authored list untouched, `BulkActionBar` rendered a `BulkActionButton` for + * it, and `def.label ?? formatActionLabel(def.name)` threw + * `TypeError: Cannot read properties of undefined (reading 'replace')` DURING + * RENDER — the author's first multi-row selection lost the entire selection + * bar. `key={def.name}` was `undefined` too, so React logged a duplicate-key + * warning on the way down. + * + * ## What is pinned here + * + * "Usable" is defined by what the RENDERER READS, not by a fresh opinion: + * `BulkActionBar` uses `def.name` both as the React `key` and as + * `formatActionLabel`'s argument, so a member must be an object carrying a + * non-empty string `name`. One test covers the whole unusable class — the + * reported bare string, `null`, a number, `{}` and `{ name: '' }`. + * + * ⭐ The load-bearing half is the NON-REGRESSION axis, because "skip the bad + * member" is also satisfied by an implementation that skips EVERYTHING. So the + * class rows below are paired with rows that fail under exactly that caricature: + * + * - reddens under "skip everything": rows 6 (a mixed list renders exactly the + * two good defs, in order, with their labels) and 7 (a clean list is + * untouched — same buttons, and the resolved array is the authored array BY + * REFERENCE). + * - reddens under "skip nothing": rows 1-5 (the class) and row 8's fire leg. + * + * The diagnostic is an assertion too, so it is pinned as a PAIR (row 8): it + * must fire for the bad member and NAME it, and it must NOT fire for a clean + * list. A warning that fires always is as useless as one that never fires. + * + * ## What row 9 inherits + * + * objectui#8071's member pin asserted this crash as the CURRENT SHAPE, so that + * it could not be mistaken for the SILENT drop of the mirror direction + * (`bulkActions: [{ name: 'approve' }]`, stepped over by `resolveBulkActions`'s + * `typeof name !== 'string'` guard). That distinction is not deleted by this + * fix, it is sharpened, and row 9 carries it: both directions now SKIP, but only + * the def direction is diagnosed. Direction one's silence is asserted here as + * well — it is deliberately unchanged (objectui#8730's ruling puts it out of + * scope), and this row is what would notice a diagnostic leaking into it. + * + * `selection` is declared explicitly on every row so the ONLY variable between + * them is the member shape. Left implicit, the grid auto-enables multi-select + * from `hasBulkActions`, which is itself derived from these two keys — so a + * row whose members are all skipped would lose its selection UI for the very + * reason under test and pass without ever reaching the bar. + */ + +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { resolveBulkActions } from '../resolveBulkActions'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; +import type { BulkActionDef } from '@object-ui/types'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const OBJECT = 'os_invoice'; + +/** + * The object declares ONE action, `approve`, whose label is NOT the humanized + * form of its name. That gap is the instrument for row 9: a button reading + * "Approve the invoice" can only have come from resolving a member as a NAME. + */ +const OBJECT_ACTIONS = [{ name: 'approve', label: 'Approve the invoice', variant: 'primary' }]; + +/** Two well-formed defs, used wherever a row needs survivors to count. */ +const ARCHIVE: BulkActionDef = { name: 'archive', operation: 'custom', label: 'Put it away' }; +const EXPORT: BulkActionDef = { name: 'export_pdf', operation: 'custom', label: 'Export PDF' }; + +/** + * The whole unusable class, each entry paired with the substring the + * diagnostic must use to address it. `{ name: '' }` is in here for a reason + * worth stating: it was the ONE member of the class that did not throw before + * the fix — it rendered a nameless, unlabelled button with an empty React key. + * Skipping is therefore a change in behaviour for it too, not only for the + * four that crashed. + */ +const UNUSABLE: ReadonlyArray = [ + ['a bare action name (the reported shape)', 'approve'], + ['null', null], + ['a number', 42], + ['an empty object', {}], + ['an empty-string name', { name: '' }], +]; + +function makeDataSource() { + const rows = [ + { id: 'r1', name: 'INV-1', status: 'draft' }, + { id: 'r2', name: 'INV-2', status: 'draft' }, + ]; + return { + find: vi.fn(async () => ({ + data: rows.map((r) => ({ ...r })), + total: rows.length, + hasMore: false, + pageSize: 50, + })), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + actions: OBJECT_ACTIONS, + }), + } as any; +} + +async function renderAndSelectAll(schema: Record) { + render( + + + , + ); + await waitFor(() => expect(screen.getByText('INV-1')).toBeInTheDocument()); + const headerCheckbox = document.querySelector('thead [role="checkbox"]') as HTMLElement; + expect(headerCheckbox, 'the multi-select header checkbox').toBeTruthy(); + fireEvent.click(headerCheckbox); + await waitFor(() => expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument()); +} + +/** + * Every bulk-action button in the bar, in DOM order, by `data-testid`. + * + * COUNTING is the point, not "something rendered": a census that navigates with + * `querySelector` never notices a node it did not expect (measured on + * objectui#8596, where `.rounded-full` matched two nodes per avatar). The + * prefix ends in a hyphen so the bar's own `bulk-actions-bar` testid cannot + * match, and the query is scoped to the bar so a second bar would be visible as + * a `getByTestId` failure rather than as doubled counts. + */ +function renderedBulkActionIds(): string[] { + const bar = screen.getByTestId('bulk-actions-bar'); + return Array.from(bar.querySelectorAll('[data-testid^="bulk-action-"]')).map( + (n) => n.getAttribute('data-testid') as string, + ); +} + +/** The diagnostic channel: `console.warn` lines this key owns. */ +function bulkDefWarnings(warn: ReturnType): string[] { + return warn.mock.calls + .map((args) => String(args[0])) + .filter((line) => line.includes('bulkActionDefs')); +} + +/** React's duplicate-key complaint — the second symptom of an undefined `name`. */ +function duplicateKeyErrors(error: ReturnType): string[] { + return error.mock.calls + .map((args) => args.map((a: unknown) => String(a)).join(' ')) + .filter((line) => line.includes('unique "key" prop')); +} + +describe('object-grid `bulkActionDefs`: an unusable member is skipped (objectui#8730)', () => { + UNUSABLE.forEach(([label, member], i) => { + it(`${i + 1}. ${label} renders no button, and does not take the bar down`, async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + // The pre-fix failure was a THROW during render, so reaching this line at + // all is half the assertion; the count is the other half. + await renderAndSelectAll({ bulkActionDefs: [member] }); + + expect(renderedBulkActionIds()).toEqual([]); + // The bar itself survives — that is the affordance the crash destroyed. + expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument(); + expect(duplicateKeyErrors(error)).toEqual([]); + // Skipping is never silent on this key (row 8 pins the message itself). + expect(bulkDefWarnings(warn)).toHaveLength(1); + }); + }); + + it('6. a mixed list renders EXACTLY the well-formed defs, in order', async () => { + // ⭐ The discriminating row. "Skip the bad member" is also satisfied by an + // implementation that skips every member, and by one that drops the + // survivors' order or identity. Only an exact, ordered census refuses all + // three at once. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await renderAndSelectAll({ bulkActionDefs: ['approve', ARCHIVE, EXPORT] }); + + expect(renderedBulkActionIds()).toEqual(['bulk-action-archive', 'bulk-action-export_pdf']); + expect(screen.getByTestId('bulk-action-archive')).toHaveTextContent('Put it away'); + expect(screen.getByTestId('bulk-action-export_pdf')).toHaveTextContent('Export PDF'); + // The `key` half of "the key and the label read the same validated def": + // the keys are the two surviving names, so React has nothing to complain + // about. An `undefined` key is what the reported defect produced first. + expect(duplicateKeyErrors(error)).toEqual([]); + // One skip, reported once, naming the member that was skipped. + expect(bulkDefWarnings(warn)).toHaveLength(1); + expect(bulkDefWarnings(warn)[0]).toContain('bulkActionDefs[0]'); + }); + + it('7. a list of only well-formed defs is untouched', async () => { + // ⭐ The other half of the discriminating pair, at both levels. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await renderAndSelectAll({ bulkActionDefs: [ARCHIVE, EXPORT] }); + expect(renderedBulkActionIds()).toEqual(['bulk-action-archive', 'bulk-action-export_pdf']); + expect(bulkDefWarnings(warn)).toEqual([]); + + // Unit level, and the sharper assertion of the two: a clean authored array + // comes back BY REFERENCE. An always-allocating filter would pass the DOM + // census above and fail here — and a view whose defs lose referential + // identity every render is a real cost, not a stylistic one (the array is a + // `useMemo`/`useEffect` dependency downstream). + const authored = [ARCHIVE, EXPORT]; + const { defs } = resolveBulkActions({ bulkActionDefs: authored }); + expect(defs).toBe(authored); + }); + + it('8. the diagnostic fires for the bad member, by name — and not for a clean list', async () => { + // An assertion that a warning EXISTS is worth little on its own: a warning + // that fires always is as useless as one that never fires. Both legs, in + // one row, so the pair cannot drift apart. + const fired = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: ['approve'] }); + + const lines = bulkDefWarnings(fired); + expect(lines).toHaveLength(1); + // WHERE: the addressed block, and the index inside the authored array. + expect(lines[0]).toContain("objectName: 'os_invoice'"); + expect(lines[0]).toContain('bulkActionDefs[0]'); + // WHAT: the member itself, quoted — this is the "by name" half. + expect(lines[0]).toContain("'approve'"); + // WHAT TO DO: the other vocabulary is named, since that is where a bare + // action name belongs. + expect(lines[0]).toContain('bulkActions'); + fired.mockRestore(); + + // No-fire leg, on a fresh render of a clean list. + document.body.innerHTML = ''; + const quiet = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: [ARCHIVE] }); + expect(bulkDefWarnings(quiet)).toEqual([]); + }); +}); + +describe('the two vocabularies still fail DIFFERENTLY (inherited from objectui#8071)', () => { + it('9. direction one stays silent; direction two skips and says so', async () => { + // What objectui#8071's row 5 was protecting: direction two is NOT direction + // one's silent drop. Before this fix the difference was crash-vs-silence; + // it is now diagnostic-vs-silence. Both are skips, and neither throws. + // + // Direction one — the DEF vocabulary written into `bulkActions`. Stepped + // over by `resolveBulkActions`'s `typeof name !== 'string'` guard, with no + // diagnostic. Deliberately unchanged (objectui#8730 scopes it out); this + // leg is what would notice a diagnostic leaking into it. + const one = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActions: [{ name: 'approve' }] }); + expect(renderedBulkActionIds()).toEqual([]); + expect(one).not.toHaveBeenCalled(); + one.mockRestore(); + + // Direction two — the NAME vocabulary written into `bulkActionDefs`. Same + // disposition, opposite treatment: skipped, and reported. + document.body.innerHTML = ''; + const two = vi.spyOn(console, 'warn').mockImplementation(() => {}); + await renderAndSelectAll({ bulkActionDefs: ['approve'] }); + expect(renderedBulkActionIds()).toEqual([]); + expect(bulkDefWarnings(two)).toHaveLength(1); + }); + + it('10. a name in the RIGHT key still resolves and promotes', async () => { + // The guard must not be reachable from the vocabulary that is supposed to + // carry bare names. The object action's OWN label proves the promotion + // happened rather than a humanized fallback. + await renderAndSelectAll({ bulkActions: ['approve'] }); + expect(renderedBulkActionIds()).toEqual(['bulk-action-approve']); + expect(screen.getByTestId('bulk-action-approve')).toHaveTextContent('Approve the invoice'); + }); +}); diff --git a/packages/plugin-grid/src/resolveBulkActions.ts b/packages/plugin-grid/src/resolveBulkActions.ts index ba993c40bc..c094482097 100644 --- a/packages/plugin-grid/src/resolveBulkActions.ts +++ b/packages/plugin-grid/src/resolveBulkActions.ts @@ -153,6 +153,133 @@ function toBulkActionDef(action: NamedActionDef, localize?: ActionLabelResolver) }; } +/** + * Is this authored `bulkActionDefs` member a def the selection bar can render? + * + * "Usable" is defined by WHAT THE RENDERER READS, not by a fresh opinion about + * what a def ought to carry: `BulkActionBar` uses `def.name` twice — as the + * React `key` of the button it maps to, and as the argument to + * `formatActionLabel` when the def declares no `label`. A member without a + * non-empty string `name` therefore has no identity and no label, and + * `formatActionLabel(undefined)` threw `TypeError: Cannot read properties of + * undefined (reading 'replace')` DURING RENDER — taking the whole selection bar + * down on the author's first multi-row selection (objectui#8730). + * + * Every other `BulkActionDef` key is optional at the read site (an absent + * `operation` falls through to the button's default treatment, an absent + * `visible` means ungated), so `name` is the whole predicate. It covers the + * reported bare string and, by the same test, `null`, a number, `{}` and + * `{ name: '' }`. + * + * ⛔ NOT a coercion. A bare `'approve'` is NOT lifted into `{ name: 'approve' }` + * and resolved the way `bulkActions` is: that would make the two vocabularies + * interchangeable, which is a product change to what a `bulkActionDefs` member + * MEANS (objectui#3002 / objectui#3139 established them as two vocabularies + * deliberately), not a crash fix. The member is skipped, and said out loud. + */ +export function isUsableBulkActionDef(member: unknown): member is BulkActionDef { + if (!member || typeof member !== 'object') return false; + const name = (member as { name?: unknown }).name; + return typeof name === 'string' && name.length > 0; +} + +/** Where the offending block lives, for the first line of the message. */ +export interface BulkActionDefsAddress { + /** The schema node's `type` — `object-grid`, or the `view:grid` alias. */ + blockType?: unknown; + /** The object the grid queries, when it names one. */ + objectName?: unknown; + /** The grid's authored label, when it has one — often the only human name. */ + label?: unknown; +} + +function quote(value: unknown): string { + return typeof value === 'string' ? `'${value}'` : String(value); +} + +function describeAddress({ blockType, objectName, label }: BulkActionDefsAddress): string { + const block = typeof blockType === 'string' && blockType.length > 0 ? blockType : 'object-grid'; + const parts: string[] = []; + if (typeof objectName === 'string' && objectName.length > 0) parts.push(`objectName: '${objectName}'`); + else parts.push('no objectName'); + if (typeof label === 'string' && label.length > 0) parts.push(`label: '${label}'`); + return `${block} (${parts.join(', ')})`; +} + +/** + * Describe ONE skipped member: what it spells, and what to write instead. + * + * Each branch reports only what it checked. The string arm carries the extra + * sentence because a bare action name is not nonsense — it is the OTHER key's + * vocabulary, and naming that key is the whole fix for the author. + */ +function describeMember(member: unknown): string { + if (member === null || member === undefined) { + return `the entry is ${String(member)}, not a def object`; + } + if (typeof member !== 'object') { + const rewrite = typeof member === 'string' && member.length > 0 + ? ` Write \`{ name: '${member}', operation: 'custom' }\` here, or move the bare name to ` + + '`bulkActions`, which resolves it against the object\'s declared actions and promotes the match.' + : ''; + return `the entry is a ${typeof member} (${quote(member)}), not a def object — this key's ` + + `members are full \`BulkActionDef\` objects, used as authored.${rewrite}`; + } + const entry = member as Record; + const keys = Object.keys(entry); + const seen = keys.length > 0 + ? `keys seen: ${keys.map((k) => `\`${k}\``).join(', ')}` + : 'the entry is empty `{}`'; + if (!('name' in entry)) { + return `${seen} — no \`name\` key, so the def has no identity.`; + } + const name = entry.name; + if (typeof name !== 'string') { + return `${seen} — \`name\` is a ${name === null ? 'null' : typeof name} (${quote(name)}), ` + + 'and only a non-empty string names an action.'; + } + return `${seen} — \`name\` is an empty string, so the button would carry no key and no label.`; +} + +/** + * The message for a `bulkActionDefs` array carrying members the bar cannot + * render, or `null` when every authored member is usable. + * + * Naming the ADDRESS is the whole point: which block, which object, which + * index, what was seen, and what to write instead. A message that only said + * something went wrong would leave the author exactly where the crash did. + * + * The channel is `ObjectGrid`'s existing one for "you declared it, the renderer + * dropped it" — a `useEffect` keyed on the schema slice and one `console.warn` + * prefixed `[ObjectUI] ObjectGrid :`, the same shape as the columns + * diagnostic and the export-format warning, rather than a third differently + * shaped one beside them. + */ +export function describeUnusableBulkActionDefs( + members: unknown, + address: BulkActionDefsAddress, +): string | null { + if (!Array.isArray(members) || members.length === 0) return null; + const skipped = members + .map((member, index) => ({ index, member })) + .filter(({ member }) => !isUsableBulkActionDef(member)); + if (skipped.length === 0) return null; + + const survivors = members.length - skipped.length; + const subject = `${skipped.length} of ${members.length} authored bulk-action ` + + `def${members.length === 1 ? '' : 's'} cannot be rendered and ` + + `${skipped.length === 1 ? 'is' : 'are'} skipped`; + const headline = survivors === 0 + ? `${subject}, so this selection bar offers NO bulk-action buttons` + : `${subject} (${survivors} still render)`; + const lines = skipped.map(({ index, member }) => ` • bulkActionDefs[${index}]: ${describeMember(member)}`); + + return `[ObjectUI] ObjectGrid bulkActionDefs: ${describeAddress(address)} — ${headline}.\n` + + `${lines.join('\n')}\n` + + ' A `bulkActionDefs` member must be an object with a non-empty string `name`: the selection ' + + 'bar uses it as the button\'s React key AND as the source of its label.'; +} + export function resolveBulkActions(opts: { /** * The view's `bulkActions` names, already stripped of the canonical @@ -180,7 +307,22 @@ export function resolveBulkActions(opts: { /** Names that matched no declared action; dispatched by name. */ unresolved: string[]; } { - const rawAuthored = Array.isArray(opts.bulkActionDefs) ? opts.bulkActionDefs : []; + // [objectui#8730] THE GUARD SITE. This is the one place where the authored + // `bulkActionDefs` array becomes the list the selection bar maps over, so the + // React `key` and the label are read off the SAME validated def. A member the + // bar cannot render is dropped here rather than refused, coerced, or defended + // against again downstream — `BulkActionBar` keeps reading `def.name` + // unconditionally, because by this line it can. + // + // Referential identity is preserved when nothing is dropped (`every` before + // `filter`): the `defs` contract below promises the authored array BY + // REFERENCE when nothing folds in, and an always-allocating filter would + // break that for every clean view — see `resolveBulkActions.test.ts`'s + // "returns the authored array by reference when nothing folds in". + const authoredMembers = Array.isArray(opts.bulkActionDefs) ? opts.bulkActionDefs : []; + const rawAuthored = authoredMembers.every(isUsableBulkActionDef) + ? authoredMembers + : authoredMembers.filter(isUsableBulkActionDef); const names = Array.isArray(opts.bulkActions) ? opts.bulkActions : []; const objectActions = Array.isArray(opts.objectActions) ? opts.objectActions : []; From b9b0abeab229373d26e931b670eba3bbc4015c94 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 02:36:34 +0000 Subject: [PATCH 2/2] test(plugin-grid): type the console-spy readers in the objectui#8730 pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ReturnType` erases the call-log element type, so both helpers took `args` implicitly `any` and `tsc -p tsconfig.test.json` refused the file with four TS7006s. The package's `type-check` is what caught it — the file was green under vitest either way, which is exactly the gap between "the tests pass" and "the tests are checked". A local structural `ConsoleSpy` reads only what these helpers use (the call log) and stays true for both `console.warn` and `console.error` spies. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../bulkActionDefsUnusableMember-8730.test.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx index 73624f03de..0ea434e14f 100644 --- a/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx +++ b/packages/plugin-grid/src/__tests__/bulkActionDefsUnusableMember-8730.test.tsx @@ -183,18 +183,21 @@ function renderedBulkActionIds(): string[] { ); } +/** Just enough of a spy to read its call log, without importing vitest's generics. */ +type ConsoleSpy = { mock: { calls: unknown[][] } }; + /** The diagnostic channel: `console.warn` lines this key owns. */ -function bulkDefWarnings(warn: ReturnType): string[] { +function bulkDefWarnings(warn: ConsoleSpy): string[] { return warn.mock.calls - .map((args) => String(args[0])) - .filter((line) => line.includes('bulkActionDefs')); + .map((args: unknown[]) => String(args[0])) + .filter((line: string) => line.includes('bulkActionDefs')); } /** React's duplicate-key complaint — the second symptom of an undefined `name`. */ -function duplicateKeyErrors(error: ReturnType): string[] { +function duplicateKeyErrors(error: ConsoleSpy): string[] { return error.mock.calls - .map((args) => args.map((a: unknown) => String(a)).join(' ')) - .filter((line) => line.includes('unique "key" prop')); + .map((args: unknown[]) => args.map((a: unknown) => String(a)).join(' ')) + .filter((line: string) => line.includes('unique "key" prop')); } describe('object-grid `bulkActionDefs`: an unusable member is skipped (objectui#8730)', () => {