diff --git a/.changeset/7217-grouping-null-entry-guard.md b/.changeset/7217-grouping-null-entry-guard.md new file mode 100644 index 000000000..ba37e2c0b --- /dev/null +++ b/.changeset/7217-grouping-null-entry-guard.md @@ -0,0 +1,13 @@ +--- +'@object-ui/plugin-grid': patch +--- + +Grid: a malformed entry in `grouping.fields[]` no longer crashes the whole grid. + +A `null` (or `undefined`) hole in the array was dereferenced with no guard at +two places — `ObjectGrid`'s `groupValueFormatter` memo and `useGroupedData`'s +`buildLevel` — throwing `TypeError: Cannot read properties of null (reading +'field')` during render, before any projection was built. Both sites now read +one normalized entry list, admitting exactly the entries `collectGroupingFieldRefs` +harvests into the query projection, so the usable grouping levels still group +and an unusable entry is simply dropped rather than taking the view down. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 231b6e5b0..b8053e5e9 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -40,7 +40,7 @@ import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, c import { usePermissions } from '@object-ui/permissions'; import { ChevronRight, ChevronDown, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react'; import { useRowColor } from './useRowColor'; -import { useGroupedData } from './useGroupedData'; +import { useGroupedData, usableGroupingFields } from './useGroupedData'; import { GroupRow } from './GroupRow'; import { useColumnSummary } from './useColumnSummary'; import { resolveRowCrudAffordances, resolveRowRecordCrudAffordance } from './rowCrudAffordances'; @@ -2034,14 +2034,21 @@ export const ObjectGrid: React.FC = ({ // readable label for select/boolean fields rather than the raw value // (e.g. "In Progress" instead of "in_progress", "Yes" instead of "true"). const groupValueFormatter = React.useMemo(() => { - const grouping = schema.grouping; - if (!grouping?.fields?.length) return undefined; + // [objectui#7217] ONE normalized entry list, shared with the + // `useGroupedData` call below. Reading `grouping.fields` raw here threw + // `TypeError: Cannot read properties of null (reading 'field')` on a null + // hole — the whole grid gone, during render, before any projection was + // built. `usableGroupingFields` admits exactly the entries + // `collectGroupingFieldRefs` harvests into the projection, so the grid can + // never group by an entry the query never asked for. + const groupingFields = usableGroupingFields(schema.grouping?.fields); + if (!groupingFields.length) return undefined; // Per-field { value -> label } lookup, plus a per-field type so we can // handle booleans / dates / users without dedicated option lists. const lookup = new Map }>(); - for (const gf of grouping.fields) { + for (const gf of groupingFields) { const fieldName = gf.field; const objectDefField = objectSchema?.fields?.[fieldName]; // Try to find a column override matching this field for type/options diff --git a/packages/plugin-grid/src/__tests__/groupingNullEntry-7217.test.tsx b/packages/plugin-grid/src/__tests__/groupingNullEntry-7217.test.tsx new file mode 100644 index 000000000..0e8e6daa8 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/groupingNullEntry-7217.test.tsx @@ -0,0 +1,171 @@ +/** + * 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. + */ + +/** + * objectui#7217 — a `null` entry in `grouping.fields[]` must not take the grid + * down. + * + * ## The defect + * + * `ObjectGrid`'s `groupValueFormatter` memo walked `grouping.fields` and read + * `gf.field` off every entry with no guard, so a single `null` hole threw + * `TypeError: Cannot read properties of null (reading 'field')` during render + * — the whole grid, gone, before any projection was built. + * + * `useGroupedData` is a SECOND dereference site of the same list (`const f = + * fields[depth]` then `f.field` / `f.order` / `f.collapsed`), so guarding the + * memo alone only moves the crash one call downstream. Both sites now read one + * normalized entry list — `usableGroupingFields` — and this file pins both: + * ablating either guard on its own turns these tests red. + * + * ## Why a guard, not a schema change (the reachability measurement) + * + * Author-time validation ALREADY refuses a null entry — `GroupingConfigSchema` + * types `fields` as an array of `$strict` objects, so `{ fields: [null] }` + * fails with `invalid_type` at `fields.0`, and objectui's own `ListViewSchema` + * inherits that by reference. The last two `it`s below measure exactly that, + * so the claim is checked rather than asserted in prose. + * + * That makes this a defensive guard rather than a validation gap — but the + * crash is still live, because NOTHING ON THE RENDER PATH RUNS THAT VALIDATOR. + * `ObjectGrid` reads `schema.grouping` straight off its props; `@object-ui/core`'s + * `validateSchema` is structural and never looks at the `grouping` key. A + * runtime-composed or generated schema therefore reaches the memo unparsed, + * which is the reachable path this pin closes. + * + * ## Test-source note + * + * The root vitest config aliases `@object-ui/*` to each package's `src`, and + * this file imports `../ObjectGrid` relatively, so no build step stands + * between the edit and the run — the ablation recorded in the PR body reads + * source directly. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; +import { GroupingConfigSchema } from '@objectstack/spec/ui'; +import { ListViewSchema } from '@object-ui/types/zod'; + +registerAllFields(); + +const ROWS = [ + { id: '1', name: 'Row 1', active: true }, + { id: '2', name: 'Row 2', active: false }, + { id: '3', name: 'Row 3', active: true }, +]; + +/** + * Mount a grid whose `grouping.fields` is exactly `fields`. + * + * `data.provider: 'value'` keeps the rows inline, so `useGroupedData` runs over + * a NON-EMPTY array: the hook's dereference is only reached once there is a row + * to bucket, and a pin mounted on empty data would leave that half unmeasured. + */ +function renderGrid(fields: unknown[]) { + const schema: any = { + type: 'object-grid', + objectName: 'test_object', + columns: [ + { field: 'name', label: 'Name' }, + { field: 'active', label: 'Active', type: 'boolean' }, + ], + data: { provider: 'value', items: ROWS }, + grouping: { fields }, + }; + return render( + + + , + ); +} + +const groupLabels = () => + Array.from(document.querySelectorAll('.group-label')).map((el) => el.textContent); + +afterEach(() => cleanup()); + +describe('ObjectGrid — a null `grouping.fields[]` entry never crashes the grid (objectui#7217)', () => { + // ── PIN 1: THE DEFECT ─────────────────────────────────────────────────── + it('renders instead of throwing when the only grouping entry is null', async () => { + expect( + () => renderGrid([null]), + 'a null hole in `grouping.fields[]` threw a TypeError out of render and ' + + 'took the whole grid down before any projection was built', + ).not.toThrow(); + await waitFor(() => expect(document.body.textContent).toContain('Row 1')); + expect(document.body.textContent).toContain('Row 2'); + expect(document.body.textContent).toContain('Row 3'); + }); + + it('renders instead of throwing when the only grouping entry is undefined', async () => { + // Same defect class as `null`: a hole a trailing comma or a sparse + // generator leaves behind, which no dereference can survive. + expect(() => renderGrid([undefined])).not.toThrow(); + await waitFor(() => expect(document.body.textContent).toContain('Row 1')); + }); + + // ── PIN 2: THE SURVIVING ENTRY STILL GROUPS ───────────────────────────── + // The guard must DROP the unusable entry, not abandon grouping altogether — + // otherwise a single hole silently degrades a working grouped view into a + // flat one, which is the objectui#7179 class of silent wrong answer. + it('still groups by the usable entry when a null precedes it', async () => { + expect(() => renderGrid([null, { field: 'active' }])).not.toThrow(); + await waitFor(() => expect(groupLabels().length).toBeGreaterThan(0)); + expect(groupLabels()).toEqual(expect.arrayContaining(['Yes', 'No'])); + }); + + it('still groups by the usable entry when a null follows it at a deeper level', async () => { + // The second entry is the NESTED level, so this reaches `buildLevel`'s + // recursion rather than only its depth-0 call. + expect(() => renderGrid([{ field: 'active' }, null])).not.toThrow(); + await waitFor(() => expect(groupLabels().length).toBeGreaterThan(0)); + expect(groupLabels()).toEqual(expect.arrayContaining(['Yes', 'No'])); + }); + + // ── PIN 3: REACHABILITY — the validator refuses it, the render path never runs one ── + it('author-time validation already refuses a null entry (`@objectstack/spec`)', () => { + const refused = GroupingConfigSchema.safeParse({ fields: [null] }); + expect(refused.success).toBe(false); + expect(refused.success === false && refused.error.issues[0]).toMatchObject({ + code: 'invalid_type', + path: ['fields', 0], + }); + // Positive control: the well-formed entry the same schema accepts, so a + // schema that refused EVERYTHING could not pass the assertion above. + expect(GroupingConfigSchema.safeParse({ fields: [{ field: 'active' }] }).success).toBe(true); + }); + + it("objectui's own `ListViewSchema` inherits that refusal by reference", () => { + const refused = ListViewSchema.safeParse({ + type: 'list-view', + objectName: 'test_object', + grouping: { fields: [null] }, + }); + expect(refused.success).toBe(false); + expect( + refused.success === false + && refused.error.issues.some((i) => i.path.join('.') === 'grouping.fields.0'), + '`grouping` is imported into `ListViewSchema` from the spec by reference, so ' + + 'the entry-shape refusal must arrive with it', + ).toBe(true); + // Positive control: the same payload with a well-formed entry is accepted, + // so the refusal above is about the null entry and not about the envelope. + expect( + ListViewSchema.safeParse({ + type: 'list-view', + objectName: 'test_object', + grouping: { fields: [{ field: 'active' }] }, + }).success, + ).toBe(true); + }); +}); diff --git a/packages/plugin-grid/src/useGroupedData.ts b/packages/plugin-grid/src/useGroupedData.ts index bf7ddc295..eeb8efc45 100644 --- a/packages/plugin-grid/src/useGroupedData.ts +++ b/packages/plugin-grid/src/useGroupedData.ts @@ -207,6 +207,64 @@ function compareGroups(a: string, b: string, order: 'asc' | 'desc'): number { return order === 'desc' ? -cmp : cmp; } +/** + * One entry of the spec's `grouping.fields[]` as AUTHORED — `field` plus the + * optional `order` / `collapsed`. + * + * ⚠️ NOT the same type as `@object-ui/components`' `GroupingFieldEntry`, which + * is the grouping EDITOR's fully-populated value shape and requires `order` + * and `collapsed`. This one is `z.input` of the spec schema, where both carry + * defaults and are therefore optional, so the two are structurally different + * and each keeps its own name (objectui#6273 — one authority per exported + * name; a shared spelling for two shapes is the collision that gate exists to + * catch). + */ +export type UsableGroupingField = NonNullable[number]; + +/** + * The `grouping.fields[]` entries a grid can actually group by (objectui#7217). + * + * ## Why this exists + * + * `grouping` is authored JSON and reaches the renderer unparsed — `ObjectGrid` + * reads `schema.grouping` straight off its props and `@object-ui/core`'s + * `validateSchema` is structural and never looks at the key. A `null` hole in + * the array (a trailing comma, a sparse generator, an agent-written block) was + * therefore dereferenced twice with no guard: once by `ObjectGrid`'s + * `groupValueFormatter` memo and once by this hook's `buildLevel`. Both threw + * `TypeError: Cannot read properties of null (reading 'field')` and took the + * whole grid down during render. + * + * ## The admission rule is the harvester's, deliberately + * + * An entry is usable when it is an object carrying a non-empty string `field` + * — exactly the entries `collectGroupingFieldRefs` (`@object-ui/core`) harvests + * into the projection. Keeping the two sets equal is the point: an entry the + * grid grouped by but the projection ignored would be fetched as `undefined` + * on every row and bucket every record into one `(empty)` group, which is the + * silent wrong answer objectui#7179 closed. This is a defensive normalizer, + * NOT a lenient alias — no off-spec spelling is taught to mean anything here; + * unusable entries are dropped, never coerced. + * + * ## Dropping the entry, not the grouping + * + * One bad entry must not flatten a working grouped view: the usable entries + * still group, at the levels they still occupy. + * + * @param fields - `grouping.fields` in any authored state. + * @returns The usable entries, in order, with their `order` / `collapsed` + * intact — the harvester answers with field NAMES, which is why this cannot + * simply route through it. + */ +export function usableGroupingFields(fields: unknown): UsableGroupingField[] { + if (!Array.isArray(fields)) return []; + return fields.filter((entry): entry is UsableGroupingField => { + if (entry === null || typeof entry !== 'object') return false; + const name = (entry as { field?: unknown }).field; + return typeof name === 'string' && name.trim() !== ''; + }); +} + /** * Hook that groups a flat data array by the fields specified in GroupingConfig. * @@ -227,14 +285,19 @@ export function useGroupedData( aggregations?: AggregationConfig[], formatValue?: GroupValueFormatter, ): UseGroupedDataResult { - const fields = config?.fields; - const isGrouped = !!(fields && fields.length > 0); + // [objectui#7217] The SAME normalized list `ObjectGrid`'s formatter memo + // reads. Memoized on the raw array rather than on `config`: hosts rebuild + // the `{ grouping }` object literal every render, so keying on `config` + // would hand `groups` a fresh array identity on every render. + const rawFields = config?.fields; + const fields = useMemo(() => usableGroupingFields(rawFields), [rawFields]); + const isGrouped = fields.length > 0; // Track which group keys have been explicitly toggled by the user. const [toggledKeys, setToggledKeys] = useState>({}); const groups: GroupEntry[] = useMemo(() => { - if (!isGrouped || !fields) return []; + if (!isGrouped) return []; /** * Recursively build a tree of groups for the slice of rows at the current @@ -308,7 +371,7 @@ export function useGroupedData( const lastSegment = key.split('__').pop() || ''; const depthMatch = /^(\d+):/.exec(lastSegment); const depth = depthMatch ? Number(depthMatch[1]) : 0; - const fieldDefault = !!fields?.[depth]?.collapsed; + const fieldDefault = !!fields[depth]?.collapsed; return { ...prev, [key]: prev[key] !== undefined ? !prev[key] : !fieldDefault,