diff --git a/packages/core/core.api.md b/packages/core/core.api.md index c1a7d6db..629e8e8b 100644 --- a/packages/core/core.api.md +++ b/packages/core/core.api.md @@ -723,6 +723,7 @@ export interface PretableGridUiCore; + readonly status?: "checking" | "editing"; }) => void; // (undocumented) readonly cancelEdit: () => void; @@ -757,7 +758,7 @@ export interface PretableGridUiCore void; // (undocumented) - readonly setEditStatus: (status: "editing" | "validating" | "saving" | "error", error?: string) => void; + readonly setEditStatus: (status: PretableOpenEditStatus, error?: string) => void; // (undocumented) readonly setFocus: (focus: PretableIndexedFocusState) => void; readonly setRowSelection: (rows: PretableRowSelectionState) => void; @@ -859,7 +860,7 @@ export type PretableIndexedEditingState readonly rowId: TRowId; readonly columnId: TColumnId; readonly value: ColumnValueOf; - readonly status: "editing" | "validating" | "saving" | "error"; + readonly status: PretableEditStatus; readonly error?: string; }; }[ColumnIdOf]; @@ -993,6 +994,9 @@ export interface PretableMutationResult { readonly updated: number; } +// @public +export type PretableOpenEditStatus = "editing" | "validating" | "saving" | "error"; + // @public export type PretableProcessingAuthority = "engine" | "external"; diff --git a/packages/core/src/public_api.ts b/packages/core/src/public_api.ts index 0879848f..68069be1 100644 --- a/packages/core/src/public_api.ts +++ b/packages/core/src/public_api.ts @@ -89,6 +89,7 @@ export type { PretableMutationResult, PretableQueryFor, PretableQueryTransition, + PretableOpenEditStatus, PretableProcessingAuthority, PretableProcessingOptions, PretableResultMeta, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 32278f18..ff293ff7 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -18,6 +18,7 @@ export type { PretableFocusState, PretableMatchingTotal, PretableMoveFocusOptions, + PretableOpenEditStatus, PretableProcessingAuthority, PretableProcessingOptions, PretableResultMeta, diff --git a/packages/grid-core/src/create-grid-ui-core.ts b/packages/grid-core/src/create-grid-ui-core.ts index 7275cb95..c3969d45 100644 --- a/packages/grid-core/src/create-grid-ui-core.ts +++ b/packages/grid-core/src/create-grid-ui-core.ts @@ -798,12 +798,13 @@ export function createGridUiCore< readonly rowId: TRowId; readonly columnId: TEditColumnId; readonly value: ColumnValueOf; + readonly status?: "checking" | "editing"; }) { const editing = Object.freeze({ rowId: input.rowId, columnId: input.columnId, value: input.value, - status: "editing" as const, + status: input.status ?? "editing", }) as PretableIndexedEditingState; command(() => { const snapshot = observed?.snapshot; diff --git a/packages/grid-core/src/index.ts b/packages/grid-core/src/index.ts index c694210d..da92c150 100644 --- a/packages/grid-core/src/index.ts +++ b/packages/grid-core/src/index.ts @@ -43,6 +43,7 @@ export type { PretableHeaderRowRef, PretableMatchingTotal, PretableMoveFocusOptions, + PretableOpenEditStatus, PretableProcessingAuthority, PretableProcessingOptions, PretableResultMeta, diff --git a/packages/grid-core/src/types.ts b/packages/grid-core/src/types.ts index 40d3d9af..b794b849 100644 --- a/packages/grid-core/src/types.ts +++ b/packages/grid-core/src/types.ts @@ -22,9 +22,33 @@ export type PretableRow = object; */ export type PretableSortDirection = "asc" | "desc" | null; +/** + * Phase of a cell edit that is already OPEN — i.e. every phase except + * `"checking"`, which is the pre-authorization phase only `beginEdit` can + * enter and which nothing can return to. + * + * This union had no name and was spelled out in four places. It is written as + * literals rather than `Exclude` for two + * reasons: an `Exclude` whose second argument stops naming a member silently + * widens to the whole union instead of failing, and the docs guard can only + * pin a table to a union whose members the API report states outright. + * + * The cost is that the two unions could drift, so they are held together by + * assertion instead of by construction — see the `toEqualTypeOf` pair in + * `packages/react/src/__tests__/narrowed-literal-unions.test.ts`, which fails + * if either list changes without the other. + * + * @public + */ +export type PretableOpenEditStatus = + "editing" | "validating" | "saving" | "error"; + /** * Phase of an in-progress cell edit. * + * `"checking"` is the pre-authorization phase an async `editable` predicate + * runs under; the rest are {@link PretableOpenEditStatus}. + * * @public */ export type PretableEditStatus = @@ -768,7 +792,7 @@ export type PretableIndexedEditingState< readonly rowId: TRowId; readonly columnId: TColumnId; readonly value: ColumnValueOf; - readonly status: "editing" | "validating" | "saving" | "error"; + readonly status: PretableEditStatus; readonly error?: string; }; }[ColumnIdOf]; @@ -872,15 +896,23 @@ export interface PretableGridUiCore< * carries a value, and only the schema says what type the value in a given * column has. A drawn-but-unschema'd column (a checkbox gutter, a group * label) has no value to edit, and this signature is what says so. + * + * `status` is the phase the session OPENS in and defaults to `"editing"`. + * `"checking"` is the only other legal entry phase: it is what an async + * `editable` predicate runs under, and it exists so the editor can render + * read-only and `aria-busy` while the answer is in flight. Nothing can + * return to it, which is why {@link PretableOpenEditStatus} — the type of + * every later transition — excludes it. */ readonly beginEdit: >(input: { readonly rowId: TRowId; readonly columnId: TEditColumnId; readonly value: ColumnValueOf; + readonly status?: "checking" | "editing"; }) => void; readonly setEditDraft: (value: unknown) => void; readonly setEditStatus: ( - status: "editing" | "validating" | "saving" | "error", + status: PretableOpenEditStatus, error?: string, ) => void; readonly cancelEdit: () => void; diff --git a/packages/react/react.api.md b/packages/react/react.api.md index 7be749be..9fb99f7e 100644 --- a/packages/react/react.api.md +++ b/packages/react/react.api.md @@ -1200,6 +1200,7 @@ export interface PretableGridUiCore; + readonly status?: "checking" | "editing"; }) => void; // (undocumented) readonly cancelEdit: () => void; @@ -1234,7 +1235,7 @@ export interface PretableGridUiCore void; // (undocumented) - readonly setEditStatus: (status: "editing" | "validating" | "saving" | "error", error?: string) => void; + readonly setEditStatus: (status: PretableOpenEditStatus, error?: string) => void; // (undocumented) readonly setFocus: (focus: PretableIndexedFocusState) => void; readonly setRowSelection: (rows: PretableRowSelectionState) => void; @@ -1353,7 +1354,7 @@ export type PretableIndexedEditingState readonly rowId: TRowId; readonly columnId: TColumnId; readonly value: ColumnValueOf; - readonly status: "editing" | "validating" | "saving" | "error"; + readonly status: PretableEditStatus; readonly error?: string; }; }[ColumnIdOf]; @@ -1547,6 +1548,9 @@ export interface PretableMutationResult { readonly updated: number; } +// @public +export type PretableOpenEditStatus = "editing" | "validating" | "saving" | "error"; + // @public export type PretablePresentationColumns = TColumns extends readonly (infer TColumn)[] ? readonly (TColumn extends { readonly id: infer TId extends string; @@ -1688,9 +1692,10 @@ export type PretableReactGrid; + readonly status?: "checking" | "editing"; }) => void; readonly setEditDraft: (value: unknown) => void; - readonly setEditStatus: (status: "editing" | "validating" | "saving" | "error", error?: string) => void; + readonly setEditStatus: (status: PretableOpenEditStatus, error?: string) => void; readonly cancelEdit: () => void; readonly setColumnWidth: (columnId: TColumnId, width: number) => void; readonly setColumnPinned: (columnId: TColumnId, pinned: "left" | "right" | null) => void; diff --git a/packages/react/src/__tests__/csv.test.ts b/packages/react/src/__tests__/csv.test.ts index 1ff5c673..c7e97ab0 100644 --- a/packages/react/src/__tests__/csv.test.ts +++ b/packages/react/src/__tests__/csv.test.ts @@ -472,13 +472,18 @@ describe("serializeCsv vouches on the value, not the declaration", () => { }); describe("serializeCsv reports rows hidden by collapsed groups", () => { - async function grouped(collapse: boolean) { + async function groupedModel() { const model = createLocalRowModel({ rows, columns: modelColumns }); await model.setQuery({ filters: [], sort: [], rowGroups: [{ columnId: "a" }], }).finished; + return model; + } + + async function grouped(collapse: boolean) { + const model = await groupedModel(); if (collapse) model.collapseAll(); return serializeCsv({ rowModelSnapshot: model.getState().snapshot, @@ -503,6 +508,47 @@ describe("serializeCsv reports rows hidden by collapsed groups", () => { expect(file?.complete).toBe(false); expect(file!.rowCount).toBeLessThan((await grouped(false))!.rowCount); }); + + // The two clauses of `hidesCollapsedRows` are checked separately, because a + // suite that only ever exercises expand-all vs collapse-all cannot tell + // `default.kind !== "expanded"` from `overrideCount > 0` — either clause + // alone would carry both cases above. + async function withExpansion( + mutate: (model: Awaited>) => void, + ) { + const model = await groupedModel(); + mutate(model); + return serializeCsv({ + rowModelSnapshot: model.getState().snapshot, + columns: [ + { id: GROUP_COLUMN_ID, header: "Group" }, + { id: "n", header: "N", type: "number" }, + ], + scope: "all", + options: { bom: false }, + }); + } + + it("is INCOMPLETE for a non-'expanded' default even with no overrides", async () => { + const file = await withExpansion((model) => { + model.setExpansionDefault({ kind: "through-depth", depth: 0 }); + }); + expect(file?.complete).toBe(false); + expect(file?.omissions.map((o) => o.kind)).toEqual(["collapsed-groups"]); + }); + + it("is INCOMPLETE for an expanded default carrying an override", async () => { + const file = await withExpansion((model) => { + const snapshot = model.getState().snapshot; + const group = snapshot.rowAt(0); + if (group?.kind !== "group") throw new Error("expected a group row"); + model.setGroupExpanded(group.groupId, false); + }); + expect(file?.complete).toBe(false); + expect(file?.omissions).toEqual([ + { kind: "collapsed-groups", expansionOverrideCount: 1 }, + ]); + }); }); describe("serializeCsv pins each formula trigger individually", () => { diff --git a/packages/react/src/__tests__/narrowed-literal-unions.test.ts b/packages/react/src/__tests__/narrowed-literal-unions.test.ts new file mode 100644 index 00000000..ea91c584 --- /dev/null +++ b/packages/react/src/__tests__/narrowed-literal-unions.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, expectTypeOf, it } from "vitest"; + +import type { + PretableEditStatus, + PretableExpansionState, + PretableIndexedEditingState, + PretableOpenEditStatus, +} from "@pretable/core"; + +import { hidesCollapsedRows } from "../csv"; + +/** + * Two copies of a named union had been widened to `string` and then compared + * against string literals. A `string` makes every such comparison unchecked: + * renaming the phase, or typo-ing the literal, compiles and silently changes + * behavior. These assertions are the compiler-enforced half of the fix — the + * behavioral half lives in `pretable-surface-editing.test.tsx` (the edit + * lifecycle) and `csv.test.ts` (export completeness). + * + * Every `@ts-expect-error` below is load-bearing in BOTH directions: it fails + * as an unused directive the moment the field widens back to `string`. + */ + +type Columns = readonly [ + { + readonly id: "name"; + readonly accessor: (row: { id: string; name: string }) => string; + }, +]; +type EditingStatus = NonNullable< + PretableIndexedEditingState +>["status"]; + +describe("edit status is a checked union", () => { + it("names the post-authorization phases", () => { + // The union that appeared unnamed in four places, now named once. + expectTypeOf().toEqualTypeOf< + "editing" | "validating" | "saving" | "error" + >(); + // THE ANTI-DRIFT CLAUSE. Both unions spell their members out, because the + // docs guard pins a table to the members the API report states and an + // `Exclude<>` hides them. That leaves two lists that could disagree, so + // their relationship is asserted here instead: `PretableEditStatus` is + // exactly `"checking"` plus the open phases, no more and no less. + expectTypeOf().toEqualTypeOf< + "checking" | PretableOpenEditStatus + >(); + expectTypeOf< + Exclude + >().toEqualTypeOf(); + }); + + it("keeps 'checking' reachable in the observable editing state", () => { + // THE BUG. The store's editing state excluded `"checking"` while + // `useCellEditController` asked `beginEdit` to open in it and three + // consumers (`useEditorField`'s pending set, `BooleanCellControl`, the + // controller's own gate) compared against it — comparisons no value could + // ever satisfy. The surface facade's `status: string` is what kept the + // compiler quiet about all of them. + expectTypeOf<"checking">().toMatchTypeOf(); + expectTypeOf().toEqualTypeOf(); + }); + + it("rejects a status the union cannot produce", () => { + const status = "editing" as EditingStatus; + // @ts-expect-error -- a typo'd phase has no overlap with the union. Unused + // (and therefore itself an error) if `status` ever widens to `string`. + const typo: boolean = status === "editting"; + void typo; + }); + + it("keeps 'checking' out of the post-authorization transitions", () => { + // Nothing can return to `"checking"`; only `beginEdit` can enter it. That + // is the entire reason `PretableOpenEditStatus` exists as a second name. + // @ts-expect-error -- `"checking"` is not an open-edit transition. + const notATransition: PretableOpenEditStatus = "checking"; + void notATransition; + }); +}); + +describe("expansion kind is a checked union", () => { + it("accepts the real expansion state", () => { + const expanded: PretableExpansionState = { + default: { kind: "expanded" }, + overrideCount: 0, + }; + expectTypeOf(expanded).toMatchTypeOf< + Parameters[0] + >(); + expect(hidesCollapsedRows(expanded)).toBe(false); + }); + + it("rejects a typo'd expansion kind", () => { + // The comparison inside `hidesCollapsedRows` is what decides whether a + // CSV export reports `complete: true` or a `collapsed-groups` omission. + // Before the narrowing the parameter was `{ default: { kind: string } }`, + // so this call compiled and this directive went unused. + hidesCollapsedRows({ + // @ts-expect-error -- "expandedd" is not a PretableExpansionDefault kind. + default: { kind: "expandedd" }, + overrideCount: 0, + }); + }); + + it("rejects an expansion shape the row model never publishes", () => { + hidesCollapsedRows({ + // @ts-expect-error -- the through-depth variant also carries `depth`. + default: { kind: "through-depth" }, + overrideCount: 0, + }); + }); +}); diff --git a/packages/react/src/__tests__/pretable-surface-editing.test.tsx b/packages/react/src/__tests__/pretable-surface-editing.test.tsx index 823ca835..275c7fe6 100644 --- a/packages/react/src/__tests__/pretable-surface-editing.test.tsx +++ b/packages/react/src/__tests__/pretable-surface-editing.test.tsx @@ -289,4 +289,85 @@ describe("PretableSurface editing", () => { await flush(); expect(onRowChange).toHaveBeenCalledTimes(2); }); + + // Drives the REAL surface, not the controller's stub grid. The stub in + // `use-cell-edit-controller.test.ts` honours the entry status it is handed, + // so it reported `"checking"` while the surface silently dropped it and + // opened every edit in `"editing"` — a green test over a phase the shipped + // grid could not reach. + it("holds an async-editable edit in 'checking' until the predicate answers", async () => { + let allow!: (v: boolean) => void; + const onRowChange = vi.fn(); + render( + + ariaLabel="people" + columns={[ + { + id: "name", + header: "Name", + editable: () => new Promise((r) => (allow = r)), + }, + ]} + rows={ROWS} + getRowId={(r) => r.id} + viewportHeight={300} + onRowChange={onRowChange} + />, + ); + const cell = firstNameCell(); + fireEvent.click(cell); + fireEvent.keyDown(cell, { key: "Enter" }); + + // Predicate still in flight: the editor is mounted but inert. + expect(cell).toHaveAttribute("data-pretable-edit-status", "checking"); + const box = screen.getByRole("textbox"); + expect(box).toHaveAttribute("aria-busy", "true"); + expect(box).toHaveAttribute("readonly"); + + // ...and a blur cannot commit a draft the grid has not agreed to accept. + // `useEditorField` gates blur-commit on `status === "editing"`, which is + // exactly the comparison the widened `string` left unchecked. + fireEvent.blur(box); + await flush(); + expect(onRowChange).not.toHaveBeenCalled(); + expect(screen.getByRole("textbox")).toBeInTheDocument(); + + allow(true); + await flush(); + expect(firstNameCell()).toHaveAttribute( + "data-pretable-edit-status", + "editing", + ); + const open = screen.getByRole("textbox"); + expect(open).not.toHaveAttribute("aria-busy"); + expect(open).not.toHaveAttribute("readonly"); + }); + + it("closes without opening when async editable resolves false", async () => { + let allow!: (v: boolean) => void; + render( + + ariaLabel="people" + columns={[ + { + id: "name", + header: "Name", + editable: () => new Promise((r) => (allow = r)), + }, + ]} + rows={ROWS} + getRowId={(r) => r.id} + viewportHeight={300} + onRowChange={vi.fn()} + />, + ); + const cell = firstNameCell(); + fireEvent.click(cell); + fireEvent.keyDown(cell, { key: "Enter" }); + expect(cell).toHaveAttribute("data-pretable-edit-status", "checking"); + allow(false); + await flush(); + expect(screen.queryByRole("textbox")).not.toBeInTheDocument(); + expect(firstNameCell()).not.toHaveAttribute("data-pretable-edit-status"); + }); }); diff --git a/packages/react/src/csv.ts b/packages/react/src/csv.ts index acec79c9..128d0c27 100644 --- a/packages/react/src/csv.ts +++ b/packages/react/src/csv.ts @@ -15,6 +15,7 @@ import { GROUP_COLUMN_ID } from "@pretable/core"; import type { ColumnType, + PretableExpansionState, PretableRow, PretableRowId, PretableRowModelSnapshot, @@ -344,11 +345,21 @@ export const DEFAULT_CSV_OPTIONS = { * (Both AG Grid and MUI export collapsed children instead. Doing that needs a * traversal the snapshot does not expose today, so this reports honestly rather * than guessing — see the follow-up noted in the spec.) + * + * Takes {@link PretableExpansionState} itself, NOT a structural copy whose + * `kind` is `string`. The copy compiled the `!== "expanded"` test against an + * unchecked literal: renaming or typo-ing the expansion kind would have left + * this returning `true` for a fully-expanded grid, i.e. stamping every export + * `-PARTIAL` with a `collapsed-groups` omission it does not have — or, for the + * inverse typo, reporting `complete: true` on a file that lost rows. The + * completeness contract is the whole point of this module, so the comparison + * has to be checked. + * + * @internal Exported for the type test that pins that narrowing. */ -function hidesCollapsedRows(expansion: { - readonly default: { readonly kind: string }; - readonly overrideCount: number; -}): boolean { +export function hidesCollapsedRows( + expansion: Readonly, +): boolean { return expansion.default.kind !== "expanded" || expansion.overrideCount > 0; } diff --git a/packages/react/src/pretable-model.ts b/packages/react/src/pretable-model.ts index aa799974..4fa5e75c 100644 --- a/packages/react/src/pretable-model.ts +++ b/packages/react/src/pretable-model.ts @@ -16,6 +16,7 @@ import type { PretableIndexedCellSelectionSummary, PretableIndexedFocusMovement, PretableIndexedMoveFocusOptions, + PretableOpenEditStatus, PretableRowId, PretableRowModel, PretableRowModelSnapshot, @@ -127,10 +128,11 @@ export type PretableReactGrid< readonly rowId: TRowId; readonly columnId: TEditColumnId; readonly value: ColumnValueOf; + readonly status?: "checking" | "editing"; }) => void; readonly setEditDraft: (value: unknown) => void; readonly setEditStatus: ( - status: "editing" | "validating" | "saving" | "error", + status: PretableOpenEditStatus, error?: string, ) => void; readonly cancelEdit: () => void; diff --git a/packages/react/src/pretable-surface.tsx b/packages/react/src/pretable-surface.tsx index 5a968b9a..c4f6cac6 100644 --- a/packages/react/src/pretable-surface.tsx +++ b/packages/react/src/pretable-surface.tsx @@ -38,6 +38,7 @@ import type { PretableResultMeta, PretableVisibleRowRef, PretableViewportState, + PretableEditStatus, PretableIndexedDatasetRowSpan, PretableIndexedFocusRef, PretableIndexedSelectionState, @@ -395,7 +396,13 @@ interface SurfaceFacade { readonly rowId: PretableRowId; readonly columnId: string; readonly draft: unknown; - readonly status: string; + // NOT `string`. Every consumer of this field compares it against a + // literal, and a `string` makes every one of those comparisons + // unchecked — a renamed phase or a typo'd literal would compile and + // silently change behavior. Widened only in `columnId`, where the + // schema/drawn vocabularies genuinely differ; the status vocabulary is + // the engine's, verbatim. + readonly status: PretableEditStatus; readonly error?: string; } | null; readonly rowGroups: readonly string[]; @@ -2646,6 +2653,12 @@ export function PretableSurface< // tuple. A draft is genuinely `unknown` at this point (it comes from // an editor's DOM value), so there is no narrower honest target. value: edit?.draft as never, + // Forwarded, not dropped. `useCellEditController` opens an async + // `editable` check in `"checking"` so the editor renders read-only + // and `aria-busy` until the predicate answers; swallowing it here + // left the field fully interactive — and blur-committable — while + // the check was still in flight. + ...(edit?.status === undefined ? {} : { status: edit.status }), }); }, setEditDraft: indexedGrid.setEditDraft, diff --git a/packages/react/src/public_api.ts b/packages/react/src/public_api.ts index 69949f5d..91aae096 100644 --- a/packages/react/src/public_api.ts +++ b/packages/react/src/public_api.ts @@ -208,6 +208,7 @@ export type { PretableGroupId, PretableMoveFocusOptions, PretableMatchingTotal, + PretableOpenEditStatus, PretableProcessingAuthority, PretableProcessingOptions, PretableResultMeta,