Merge pull request #36470 from BerriAI/litellm_/standard-lists-api-d1dc4a

refactor(ui): make illegal DataTable prop combinations unrepresentable
This commit is contained in:
yuneng-jiang 2026-08-10 16:45:06 -07:00 committed by GitHub
commit b1369b56cc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 168 additions and 110 deletions

View file

@ -42,6 +42,11 @@ jobs:
- name: Install dependencies
run: npm ci
- name: Run UI type tests (Vitest)
env:
CI: "true"
run: npm run test:types
- name: Run UI unit tests (Vitest)
env:
CI: "true"

View file

@ -1,6 +1,6 @@
{
"$schema": "https://unpkg.com/knip@5/schema.json",
"entry": ["scripts/**/*.{ts,mjs}", "src/components/ui/**/*.{ts,tsx}"],
"entry": ["scripts/**/*.{ts,mjs}", "src/components/ui/**/*.{ts,tsx}", "src/**/*.test-d.{ts,tsx}"],
"project": ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}", "scripts/**/*.{ts,mjs}"],
"ignore": ["src/lib/http/schema.d.ts"],
"ignoreDependencies": [

View file

@ -10,6 +10,7 @@
"lint": "eslint .",
"test": "vitest",
"test:dot": "vitest --reporter=dot",
"test:types": "vitest --run --typecheck.only",
"test:watch": "vitest -w",
"test:coverage": "vitest run --coverage",
"format": "prettier --write .",

View file

@ -0,0 +1,67 @@
import type { ColumnDef, PaginationState, RowSelectionState, SortingState } from "@tanstack/react-table";
import { DataTable } from "./DataTable";
interface Row {
id: string;
name: string;
}
const data: Row[] = [];
const columns: ColumnDef<Row, unknown>[] = [];
const sorting: SortingState = [{ id: "name", desc: false }];
const pagination: PaginationState = { pageIndex: 0, pageSize: 10 };
const rowSelection: RowSelectionState = { r1: true };
const noop = () => {};
export const uncontrolled = <DataTable data={data} columns={columns} defaultSorting={sorting} />;
export const controlled = (
<DataTable
data={data}
columns={columns}
sortingMode="server"
sorting={sorting}
onSortingChange={noop}
paginationMode="server"
pagination={pagination}
onPaginationChange={noop}
rowCount={0}
filterMode="server"
columnFilters={[]}
onColumnFiltersChange={noop}
rowSelection={rowSelection}
onRowSelectionChange={noop}
/>
);
export const clientSortingWithServerPagination = (
<DataTable
data={data}
columns={columns}
defaultSorting={sorting}
paginationMode="server"
pagination={pagination}
onPaginationChange={noop}
rowCount={0}
/>
);
// @ts-expect-error sortingMode="server" requires `sorting` and `onSortingChange`
export const serverSortingWithoutState = <DataTable data={data} columns={columns} sortingMode="server" />;
// @ts-expect-error paginationMode="server" requires `pagination`, `onPaginationChange` and `rowCount`
export const serverPaginationWithoutState = <DataTable data={data} columns={columns} paginationMode="server" />;
// @ts-expect-error filterMode="server" requires `columnFilters` and `onColumnFiltersChange`
export const serverFilteringWithoutState = <DataTable data={data} columns={columns} filterMode="server" />;
export const bothSortingSources = (
// @ts-expect-error `defaultSorting` seeds uncontrolled sorting, so it cannot pair with a controlled `sorting`
<DataTable data={data} columns={columns} defaultSorting={sorting} sorting={sorting} onSortingChange={noop} />
);
export const selectionWithoutHandler = (
// @ts-expect-error a controlled `rowSelection` needs `onRowSelectionChange` or selection changes are dropped
<DataTable data={data} columns={columns} rowSelection={rowSelection} />
);

View file

@ -337,14 +337,6 @@ describe("DataTable filtering", () => {
);
expect(names()).toEqual(["Charlie", "Alice", "Bob"]);
});
it("throws when server filtering is missing required props", () => {
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
expect(() => render(<DataTable data={[]} columns={filterableColumns} filterMode="server" />)).toThrow(
/filterMode='server'/,
);
spy.mockRestore();
});
});
describe("DataTable loading", () => {
@ -664,36 +656,3 @@ describe("DataTable layout", () => {
expect(container.querySelector("thead")?.className).not.toContain("bg-background");
});
});
describe("DataTable misconfiguration guards", () => {
it("throws when server sorting is missing required props", () => {
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
expect(() => render(<DataTable data={[]} columns={nameCellColumns} sortingMode="server" />)).toThrow(
/sortingMode='server'/,
);
spy.mockRestore();
});
it("throws when server pagination is missing required props", () => {
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
expect(() => render(<DataTable data={[]} columns={nameCellColumns} paginationMode="server" />)).toThrow(
/paginationMode='server'/,
);
spy.mockRestore();
});
it("throws when both defaultSorting and sorting are provided", () => {
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
expect(() =>
render(
<DataTable
data={[]}
columns={nameCellColumns}
defaultSorting={[{ id: "name", desc: false }]}
sorting={[{ id: "name", desc: false }]}
/>,
),
).toThrow(/defaultSorting/);
spy.mockRestore();
});
});

