From 729ec315e2f9041697db0ddb253a9b50aae821be Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 10 Aug 2026 15:19:39 -0700 Subject: [PATCH 1/3] refactor(ui): make illegal DataTable prop combinations unrepresentable DataTable accepted any mix of its 40-odd props and rejected the incoherent combinations at runtime, from a validator that threw during the first render. A caller only found out it had wired server sorting without a `sorting` prop when the page blew up in front of them. Split the public prop type into mode-keyed unions instead, so the compiler rejects those combinations at the call site. `validateDataTableConfig` and `DataTableConfigError` go away; the component body reads an unchanged flat `DataTableResolvedProps`, which every union member is assignable to, so there is no narrowing inside it. All 44 existing call sites typecheck against the new union unchanged, which `next build` covers. That build only typechecks the app module graph, so the prop type itself needed a gate of its own: `npm run test:types` runs vitest's typecheck mode over `*.test-d.tsx`, and the unit workflow now runs it. The four guards deleted from `DataTable.test.tsx` come back there as compile-time assertions, and loosening the union back to the flat shape fails all five. --- .github/workflows/test-litellm-ui-unit.yml | 5 ++ ui/litellm-dashboard/package.json | 1 + .../shared/DataTable/DataTable.test-d.tsx | 67 ++++++++++++++++ .../shared/DataTable/DataTable.test.tsx | 41 ---------- .../components/shared/DataTable/DataTable.tsx | 68 ++++------------ .../DataTable/DataTableRowSelection.test.tsx | 14 +--- .../src/components/shared/DataTable/index.ts | 3 +- .../src/components/shared/DataTable/types.ts | 80 ++++++++++++++++++- ui/litellm-dashboard/vitest.config.ts | 5 ++ 9 files changed, 175 insertions(+), 109 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test-d.tsx diff --git a/.github/workflows/test-litellm-ui-unit.yml b/.github/workflows/test-litellm-ui-unit.yml index 8f2199017d9..69cbc082d98 100644 --- a/.github/workflows/test-litellm-ui-unit.yml +++ b/.github/workflows/test-litellm-ui-unit.yml @@ -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" diff --git a/ui/litellm-dashboard/package.json b/ui/litellm-dashboard/package.json index 3953b41a2c2..62bf5fff4b4 100644 --- a/ui/litellm-dashboard/package.json +++ b/ui/litellm-dashboard/package.json @@ -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 .", diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test-d.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test-d.tsx new file mode 100644 index 00000000000..7bbd4f918cd --- /dev/null +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test-d.tsx @@ -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[] = []; +const sorting: SortingState = [{ id: "name", desc: false }]; +const pagination: PaginationState = { pageIndex: 0, pageSize: 10 }; +const rowSelection: RowSelectionState = { r1: true }; +const noop = () => {}; + +export const uncontrolled = ; + +export const controlled = ( + +); + +export const clientSortingWithServerPagination = ( + +); + +// @ts-expect-error sortingMode="server" requires `sorting` and `onSortingChange` +export const serverSortingWithoutState = ; + +// @ts-expect-error paginationMode="server" requires `pagination`, `onPaginationChange` and `rowCount` +export const serverPaginationWithoutState = ; + +// @ts-expect-error filterMode="server" requires `columnFilters` and `onColumnFiltersChange` +export const serverFilteringWithoutState = ; + +export const bothSortingSources = ( + // @ts-expect-error `defaultSorting` seeds uncontrolled sorting, so it cannot pair with a controlled `sorting` + +); + +export const selectionWithoutHandler = ( + // @ts-expect-error a controlled `rowSelection` needs `onRowSelectionChange` or selection changes are dropped + +); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx index 8555a10c326..3afdd2849ae 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx @@ -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()).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()).toThrow( - /sortingMode='server'/, - ); - spy.mockRestore(); - }); - - it("throws when server pagination is missing required props", () => { - const spy = vi.spyOn(console, "error").mockImplementation(() => {}); - expect(() => render()).toThrow( - /paginationMode='server'/, - ); - spy.mockRestore(); - }); - - it("throws when both defaultSorting and sorting are provided", () => { - const spy = vi.spyOn(console, "error").mockImplementation(() => {}); - expect(() => - render( - , - ), - ).toThrow(/defaultSorting/); - spy.mockRestore(); - }); -}); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx index 8cd0e25dfc4..2f47887d01f 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx @@ -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( - props: DataTableProps, -): 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(column: ColumnDef): string | undefined { if ("id" in column && typeof column.id === "string") { return column.id; @@ -442,7 +409,9 @@ function useControllable( return { value: internal, onChange: setInternal }; } -function useDataTableInstance(props: DataTableProps): Table { +function useDataTableInstance( + props: DataTableResolvedProps, +): Table { const { data, columns, @@ -532,14 +501,7 @@ function useDataTableInstance(props: DataTablePro } export function DataTable(props: DataTableProps) { - // Validate once at construction so a misconfig surfaces immediately instead of on every render. - useState(() => { - const errors = validateDataTableConfig(props); - if (errors.length > 0) { - throw new DataTableConfigError(errors); - } - return null; - }); + const resolved: DataTableResolvedProps = props; const { isLoading = false, @@ -559,9 +521,9 @@ export function DataTable(props: DataTableProps { await user.click(rowBox("m1")); expect(selectedCount()).toHaveTextContent("1"); }); - - it("rejects controlled rowSelection without onRowSelectionChange", () => { - const errors = validateDataTableConfig({ 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({ data, columns })).toHaveLength(0); - }); }); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/index.ts b/ui/litellm-dashboard/src/components/shared/DataTable/index.ts index 62ddd1b0742..39a887ba948 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/index.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/index.ts @@ -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, diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts index dd578f4df45..58f0d2c2539 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts @@ -21,7 +21,11 @@ export type DataTableSize = "compact" | "default"; export type ColumnPinnedSide = "left" | "right"; export type DataTableSkeletonShape = "text" | "twoLine" | "badge" | "chips" | "meter"; -export interface DataTableProps { +/** + * The flat shape the component reads internally. Every member of the public + * `DataTableProps` union is assignable to it, so the component body needs no narrowing. + */ +export interface DataTableResolvedProps { data: TData[]; columns: ColumnDef[]; getRowId?: (row: TData, index: number, parent?: Row) => string; @@ -81,3 +85,77 @@ export interface DataTableProps { paginationSlot?: (table: Table) => React.ReactNode; footer?: (table: Table) => React.ReactNode; } + +type DataTableBaseProps = Omit< + DataTableResolvedProps, + | "sortingMode" + | "sorting" + | "onSortingChange" + | "defaultSorting" + | "paginationMode" + | "pagination" + | "onPaginationChange" + | "rowCount" + | "filterMode" + | "columnFilters" + | "onColumnFiltersChange" + | "defaultColumnFilters" + | "rowSelection" + | "onRowSelectionChange" +>; + +type SortingProps = + | { + sorting: SortingState; + onSortingChange: OnChangeFn; + sortingMode?: SortingMode; + defaultSorting?: never; + } + | { + sortingMode?: Exclude; + sorting?: never; + onSortingChange?: never; + defaultSorting?: SortingState; + }; + +type PaginationProps = + | { + paginationMode: "server"; + pagination: PaginationState; + onPaginationChange: OnChangeFn; + rowCount: number; + } + | { + paginationMode?: Exclude; + pagination?: PaginationState; + onPaginationChange?: OnChangeFn; + rowCount?: number; + }; + +type FilterProps = + | { + columnFilters: ColumnFiltersState; + onColumnFiltersChange: OnChangeFn; + filterMode?: FilterMode; + defaultColumnFilters?: never; + } + | { + filterMode?: Exclude; + columnFilters?: never; + onColumnFiltersChange?: never; + defaultColumnFilters?: ColumnFiltersState; + }; + +type RowSelectionProps = + | { rowSelection: RowSelectionState; onRowSelectionChange: OnChangeFn } + | { rowSelection?: never; onRowSelectionChange?: OnChangeFn }; + +/** + * Public prop type. The mode-keyed unions make the combinations + * `validateDataTableConfig` used to reject at runtime unrepresentable instead. + */ +export type DataTableProps = DataTableBaseProps & + SortingProps & + PaginationProps & + FilterProps & + RowSelectionProps; diff --git a/ui/litellm-dashboard/vitest.config.ts b/ui/litellm-dashboard/vitest.config.ts index 19af41ab8ed..469f7fa3520 100644 --- a/ui/litellm-dashboard/vitest.config.ts +++ b/ui/litellm-dashboard/vitest.config.ts @@ -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: { From 3ced0e433ad107593fbfca53d0fad7a681587439 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 10 Aug 2026 15:21:14 -0700 Subject: [PATCH 2/3] refactor(ui): drop a doc comment naming the deleted DataTable validator --- ui/litellm-dashboard/src/components/shared/DataTable/types.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts index 58f0d2c2539..8c4d7cbb161 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts @@ -150,10 +150,6 @@ type RowSelectionProps = | { rowSelection: RowSelectionState; onRowSelectionChange: OnChangeFn } | { rowSelection?: never; onRowSelectionChange?: OnChangeFn }; -/** - * Public prop type. The mode-keyed unions make the combinations - * `validateDataTableConfig` used to reject at runtime unrepresentable instead. - */ export type DataTableProps = DataTableBaseProps & SortingProps & PaginationProps & From 4f1c92b9751af9cbdf21d19f4ca70f2742d4d781 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 10 Aug 2026 15:42:09 -0700 Subject: [PATCH 3/3] fix(ui): register type-test files as knip entry points knip derives its entry points from vitest's `test.include`, which does not cover `test.typecheck.include`, so the new `*.test-d.tsx` file read as an unused file and failed the lint job. Declare the glob as an entry point. Also drops the doc comment on `DataTableResolvedProps`; the rationale for the resolved/public split belongs in the commit that introduced it. --- ui/litellm-dashboard/knip.json | 2 +- ui/litellm-dashboard/src/components/shared/DataTable/types.ts | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/ui/litellm-dashboard/knip.json b/ui/litellm-dashboard/knip.json index 48b39e8122d..f6cd8ace112 100644 --- a/ui/litellm-dashboard/knip.json +++ b/ui/litellm-dashboard/knip.json @@ -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": [ diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts index 8c4d7cbb161..c767a0a64c0 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/types.ts +++ b/ui/litellm-dashboard/src/components/shared/DataTable/types.ts @@ -21,10 +21,6 @@ export type DataTableSize = "compact" | "default"; export type ColumnPinnedSide = "left" | "right"; export type DataTableSkeletonShape = "text" | "twoLine" | "badge" | "chips" | "meter"; -/** - * The flat shape the component reads internally. Every member of the public - * `DataTableProps` union is assignable to it, so the component body needs no narrowing. - */ export interface DataTableResolvedProps { data: TData[]; columns: ColumnDef[];