View file

@ -42,7 +42,15 @@ import { cn } from "@/lib/cva.config";
import "./columnMeta";
import { DataTablePagination, DEFAULT_PAGE_SIZE_OPTIONS } from "./DataTablePagination";
import type { ColumnPinnedSide, DataTableProps, DataTableSize, FilterMode, PaginationMode, SortingMode } from "./types";
import type {
ColumnPinnedSide,
DataTableProps,
DataTableResolvedProps,
DataTableSize,
FilterMode,
PaginationMode,
SortingMode,
} from "./types";
const INTERACTIVE_SELECTOR = "button, a, input, select, textarea, [role=checkbox], [data-row-click-exempt]";
@ -64,47 +72,6 @@ const FILL_CLASSES = {
const NO_FILL_CLASSES = { outer: "", frame: "", body: "", header: "" } as const;
export class DataTableConfigError extends Error {
constructor(messages: readonly string[]) {
super(`DataTable misconfiguration:\n- ${messages.join("\n- ")}`);
this.name = "DataTableConfigError";
}
}
export function validateDataTableConfig<TData extends RowData, TValue>(
props: DataTableProps<TData, TValue>,
): readonly string[] {
const serverSortingIncomplete =
props.sortingMode === "server" && (props.sorting === undefined || props.onSortingChange === undefined);
const serverPaginationPropsMissing =
props.pagination === undefined || props.onPaginationChange === undefined || props.rowCount === undefined;
const serverPaginationIncomplete = props.paginationMode === "server" && serverPaginationPropsMissing;
const serverFilteringIncomplete =
props.filterMode === "server" && (props.columnFilters === undefined || props.onColumnFiltersChange === undefined);
const bothSortingSources = props.defaultSorting !== undefined && props.sorting !== undefined;
const bothFilterSources = props.defaultColumnFilters !== undefined && props.columnFilters !== undefined;
const controlledSelectionIncomplete = props.rowSelection !== undefined && props.onRowSelectionChange === undefined;
return [
serverSortingIncomplete ? "sortingMode='server' requires both `sorting` and `onSortingChange`." : null,
serverPaginationIncomplete
? "paginationMode='server' requires `pagination`, `onPaginationChange`, and `rowCount`."
: null,
serverFilteringIncomplete ? "filterMode='server' requires both `columnFilters` and `onColumnFiltersChange`." : null,
bothSortingSources ? "Provide either `defaultSorting` (uncontrolled) or `sorting` (controlled), not both." : null,
bothFilterSources
? "Provide either `defaultColumnFilters` (uncontrolled) or `columnFilters` (controlled), not both."
: null,
controlledSelectionIncomplete
? "Controlled `rowSelection` requires `onRowSelectionChange`; without it selection changes are dropped."
: null,
].filter((message): message is string => message !== null);
}
function columnDefId<TData, TValue>(column: ColumnDef<TData, TValue>): string | undefined {
if ("id" in column && typeof column.id === "string") {
return column.id;
@ -442,7 +409,9 @@ function useControllable<T>(
return { value: internal, onChange: setInternal };
}
function useDataTableInstance<TData extends RowData, TValue>(props: DataTableProps<TData, TValue>): Table<TData> {
function useDataTableInstance<TData extends RowData, TValue>(
props: DataTableResolvedProps<TData, TValue>,
): Table<TData> {
const {
data,
columns,
@ -532,14 +501,7 @@ function useDataTableInstance<TData extends RowData, TValue>(props: DataTablePro
}
export function DataTable<TData extends RowData, TValue>(props: DataTableProps<TData, TValue>) {
// Validate once at construction so a misconfig surfaces immediately instead of on every render.
useState<null>(() => {
const errors = validateDataTableConfig(props);
if (errors.length > 0) {
throw new DataTableConfigError(errors);
}
return null;
});
const resolved: DataTableResolvedProps<TData, TValue> = props;
const {
isLoading = false,
@ -559,9 +521,9 @@ export function DataTable<TData extends RowData, TValue>(props: DataTableProps<T
toolbar,
paginationSlot,
footer,
} = props;
} = resolved;
const table = useDataTableInstance(props);
const table = useDataTableInstance(resolved);
const rows = table.getRowModel().rows;
const visibleColumnCount = table.getVisibleLeafColumns().length;

View file

@ -4,7 +4,7 @@ import userEvent from "@testing-library/user-event";
import { useState } from "react";
import { describe, expect, it } from "vitest";
import { createSelectionColumn, DataTable, validateDataTableConfig } from "./index";
import { createSelectionColumn, DataTable } from "./index";
interface Model {
id: string;
@ -122,16 +122,4 @@ describe("DataTable row selection", () => {
await user.click(rowBox("m1"));
expect(selectedCount()).toHaveTextContent("1");
});
it("rejects controlled rowSelection without onRowSelectionChange", () => {
const errors = validateDataTableConfig<Model, unknown>({ data, columns, rowSelection: { m1: true } });
expect(errors).toContain(
"Controlled `rowSelection` requires `onRowSelectionChange`; without it selection changes are dropped.",
);
});
it("does not complain when selection is left uncontrolled", () => {
expect(validateDataTableConfig<Model, unknown>({ data, columns })).toHaveLength(0);
});
});

View file

@ -1,6 +1,6 @@
import "./columnMeta";
export { DataTable, DataTableConfigError, validateDataTableConfig } from "./DataTable";
export { DataTable } from "./DataTable";
export { DataTableFilterDrawer, DataTableFilterField, type FilterDraft } from "./DataTableFilterDrawer";
export { DataTablePagination, DEFAULT_PAGE_SIZE_OPTIONS } from "./DataTablePagination";
export { createSelectionColumn } from "./DataTableSelectionColumn";
@ -17,6 +17,7 @@ export type {
ColumnPinnedSide,
ColumnResizeMode,
DataTableProps,
DataTableResolvedProps,
DataTableSize,
FilterMode,
PaginationMode,

View file

@ -21,7 +21,7 @@ export type DataTableSize = "compact" | "default";
export type ColumnPinnedSide = "left" | "right";
export type DataTableSkeletonShape = "text" | "twoLine" | "badge" | "chips" | "meter";
export interface DataTableProps<TData extends RowData, TValue> {
export interface DataTableResolvedProps<TData extends RowData, TValue> {
data: TData[];
columns: ColumnDef<TData, TValue>[];
getRowId?: (row: TData, index: number, parent?: Row<TData>) => string;
@ -81,3 +81,73 @@ export interface DataTableProps<TData extends RowData, TValue> {
paginationSlot?: (table: Table<TData>) => React.ReactNode;
footer?: (table: Table<TData>) => React.ReactNode;
}
type DataTableBaseProps<TData extends RowData, TValue> = Omit<
DataTableResolvedProps<TData, TValue>,
| "sortingMode"
| "sorting"
| "onSortingChange"
| "defaultSorting"
| "paginationMode"
| "pagination"
| "onPaginationChange"
| "rowCount"
| "filterMode"
| "columnFilters"
| "onColumnFiltersChange"
| "defaultColumnFilters"
| "rowSelection"
| "onRowSelectionChange"
>;
type SortingProps =
| {
sorting: SortingState;
onSortingChange: OnChangeFn<SortingState>;
sortingMode?: SortingMode;
defaultSorting?: never;
}
| {
sortingMode?: Exclude<SortingMode, "server">;
sorting?: never;
onSortingChange?: never;
defaultSorting?: SortingState;
};
type PaginationProps =
| {
paginationMode: "server";
pagination: PaginationState;
onPaginationChange: OnChangeFn<PaginationState>;
rowCount: number;
}
| {
paginationMode?: Exclude<PaginationMode, "server">;
pagination?: PaginationState;
onPaginationChange?: OnChangeFn<PaginationState>;
rowCount?: number;
};
type FilterProps =
| {
columnFilters: ColumnFiltersState;
onColumnFiltersChange: OnChangeFn<ColumnFiltersState>;
filterMode?: FilterMode;
defaultColumnFilters?: never;
}
| {
filterMode?: Exclude<FilterMode, "server">;
columnFilters?: never;
onColumnFiltersChange?: never;
defaultColumnFilters?: ColumnFiltersState;
};
type RowSelectionProps =
| { rowSelection: RowSelectionState; onRowSelectionChange: OnChangeFn<RowSelectionState> }
| { rowSelection?: never; onRowSelectionChange?: OnChangeFn<RowSelectionState> };
export type DataTableProps<TData extends RowData, TValue> = DataTableBaseProps<TData, TValue> &
SortingProps &
PaginationProps &
FilterProps &
RowSelectionProps;

View file

@ -29,6 +29,7 @@ const config: ViteUserConfig = {
exclude: [
"**/*.d.ts",
"**/*.test.*",
"**/*.test-d.*",
"**/*.spec.*",
"tests/**",
@ -45,6 +46,10 @@ const config: ViteUserConfig = {
},
exclude: ["node_modules/**"],
include: ["src/**/*.test.ts", "src/**/*.test.tsx", "tests/**/*.test.ts", "tests/**/*.test.tsx"],
typecheck: {
include: ["src/**/*.test-d.ts", "src/**/*.test-d.tsx"],
ignoreSourceErrors: true,
},
},
resolve: {
alias: {