diff --git a/CHANGELOG.md b/CHANGELOG.md index a3a23ac..4d3e453 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,48 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.3.1] - 2026-09-03 + +### Fixed + +- A malformed filter snapshot no longer brings the grid down. `filtering`'s + `hydrate` cast the slice straight to `FilterModel`, so an operator that + belonged to another kind, a `set` whose `values` was not a list, or a + condition missing the value its operator needs all threw while the pipeline + was reading them - a throw inside a `$derived` costs the render pass, not one + column. Six of the seven shapes measured against 1.3.0 threw. The slice is + now sanitized at the boundary: what cannot be read is dropped, and a column + left with nothing stops filtering, which shows more rows rather than none. +- The filter predicates no longer assume the condition handed to them is well + formed. An unknown operator, a missing value, a `set` whose values are not a + list and a kind nothing knows now pass every row instead of throwing. This is + the layer that covers `applyFilterModel`, which an app can call with a model + it read back from its own storage. +- `sorting`'s `hydrate` checked `Array.isArray` and then cast, so a null entry + in the array threw on `columnId`. Entries that do not name a column and a + direction are now dropped. +- A column width that the layout cannot draw no longer destroys the grid. A + `NaN` or `Infinity` width reached the CSS custom property as `NaNpx`, which + makes `grid-template-columns` invalid at computed-value time: the browser + dropped the declaration, every column folded into one track and the cells + stacked down the page, with nothing thrown and nothing logged. Such a width + is now refused where it becomes CSS, so no route reaches the property: a + container measured as `NaN`, a definition written with `width: NaN` or + `flex: NaN`, a snapshot carrying one, and `setWidth`/`setWidths`, where + `clamp` and `Math.round` had been passing `NaN` straight through. A track + that cannot be drawn falls back to the column's minimum. +- Two rows sharing an id no longer fail silently. The row index keeps the last + row for a repeated id, so an edit addressed to the row the user opened was + written to the other one and nothing said so. A development build now names + the ids that collided. Production pays one integer comparison for the check + and nothing more; working out which ids repeated happens only in a build that + will print it. +- `setState` no longer throws on a corrupt `columns` slice. An `order` that was + not an array reached `.filter` and threw inside the caller's own call. Values + are now read as carefully as keys already were: only string ids order the + columns, only real booleans hide a column or fold a group, and a `columns` + slice that is not an object is taken as nothing at all. + ## [1.3.0] - 2026-08-24 ### Added @@ -872,6 +914,7 @@ full table. - Performance budgets in CI as coarse regression ceilings, measured best-of-3 so a loaded machine does not fail a build. +[1.3.1]: https://github.com/ndlabdev/sv5ui-datagrid/releases/tag/v1.3.1 [1.3.0]: https://github.com/ndlabdev/sv5ui-datagrid/releases/tag/v1.3.0 [1.2.0]: https://github.com/ndlabdev/sv5ui-datagrid/releases/tag/v1.2.0 [1.1.0]: https://github.com/ndlabdev/sv5ui-datagrid/releases/tag/v1.1.0 diff --git a/package.json b/package.json index 759a7fa..9ae37c8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@sv5ui/datagrid", - "version": "1.3.0", + "version": "1.3.1", "description": "A high-performance data grid for Svelte 5, built on sv5ui", "author": "ndlabdev", "license": "MIT", diff --git a/src/lib/core/columns/column-model.svelte.ts b/src/lib/core/columns/column-model.svelte.ts index 31404ca..1e946ac 100644 --- a/src/lib/core/columns/column-model.svelte.ts +++ b/src/lib/core/columns/column-model.svelte.ts @@ -241,6 +241,11 @@ export class ColumnModel { setWidth(id: string, width: number): number { const column = this.get(id) if (!column) return 0 + // `clamp` and `Math.round` both carry `NaN` through, and a width the + // layout cannot draw takes `grid-template-columns` down with it rather + // than making one column wrong. Refuse it and say what the width still + // is. + if (!Number.isFinite(width)) return this.widthOf(id) ?? 0 const clamped = Math.round(clamp(width, column.minWidth, column.maxWidth)) this.widthOverrides = { ...this.widthOverrides, [id]: clamped } return clamped @@ -250,7 +255,8 @@ export class ColumnModel { const next = { ...this.widthOverrides } for (const [id, width] of Object.entries(widths)) { const column = this.get(id) - if (column) next[id] = Math.round(clamp(width, column.minWidth, column.maxWidth)) + if (!column || !Number.isFinite(width)) continue + next[id] = Math.round(clamp(width, column.minWidth, column.maxWidth)) } this.widthOverrides = next } diff --git a/src/lib/core/columns/column-model.test.ts b/src/lib/core/columns/column-model.test.ts index 8bac05b..03c0b18 100644 --- a/src/lib/core/columns/column-model.test.ts +++ b/src/lib/core/columns/column-model.test.ts @@ -85,3 +85,33 @@ describe('ColumnModel runtime state', () => { expect(model.visible.map((column) => column.id)).toEqual(['x', 'y', 'z']) }) }) + +describe('a width the layout could not draw', () => { + it('refuses it and reports the width the column still has', () => { + const model = createModel() + expect(model.setWidth('a', Number.NaN)).toBe(100) + expect(model.setWidth('a', Number.POSITIVE_INFINITY)).toBe(100) + expect(model.widthOverrides).toEqual({}) + expect(model.widthOf('a')).toBe(100) + }) + + it('reports zero for a column that is not there, as before', () => { + expect(createModel().setWidth('missing', Number.NaN)).toBe(0) + }) + + it('skips it in a batch and keeps the rest', () => { + const model = createModel() + model.setWidths({ a: Number.NaN, c: 150, d: Number.NEGATIVE_INFINITY }) + + expect(model.widthOverrides).toEqual({ c: 150 }) + expect(model.widthOf('a')).toBe(100) + expect(model.widthOf('d')).toBe(90) + }) + + it('leaves a width already set alone', () => { + const model = createModel() + model.setWidth('a', 250) + expect(model.setWidth('a', Number.NaN)).toBe(250) + expect(model.widthOf('a')).toBe(250) + }) +}) diff --git a/src/lib/core/columns/column-sizing.test.ts b/src/lib/core/columns/column-sizing.test.ts index 0ad1185..184225d 100644 --- a/src/lib/core/columns/column-sizing.test.ts +++ b/src/lib/core/columns/column-sizing.test.ts @@ -108,3 +108,59 @@ describe('toStyleString', () => { expect(toStyleString({ '--a': '1px', '--b': '2px' })).toBe('--a: 1px; --b: 2px') }) }) + +describe('a track the browser could not parse', () => { + // Every one of these produced `NaNpx`, which makes grid-template-columns + // invalid at computed-value time: the declaration is dropped, the columns + // fold into one track and the cells stack down the page. + it('never writes a non-finite width, whichever way it arrived', () => { + const fromOverride = buildColumnCssVars([createColumnState({ id: 'a' })], null, null, { + a: Number.NaN + }) + const fromDefinition = buildColumnCssVars([ + createColumnState({ id: 'a', width: Number.NaN }) + ]) + const fromResolved = buildColumnCssVars([createColumnState({ id: 'a' })], [Number.NaN]) + + for (const vars of [fromOverride, fromDefinition, fromResolved]) { + expect(vars['--dg-col-a-w']).not.toContain('NaN') + } + }) + + it('falls back to the column minimum rather than to nothing', () => { + const vars = buildColumnCssVars( + [createColumnState({ id: 'a', minWidth: 64 })], + [Number.NaN] + ) + expect(vars['--dg-col-a-w']).toBe('64px') + }) + + it('reads an unusable flex weight as one', () => { + expect(columnTrackSize(createColumnState({ id: 'a', flex: Number.NaN }))).toBe( + 'minmax(40px, 1fr)' + ) + }) + + it('reads an unusable minimum as a width it can draw', () => { + expect(columnTrackSize(createColumnState({ id: 'a', minWidth: Number.NaN }))).toBe( + 'minmax(100px, 1fr)' + ) + }) + + it('writes a pin offset it can draw', () => { + const vars = buildColumnCssVars([createColumnState({ id: 'a', pinned: 'left' })], [100], { + a: Number.NaN + }) + expect(vars['--dg-col-a-pin']).toBe('0px') + }) + + it('leaves a healthy track exactly as it was', () => { + expect(columnTrackSize(createColumnState({ id: 'a', width: 120 }))).toBe('120px') + expect(columnTrackSize(createColumnState({ id: 'a', flex: 2, minWidth: 80 }))).toBe( + 'minmax(80px, 2fr)' + ) + expect(buildColumnCssVars([createColumnState({ id: 'a' })], [220])['--dg-col-a-w']).toBe( + '220px' + ) + }) +}) diff --git a/src/lib/core/columns/column-sizing.ts b/src/lib/core/columns/column-sizing.ts index d5eba54..249ce12 100644 --- a/src/lib/core/columns/column-sizing.ts +++ b/src/lib/core/columns/column-sizing.ts @@ -42,6 +42,27 @@ export function createColumnState( export type WidthOverrides = Record +/** What a track falls back to when nothing usable is left to fall back on. */ +const LAST_RESORT_WIDTH = 100 + +/** + * The last gate before a number becomes CSS. A non-finite one reaches the + * custom property as `NaNpx`, and `grid-template-columns: var(...)` is then + * invalid at computed-value time: the browser drops the whole declaration, + * every column folds into a single track and the cells stack down the page, + * with nothing thrown and nothing logged. + * + * The width setters and the snapshot boundary each refuse such a value on the + * way in. This is here because they are not the only way in - a container + * measured as `NaN` and a column definition written with `width: NaN` reach + * the same line without passing either - and because a track that is merely + * the wrong size is a far smaller failure than a grid that will not lay out. + */ +function px(value: number | undefined, fallback: number): string { + if (typeof value === 'number' && Number.isFinite(value)) return `${value}px` + return `${Number.isFinite(fallback) ? fallback : LAST_RESORT_WIDTH}px` +} + function effectiveWidth( column: ColumnState, overrides: WidthOverrides @@ -55,9 +76,10 @@ export function columnTrackSize( ): string { const width = effectiveWidth(column, overrides) if (width !== undefined) { - return `${clamp(width, column.minWidth, column.maxWidth)}px` + return px(clamp(width, column.minWidth, column.maxWidth), column.minWidth) } - return `minmax(${column.minWidth}px, ${column.flex ?? 1}fr)` + const flex = typeof column.flex === 'number' && Number.isFinite(column.flex) ? column.flex : 1 + return `minmax(${px(column.minWidth, LAST_RESORT_WIDTH)}, ${flex}fr)` } export function trackWidthEstimates( @@ -106,10 +128,10 @@ export function buildColumnCssVars( const vars: Record = {} visible.forEach((column, index) => { vars[column.cssVar] = resolvedWidths - ? `${resolvedWidths[index]}px` + ? px(resolvedWidths[index], column.minWidth) : columnTrackSize(column, overrides) if (column.pinned && pins) { - vars[column.pinVar] = `${pins[column.id] ?? 0}px` + vars[column.pinVar] = px(pins[column.id], 0) } }) vars['--dg-grid-template'] = visible.map((column) => `var(${column.cssVar})`).join(' ') diff --git a/src/lib/core/grid/row-node.test.ts b/src/lib/core/grid/row-node.test.ts new file mode 100644 index 0000000..2fd9174 --- /dev/null +++ b/src/lib/core/grid/row-node.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { buildRowNodes, nodesById } from './row-node.js' + +interface Row { + id: string + name: string +} + +const build = (rows: Row[]) => buildRowNodes(rows, (row) => row.id) + +afterEach(() => { + vi.restoreAllMocks() +}) + +describe('nodesById', () => { + it('keys every row by its id', () => { + const index = nodesById( + build([ + { id: 'a', name: 'first' }, + { id: 'b', name: 'second' } + ]) + ) + + expect(index.size).toBe(2) + expect(index.get('a')?.row.name).toBe('first') + }) + + it('says nothing when the ids are unique', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + nodesById( + build([ + { id: 'a', name: 'first' }, + { id: 'b', name: 'second' } + ]) + ) + + expect(warn).not.toHaveBeenCalled() + }) + + it('names the ids that collided', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + nodesById( + build([ + { id: 'a', name: 'first' }, + { id: 'a', name: 'second' }, + { id: 'b', name: 'third' } + ]) + ) + + expect(warn).toHaveBeenCalledTimes(1) + const message = warn.mock.calls[0]?.[0] as string + expect(message).toContain('getRowId') + expect(message).toContain('a') + expect(message).toContain('3 rows share 2 ids') + }) + + it('caps the list rather than printing every id of a broken set', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const rows = Array.from({ length: 20 }, (_, i) => ({ id: `dup-${i % 10}`, name: `r${i}` })) + nodesById(build(rows)) + + const message = warn.mock.calls[0]?.[0] as string + expect(message).toContain('and 5 more') + }) + + it('keeps the last row for a repeated id, which is why it warns', () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + const index = nodesById( + build([ + { id: 'a', name: 'first' }, + { id: 'a', name: 'second' } + ]) + ) + + expect(index.size).toBe(1) + expect(index.get('a')?.row.name).toBe('second') + }) +}) diff --git a/src/lib/core/grid/row-node.ts b/src/lib/core/grid/row-node.ts index 67e3619..e1b990a 100644 --- a/src/lib/core/grid/row-node.ts +++ b/src/lib/core/grid/row-node.ts @@ -11,6 +11,42 @@ export function buildRowNodes( return nodes } +const DUPLICATES_SHOWN = 5 + +/** + * Two rows sharing an id is the app's bug, but the grid fails at it silently + * and in the worst possible way: the map keeps the last row for an id, and an + * edit addressed to the row the user opened is written to the other one. Said + * out loud in development, where it is cheap, rather than left to be found in + * the data later. + * + * Repeated on every rebuild on purpose. Nothing here remembers what it already + * reported, so no state outlives a grid, and the data stays wrong until it is + * fixed. + */ +function warnDuplicateIds(nodes: RowNode[], unique: number): void { + const seen = new Set() + const repeated = new Set() + for (const node of nodes) { + if (seen.has(node.id)) repeated.add(node.id) + else seen.add(node.id) + } + + const shown = [...repeated].slice(0, DUPLICATES_SHOWN).join(', ') + const rest = repeated.size - DUPLICATES_SHOWN + // The one console statement in the library. A silent wrong write is worse + // than a line in a development console, and there is no other channel: the + // grid has no logger, and an error would take down an app over data it can + // still draw. + // eslint-disable-next-line no-console + console.warn( + `[sv5ui-datagrid] getRowId returned the same id for more than one row: ${shown}` + + (rest > 0 ? ` and ${rest} more` : '') + + `. ${nodes.length} rows share ${unique} ids, so an edit or a selection meant for ` + + 'one of them will land on another. Give every row an id of its own.' + ) +} + /** * Rebuilt whole by the derived that owns it, so a plain Map suffices. Filled * by hand rather than from `map`, which would allocate a second array of pairs @@ -19,6 +55,10 @@ export function buildRowNodes( export function nodesById(nodes: RowNode[]): ReadonlyMap> { const index = new Map>() for (const node of nodes) index.set(node.id, node) + // The whole check, in production: one integer against another. Working out + // which ids collided costs a second pass, and only a build that will print + // it pays for that. + if (index.size !== nodes.length && import.meta.env?.DEV) warnDuplicateIds(nodes, index.size) return index } diff --git a/src/lib/core/grid/snapshot.test.ts b/src/lib/core/grid/snapshot.test.ts index ea654ea..172a4fb 100644 --- a/src/lib/core/grid/snapshot.test.ts +++ b/src/lib/core/grid/snapshot.test.ts @@ -146,3 +146,64 @@ describe('folded groups in a snapshot', () => { ).toEqual({}) }) }) + +describe('a column snapshot that has been outside the grid', () => { + const knownIds = ['a', 'b'] + + it('ignores an order that is not a list', () => { + expect(resolveColumnSnapshot({ order: 'a' }, knownIds).orderIds).toEqual([]) + expect(resolveColumnSnapshot({ order: 42 }, knownIds).orderIds).toEqual([]) + }) + + it('keeps only the string ids in an order', () => { + expect(resolveColumnSnapshot({ order: [null, 7, 'b'] }, knownIds).orderIds).toEqual([ + 'b', + 'a' + ]) + }) + + it('drops a width the layout could not draw', () => { + const widths = { + a: Number.NaN, + b: Number.POSITIVE_INFINITY + } + expect(resolveColumnSnapshot({ widths }, knownIds).widthOverrides).toEqual({}) + }) + + it('drops a width that is not a number at all', () => { + expect( + resolveColumnSnapshot({ widths: { a: '120', b: null } }, knownIds).widthOverrides + ).toEqual({}) + }) + + it('keeps a width it can draw, negative included, since the model clamps it', () => { + expect( + resolveColumnSnapshot({ widths: { a: 120, b: -5 } }, knownIds).widthOverrides + ).toEqual({ a: 120, b: -5 }) + }) + + it('keeps only real booleans for hidden and collapsed', () => { + const stored = { hidden: { a: 'yes', b: true }, collapsed: { g: 1 } } + const resolved = resolveColumnSnapshot(stored, knownIds, ['g']) + expect(resolved.hiddenOverrides).toEqual({ b: true }) + expect(resolved.collapsedGroups).toEqual({}) + }) + + it('reads an unpinnable side as unpinned rather than dropping the entry', () => { + expect( + resolveColumnSnapshot({ pinned: { a: 'middle' } }, knownIds).pinnedOverrides + ).toEqual({ a: null }) + }) + + it('takes a columns slice that is not an object as nothing at all', () => { + for (const stored of ['nope', 42, ['a'], null, undefined]) { + expect(resolveColumnSnapshot(stored, knownIds)).toEqual({ + orderIds: [], + widthOverrides: {}, + hiddenOverrides: {}, + pinnedOverrides: {}, + collapsedGroups: {} + }) + } + }) +}) diff --git a/src/lib/core/grid/snapshot.ts b/src/lib/core/grid/snapshot.ts index f4e3b50..0a66146 100644 --- a/src/lib/core/grid/snapshot.ts +++ b/src/lib/core/grid/snapshot.ts @@ -16,8 +16,39 @@ export interface ColumnSnapshotSource { const DENSITIES: Density[] = ['compact', 'standard', 'comfortable'] -function pruneRecord(record: Record, known: Set): Record { - return Object.fromEntries(Object.entries(record).filter(([id]) => known.has(id))) +/** + * Keys pruned against the columns that exist, values against what the model + * can actually hold. A snapshot has been outside the grid - a share link, + * `localStorage`, anything handed back to `setState` - so a key surviving is + * no evidence its value did. + */ +function pruneRecord( + record: unknown, + known: Set, + keep: (value: unknown) => value is T +): Record { + if (typeof record !== 'object' || record === null) return {} + + const kept: Record = {} + for (const [id, value] of Object.entries(record)) { + if (known.has(id) && keep(value)) kept[id] = value + } + return kept +} + +function isBoolean(value: unknown): value is boolean { + return typeof value === 'boolean' +} + +/** + * A width the layout can draw. `NaN` and `Infinity` are the ones that matter: + * they reach the CSS custom property as `NaNpx`, which makes + * `grid-template-columns` invalid at computed-value time and collapses every + * column into one track. Nothing throws on the way there, so an unusable width + * has to be refused rather than reported. + */ +function isDrawableWidth(value: unknown): value is number { + return typeof value === 'number' && Number.isFinite(value) } function isEmpty(value: object): boolean { @@ -41,8 +72,9 @@ export function buildColumnSnapshot( * Groups are named apart from columns, since a folded group is keyed by the * group's own id and no column carries it. */ -function resolveOrder(stored: string[] | undefined, known: Set, knownIds: string[]) { - const order = (stored ?? []).filter((id) => known.has(id)) +function resolveOrder(stored: unknown, known: Set, knownIds: string[]) { + if (!Array.isArray(stored)) return [] + const order = stored.filter((id): id is string => typeof id === 'string' && known.has(id)) if (order.length === 0) return [] // A column that appeared since the snapshot was written goes last rather // than disappearing for want of a place in the order. @@ -50,29 +82,41 @@ function resolveOrder(stored: string[] | undefined, known: Set, knownIds } export function resolveColumnSnapshot( - stored: GridSnapshot['columns'], + stored: unknown, knownIds: string[], knownGroupIds: string[] = [] ): ColumnSnapshotSource { const known = new Set(knownIds) - const columns = stored ?? {} + // Read as unknown fields rather than as `GridSnapshot['columns']`. Naming + // the type here would be the same promise that let a broken snapshot in: + // every field below is checked where it is used, so none of them may claim + // a shape on the way past. + const columns: Record = + typeof stored === 'object' && stored !== null && !Array.isArray(stored) + ? (stored as Record) + : {} return { orderIds: resolveOrder(columns.order, known, knownIds), - widthOverrides: pruneRecord(columns.widths ?? {}, known), - hiddenOverrides: pruneRecord(columns.hidden ?? {}, known), - pinnedOverrides: prunePinned(columns.pinned ?? {}, known), - collapsedGroups: pruneRecord(columns.collapsed ?? {}, new Set(knownGroupIds)) + widthOverrides: pruneRecord(columns.widths, known, isDrawableWidth), + hiddenOverrides: pruneRecord(columns.hidden, known, isBoolean), + pinnedOverrides: prunePinned(columns.pinned, known), + collapsedGroups: pruneRecord(columns.collapsed, new Set(knownGroupIds), isBoolean) } } -/** Values too, not just keys: a corrupt entry must not reach the model. */ -function prunePinned( - stored: Record, - known: Set -): Record { +/** + * Values too, not just keys: a corrupt entry must not reach the model. A side + * that cannot be read is kept as an explicit `null` rather than dropped, so a + * column the user unpinned stays unpinned instead of springing back to the + * side its definition names. + */ +function prunePinned(stored: unknown, known: Set): Record { + if (typeof stored !== 'object' || stored === null) return {} + const pinned: Record = {} - for (const [id, side] of Object.entries(pruneRecord(stored, known))) { + for (const [id, side] of Object.entries(stored)) { + if (!known.has(id)) continue pinned[id] = side === 'left' || side === 'right' ? side : null } return pinned diff --git a/src/lib/features/filtering/filter-predicates.ts b/src/lib/features/filtering/filter-predicates.ts index 1923082..49d3a56 100644 --- a/src/lib/features/filtering/filter-predicates.ts +++ b/src/lib/features/filtering/filter-predicates.ts @@ -13,6 +13,14 @@ import { getCellValue, isBlank } from '../../core/utils/index.js' import { setKeyOf } from './distinct-values.js' import { normalizeFilterEntry } from './filter-model.js' +/** + * What an unreadable condition does: nothing. `sanitizeFilterModel` is the + * boundary that should have dropped it, and this is the layer that keeps a + * condition arriving some other way out of the pipeline's `$derived`, where a + * throw costs the whole render pass rather than one column. + */ +const PASSES = (): boolean => true + export function filterTypeOf(def: ColumnDef): FilterType | null { if (def.filter === false || def.filter === undefined) return null return typeof def.filter === 'string' ? def.filter : def.filter.type @@ -24,18 +32,27 @@ function customPredicateOf( return typeof def.filter === 'object' ? def.filter.predicate : undefined } +/** Folding once here keeps `toLowerCase` out of the per-row loop. */ +function foldedQuery(filter: Extract): { + query: string + read: (value: unknown) => string +} { + const fold = filter.caseSensitive + ? (text: string) => text + : (text: string) => text.toLowerCase() + return { + query: fold(String(filter.value ?? '').trim()), + read: (value: unknown) => fold(String(value)) + } +} + function textPredicate( filter: Extract ): (value: unknown) => boolean { if (filter.op === 'blank') return (value) => isBlank(value) if (filter.op === 'notBlank') return (value) => !isBlank(value) - // Folding once here keeps `toLowerCase` out of the per-row loop. - const fold = filter.caseSensitive - ? (text: string) => text - : (text: string) => text.toLowerCase() - const query = fold(filter.value.trim()) - const read = (value: unknown) => fold(String(value)) + const { query, read } = foldedQuery(filter) switch (filter.op) { case 'equals': @@ -52,12 +69,16 @@ function textPredicate( return (value) => !isBlank(value) && read(value).includes(query) case 'notContains': return (value) => isBlank(value) || !read(value).includes(query) + default: + return PASSES } } +type NumberComparator = (value: number, target: number) => boolean + const numberComparators: Record< Exclude, - (value: number, target: number) => boolean + NumberComparator > = { eq: (value, target) => value === target, neq: (value, target) => value !== target, @@ -83,7 +104,12 @@ function numberPredicate( return numeric >= target && numeric <= to } } - const compare = numberComparators[filter.op] + // Widened on purpose: the key is exhaustive by type, and an operator that + // reached here from outside the type system is exactly what this catches. + const compare = (numberComparators as Partial>)[ + filter.op + ] + if (!compare) return PASSES return (value) => !isBlank(value) && compare(Number(value), target) } @@ -134,10 +160,13 @@ function datePredicate( const day = toEpochDay(value) return day >= target && day <= to } + default: + return PASSES } } function setPredicate(filter: Extract): (value: unknown) => boolean { + if (!Array.isArray(filter.values)) return PASSES // Keyed on both sides by the same function the value list is built with, or // the cell holding a Date is never the entry the user picked, and a filter // that came back through a snapshot is never the one that went in. @@ -163,6 +192,8 @@ export function valuePredicateFor(filter: ColumnFilter): (value: unknown) => boo return setPredicate(filter) case 'boolean': return booleanPredicate(filter) + default: + return PASSES } } @@ -174,7 +205,8 @@ function entryPredicate( const { join, conditions } = normalizeFilterEntry(entry) const custom = customPredicateOf(def) - const tests = conditions.map((condition) => { + const listed = Array.isArray(conditions) ? conditions : [] + const tests = listed.map((condition) => { if (custom) return (value: unknown, row: TRow) => custom(value, row, condition) const predicate = valuePredicateFor(condition) return (value: unknown) => predicate(value) diff --git a/src/lib/features/filtering/filter-sanitize.test.ts b/src/lib/features/filtering/filter-sanitize.test.ts new file mode 100644 index 0000000..2e51889 --- /dev/null +++ b/src/lib/features/filtering/filter-sanitize.test.ts @@ -0,0 +1,224 @@ +import { describe, expect, it } from 'vitest' +import { createDataGrid } from '../../core/grid/index.js' +import type { ColumnDef, ColumnFilter } from '../../core/types/index.js' +import { filtering, getFiltering, sanitizeFilterModel } from './index.js' + +interface Row { + id: number + total: number + name: string + when: string + ok: boolean +} + +const columns: ColumnDef[] = [ + { id: 'total', header: 'Total', filter: 'number' }, + { id: 'name', header: 'Name', filter: 'text' }, + { id: 'when', header: 'When', filter: 'date' }, + { id: 'ok', header: 'Ok', filter: 'boolean' } +] + +const data: Row[] = [ + { id: 1, total: 10, name: 'alpha', when: '2026-01-02', ok: true }, + { id: 2, total: 20, name: 'beta', when: '2026-02-02', ok: false } +] + +function gridWith(columnId: string, filter: unknown) { + const grid = createDataGrid({ + columns, + data, + getRowId: (row) => String(row.id), + features: [filtering()] + }) + grid.setState({ + version: 1, + features: { filtering: { quick: '', columns: { [columnId]: filter } } } + } as never) + return grid +} + +describe('sanitizeFilterModel', () => { + it('keeps a model the editor could have built', () => { + const model = sanitizeFilterModel({ + quick: 'ph', + columns: { + total: { kind: 'number', op: 'gt', value: 15 }, + name: { kind: 'text', op: 'contains', value: 'al', caseSensitive: true } + } + }) + + expect(model).toEqual({ + quick: 'ph', + columns: { + total: { kind: 'number', op: 'gt', value: 15 }, + name: { kind: 'text', op: 'contains', value: 'al', caseSensitive: true } + } + }) + }) + + it('reads a group, dropping only the conditions that are broken', () => { + const model = sanitizeFilterModel({ + quick: '', + columns: { + name: { + kind: 'group', + join: 'or', + conditions: [ + { kind: 'text', op: 'contains', value: 'al' }, + { kind: 'text', op: 'eq', value: 'beta' } + ] + } + } + }) + + expect(model?.columns.name).toEqual({ + kind: 'group', + join: 'or', + conditions: [{ kind: 'text', op: 'contains', value: 'al' }] + }) + }) + + it('drops a group whose conditions are not a list, and one left with none', () => { + const model = sanitizeFilterModel({ + columns: { + name: { kind: 'group', join: 'and', conditions: 'nope' }, + when: { kind: 'group', join: 'and', conditions: [{ kind: 'date', op: 'gt' }] } + } + }) + + expect(model).toEqual({ quick: '', columns: {} }) + }) + + it('takes a presence operator with no value, in either spelling', () => { + const model = sanitizeFilterModel({ + columns: { + name: { kind: 'text', op: 'blank' }, + total: { kind: 'number', op: 'notBlank' } + } + }) + + expect(model?.columns).toEqual({ + name: { kind: 'text', op: 'blank', value: '' }, + total: { kind: 'number', op: 'notBlank' } + }) + }) + + it('rejects a slice that is not an object', () => { + expect(sanitizeFilterModel(null)).toBeNull() + expect(sanitizeFilterModel('filtered')).toBeNull() + expect(sanitizeFilterModel([])).toBeNull() + }) + + it('falls back to an empty quick filter rather than carrying junk', () => { + expect(sanitizeFilterModel({ quick: 42, columns: 'nope' })).toEqual({ + quick: '', + columns: {} + }) + }) +}) + +describe('hydrating a malformed filter snapshot', () => { + // One case per row of the table measured on 1.3.0, where six of the seven + // threw while the pipeline was reading them. + const cases: [string, string, unknown][] = [ + [ + 'a number operator spelled as the text one', + 'total', + { + kind: 'number', + op: 'equals', + value: 10 + } + ], + [ + 'a text operator spelled as the number one', + 'name', + { + kind: 'text', + op: 'eq', + value: 'alpha' + } + ], + [ + 'a date operator spelled as the number one', + 'when', + { + kind: 'date', + op: 'gt', + value: '2026-01-01' + } + ], + ['a set whose values are not a list', 'name', { kind: 'set', values: {} }], + ['a kind nothing knows', 'name', { kind: 'nonsense', op: 'eq' }], + ['a text condition with no value', 'name', { kind: 'text', op: 'contains' }], + ['a boolean value that is not a boolean', 'ok', { kind: 'boolean', value: 'yes' }] + ] + + for (const [label, columnId, filter] of cases) { + it(`reads every row and does not throw: ${label}`, () => { + const grid = gridWith(columnId, filter) + + expect(() => grid.nodes).not.toThrow() + expect(grid.nodes).toHaveLength(2) + expect(getFiltering(grid)?.columnFilters).toEqual({}) + }) + } + + it('keeps the readable column when another is dropped', () => { + const grid = createDataGrid({ + columns, + data, + getRowId: (row) => String(row.id), + features: [filtering()] + }) + grid.setState({ + version: 1, + features: { + filtering: { + quick: '', + columns: { + total: { kind: 'number', op: 'gt', value: 15 }, + name: { kind: 'text', op: 'eq', value: 'alpha' } + } + } + } + } as never) + + expect(grid.nodes).toHaveLength(1) + expect(grid.nodes[0]?.row.name).toBe('beta') + }) +}) + +describe('a broken condition set through the public API', () => { + // `applyFilterModel` does not sanitize: it is typed, and an app calling it + // has said what the model is. The predicate layer is what keeps a model + // that lied from reaching the pipeline as a throw. + const conditions: [string, string, ColumnFilter][] = [ + ['an operator from another kind', 'total', { kind: 'number', op: 'equals' } as never], + ['a text operator from another kind', 'name', { kind: 'text', op: 'eq' } as never], + ['a date operator from another kind', 'when', { kind: 'date', op: 'gt' } as never], + ['a set whose values are not a list', 'name', { kind: 'set', values: {} } as never], + ['a kind nothing knows', 'name', { kind: 'nonsense' } as never], + ['a text condition with no value', 'name', { kind: 'text', op: 'contains' } as never], + [ + 'a group whose conditions are not a list', + 'name', + { kind: 'group', join: 'and', conditions: 'nope' } as never + ] + ] + + for (const [label, columnId, condition] of conditions) { + it(`passes every row rather than throwing: ${label}`, () => { + const grid = createDataGrid({ + columns, + data, + getRowId: (row) => String(row.id), + features: [filtering()] + }) + getFiltering(grid)?.applyFilterModel({ quick: '', columns: { [columnId]: condition } }) + + expect(() => grid.nodes).not.toThrow() + expect(grid.nodes).toHaveLength(2) + }) + } +}) diff --git a/src/lib/features/filtering/filter-sanitize.ts b/src/lib/features/filtering/filter-sanitize.ts new file mode 100644 index 0000000..97d3226 --- /dev/null +++ b/src/lib/features/filtering/filter-sanitize.ts @@ -0,0 +1,159 @@ +import type { + ColumnFilter, + ColumnFilterEntry, + DateFilterOp, + FilterModel, + NumberFilterOp, + SetFilterValue, + TextFilterOp +} from '../../core/types/index.js' + +/** + * A snapshot is not a `FilterModel` just because it was cast to one. Share + * links, `localStorage` and anything handed back to `setState` have all been + * outside the grid, and a filter whose operator or value does not match its + * kind reaches the predicate builders as a shape they never check. That threw + * inside the pipeline's `$derived`, which takes down the render pass rather + * than one column. + * + * So the boundary drops what it cannot read. A condition that fails to + * sanitize is left out; a column left with nothing is left out; a model with + * no readable column filters nothing, which is what an empty model already + * does. That shows more rows than the snapshot asked for, deliberately: it is + * the honest reading of a filter nobody can reconstruct, and the alternative + * measured here was a grid that would not render at all. + */ + +const TEXT_OPS = new Set([ + 'contains', + 'notContains', + 'equals', + 'notEqual', + 'startsWith', + 'endsWith', + 'blank', + 'notBlank' +]) +const NUMBER_OPS = new Set([ + 'eq', + 'neq', + 'gt', + 'gte', + 'lt', + 'lte', + 'between', + 'blank', + 'notBlank' +]) +const DATE_OPS = new Set(['equals', 'before', 'after', 'between', 'blank', 'notBlank']) +const PRESENCE_OPS = new Set(['blank', 'notBlank']) + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function opOf(raw: Record, allowed: Set): string | null { + const { op } = raw + return typeof op === 'string' && allowed.has(op) ? op : null +} + +function isSetValue(value: unknown): value is SetFilterValue { + return ( + value === null || + typeof value === 'string' || + typeof value === 'number' || + typeof value === 'boolean' + ) +} + +function sanitizeText(raw: Record): ColumnFilter | null { + const op = opOf(raw, TEXT_OPS) + if (op === null) return null + // The presence operators carry no value, and the editor writes them with + // an empty one, so both spellings have to hydrate. + if (PRESENCE_OPS.has(op)) return { kind: 'text', op: op as TextFilterOp, value: '' } + if (typeof raw.value !== 'string') return null + const filter: Extract = { + kind: 'text', + op: op as TextFilterOp, + value: raw.value + } + if (raw.caseSensitive === true) filter.caseSensitive = true + return filter +} + +function sanitizeNumber(raw: Record): ColumnFilter | null { + const op = opOf(raw, NUMBER_OPS) + if (op === null) return null + if (PRESENCE_OPS.has(op)) return { kind: 'number', op: op as NumberFilterOp } + if (typeof raw.value !== 'number' || !Number.isFinite(raw.value)) return null + if (op === 'between') { + if (typeof raw.to !== 'number' || !Number.isFinite(raw.to)) return null + return { kind: 'number', op: 'between', value: raw.value, to: raw.to } + } + return { kind: 'number', op: op as NumberFilterOp, value: raw.value } +} + +function sanitizeDate(raw: Record): ColumnFilter | null { + const op = opOf(raw, DATE_OPS) + if (op === null) return null + if (PRESENCE_OPS.has(op)) return { kind: 'date', op: op as DateFilterOp } + if (typeof raw.value !== 'string' || raw.value === '') return null + if (op === 'between') { + if (typeof raw.to !== 'string' || raw.to === '') return null + return { kind: 'date', op: 'between', value: raw.value, to: raw.to } + } + return { kind: 'date', op: op as DateFilterOp, value: raw.value } +} + +function sanitizeSet(raw: Record): ColumnFilter | null { + if (!Array.isArray(raw.values)) return null + const values = raw.values.filter(isSetValue) + // The editor never builds an empty selection, so an empty one here is the + // remains of a broken list rather than a request to match nothing. + return values.length > 0 ? { kind: 'set', values } : null +} + +function sanitizeCondition(raw: unknown): ColumnFilter | null { + if (!isRecord(raw)) return null + switch (raw.kind) { + case 'text': + return sanitizeText(raw) + case 'number': + return sanitizeNumber(raw) + case 'date': + return sanitizeDate(raw) + case 'set': + return sanitizeSet(raw) + case 'boolean': + return typeof raw.value === 'boolean' ? { kind: 'boolean', value: raw.value } : null + default: + return null + } +} + +function sanitizeEntry(raw: unknown): ColumnFilterEntry | null { + if (!isRecord(raw)) return null + if (raw.kind !== 'group') return sanitizeCondition(raw) + if (!Array.isArray(raw.conditions)) return null + const conditions = raw.conditions + .map((condition) => sanitizeCondition(condition)) + .filter((condition): condition is ColumnFilter => condition !== null) + if (conditions.length === 0) return null + return { kind: 'group', join: raw.join === 'or' ? 'or' : 'and', conditions } +} + +/** A model built only from what the snapshot got right; null if it got nothing right. */ +export function sanitizeFilterModel(slice: unknown): FilterModel | null { + if (!isRecord(slice)) return null + + const columns: Record = {} + if (isRecord(slice.columns)) { + for (const [columnId, entry] of Object.entries(slice.columns)) { + const clean = sanitizeEntry(entry) + if (clean !== null) columns[columnId] = clean + } + } + + return { quick: typeof slice.quick === 'string' ? slice.quick : '', columns } +} diff --git a/src/lib/features/filtering/filtering.svelte.ts b/src/lib/features/filtering/filtering.svelte.ts index ce8a1a9..fcd5e1e 100644 --- a/src/lib/features/filtering/filtering.svelte.ts +++ b/src/lib/features/filtering/filtering.svelte.ts @@ -3,6 +3,7 @@ import type { ColumnFilterEntry, FilterModel, GridFeature } from '../../core/typ import { mutator } from '../../core/utils/index.js' import { distinctValuesCached } from './distinct-values.js' import { compileColumnFilters } from './filter-predicates.js' +import { sanitizeFilterModel } from './filter-sanitize.js' import { quickFilterNodes } from './quick-filter.js' export const FILTERING = 'filtering' @@ -112,9 +113,8 @@ export function filtering(options: FilteringOptions = {}): GridFeature { - if (slice && typeof slice === 'object') { - getFiltering(grid)?.applyFilterModel(slice as FilterModel) - } + const model = sanitizeFilterModel(slice) + if (model !== null) getFiltering(grid)?.applyFilterModel(model) }, pipelineStage: { order: PIPELINE_ORDER.filter, diff --git a/src/lib/features/filtering/index.ts b/src/lib/features/filtering/index.ts index 3846786..fe5cbf0 100644 --- a/src/lib/features/filtering/index.ts +++ b/src/lib/features/filtering/index.ts @@ -15,6 +15,7 @@ export { normalizeFilterEntry, toFilterRequest } from './filter-model.js' +export { sanitizeFilterModel } from './filter-sanitize.js' export { filterUnitScaleOf, toDisplayUnit, toModelUnit } from './filter-units.js' export { floatingCellOf, type FloatingCell } from './floating-filter.js' export { diff --git a/src/lib/features/sorting/cycle.test.ts b/src/lib/features/sorting/cycle.test.ts index c39e678..6785509 100644 --- a/src/lib/features/sorting/cycle.test.ts +++ b/src/lib/features/sorting/cycle.test.ts @@ -68,3 +68,16 @@ describe('sort cycle', () => { expect(sort.sort).toEqual([{ columnId: 'name', direction: 'asc' }]) }) }) + +describe('hydrating a malformed sort snapshot', () => { + it('reads every row and keeps only the entry it could rebuild', () => { + const target = grid() + target.setState({ + version: 1, + features: { sorting: [null, { columnId: 'name', direction: 'asc' }] } + } as never) + + expect(() => target.nodes).not.toThrow() + expect(getSorting(target)?.sort).toEqual([{ columnId: 'name', direction: 'asc' }]) + }) +}) diff --git a/src/lib/features/sorting/index.ts b/src/lib/features/sorting/index.ts index ed37c3d..e339e2f 100644 --- a/src/lib/features/sorting/index.ts +++ b/src/lib/features/sorting/index.ts @@ -1,5 +1,5 @@ export { sortNodes, type SortNulls } from './sort.js' -export { toSortRequest } from './sort-model.js' +export { sanitizeSortState, toSortRequest } from './sort-model.js' export { getSorting, Sorting, diff --git a/src/lib/features/sorting/sort-model.test.ts b/src/lib/features/sorting/sort-model.test.ts index 153e168..71520ad 100644 --- a/src/lib/features/sorting/sort-model.test.ts +++ b/src/lib/features/sorting/sort-model.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { buildRowNodes } from '../../core/grid/index.js' import type { ColumnDef } from '../../core/types/index.js' -import { toSortRequest } from './sort-model.js' +import { sanitizeSortState, toSortRequest } from './sort-model.js' import { sortNodes } from './sort.js' interface Person { @@ -113,3 +113,40 @@ describe('toSortRequest', () => { expect(toSortRequest([], columns)).toEqual([]) }) }) + +describe('sanitizeSortState', () => { + it('keeps the entries that name a column and a direction', () => { + expect( + sanitizeSortState([ + { columnId: 'lastName', direction: 'asc' }, + { columnId: 'age', direction: 'desc' } + ]) + ).toEqual([ + { columnId: 'lastName', direction: 'asc' }, + { columnId: 'age', direction: 'desc' } + ]) + }) + + it('drops a null entry rather than reading through it', () => { + expect(sanitizeSortState([null, { columnId: 'age', direction: 'asc' }])).toEqual([ + { columnId: 'age', direction: 'asc' } + ]) + }) + + it('drops what a sort cannot be built from', () => { + expect( + sanitizeSortState([ + { columnId: 'age', direction: 'sideways' }, + { direction: 'asc' }, + { columnId: '', direction: 'asc' }, + 'age', + 42 + ]) + ).toEqual([]) + }) + + it('rejects a slice that is not a list', () => { + expect(sanitizeSortState({ columnId: 'age', direction: 'asc' })).toBeNull() + expect(sanitizeSortState(null)).toBeNull() + }) +}) diff --git a/src/lib/features/sorting/sort-model.ts b/src/lib/features/sorting/sort-model.ts index 1f946a8..10caf27 100644 --- a/src/lib/features/sorting/sort-model.ts +++ b/src/lib/features/sorting/sort-model.ts @@ -37,3 +37,20 @@ export function toSortRequest( function flip(nulls: SortNulls): SortNulls { return nulls === 'first' ? 'last' : 'first' } + +/** + * A sort read back from a snapshot, keeping only the entries that name a + * column and a direction. The same reasoning as the filter model: what came + * through storage is not a `SortState[]` because it was cast to one, and a + * null entry in that array threw while the pipeline was reading it. + */ +export function sanitizeSortState(slice: unknown): SortState[] | null { + if (!Array.isArray(slice)) return null + return slice.flatMap((entry) => { + if (typeof entry !== 'object' || entry === null) return [] + const { columnId, direction } = entry as Record + if (typeof columnId !== 'string' || columnId === '') return [] + if (direction !== 'asc' && direction !== 'desc') return [] + return [{ columnId, direction }] + }) +} diff --git a/src/lib/features/sorting/sorting.svelte.ts b/src/lib/features/sorting/sorting.svelte.ts index 7eb600f..27e5dec 100644 --- a/src/lib/features/sorting/sorting.svelte.ts +++ b/src/lib/features/sorting/sorting.svelte.ts @@ -3,6 +3,7 @@ import { type GridState, PIPELINE_ORDER } from '../../core/grid/index.js' import type { GridFeature, Keybinding, SortDirection, SortState } from '../../core/types/index.js' import { mutator } from '../../core/utils/index.js' import { sortNodes, type SortNulls } from './sort.js' +import { sanitizeSortState } from './sort-model.js' export const SORTING = 'sorting' @@ -116,7 +117,8 @@ export function sorting(options: SortingOptions = {}): GridFeature { return sort.length > 0 ? sort : undefined }, hydrate: (slice, grid) => { - if (Array.isArray(slice)) getSorting(grid)?.setSort(slice as SortState[]) + const sort = sanitizeSortState(slice) + if (sort !== null) getSorting(grid)?.setSort(sort) }, pipelineStage: { order: PIPELINE_ORDER.sort, diff --git a/src/tests/column-width-snapshot.svelte.test.ts b/src/tests/column-width-snapshot.svelte.test.ts new file mode 100644 index 0000000..379af10 --- /dev/null +++ b/src/tests/column-width-snapshot.svelte.test.ts @@ -0,0 +1,88 @@ +import type { Component } from 'svelte' +import { describe, expect, it } from 'vitest' +import { render } from 'vitest-browser-svelte' +import { + createDataGrid, + DataGrid, + type ColumnDef, + type DataGridProps, + type GridState +} from '$lib/index.js' + +interface Row { + id: number + a: string + b: string +} + +const TypedDataGrid = DataGrid as unknown as Component> + +const rows: Row[] = [{ id: 1, a: 'x', b: 'y' }] +const columns: ColumnDef[] = [ + { id: 'a', header: 'A', width: 200 }, + { id: 'b', header: 'B', width: 200 } +] + +function makeGrid(): GridState { + return createDataGrid({ columns, data: rows, getRowId: (row) => String(row.id) }) +} + +async function mount(grid: GridState) { + const screen = await render(TypedDataGrid, { grid }) + await expect.element(screen.getByRole('grid')).toBeVisible() + return screen +} + +function cellAt(container: Element, row: number, col: number): HTMLElement { + const cell = container.querySelector(`[data-dg-cell="${row}:${col}"]`) + if (!cell) throw new Error(`no cell at ${row}:${col}`) + return cell +} + +/** + * Asserted on the computed style rather than on `widthOverrides`, because the + * override record looked perfectly reasonable while the layout was already + * gone: a width of `NaN` reached the custom property as `NaNpx`, which makes + * `grid-template-columns` invalid at computed-value time. The browser drops + * the whole declaration, every column folds into one track, and the cells + * stack down the page. Nothing throws on the way. + */ +function tracksOf(container: Element, row: number): string[] { + const line = cellAt(container, row, 0).parentElement + if (!line) throw new Error('no row element') + return getComputedStyle(line).gridTemplateColumns.split(' ') +} + +describe('a column width the layout cannot draw', () => { + it('keeps the track list when a snapshot carries NaN', async () => { + const grid = makeGrid() + const screen = await mount(grid) + expect(tracksOf(screen.container, 0)).toHaveLength(2) + + grid.setState({ version: 1, columns: { widths: { a: Number.NaN } } } as never) + await expect.poll(() => tracksOf(screen.container, 0).length).toBe(2) + + const [cellA, cellB] = [cellAt(screen.container, 0, 0), cellAt(screen.container, 0, 1)] + expect(cellA.getBoundingClientRect().top).toBe(cellB.getBoundingClientRect().top) + expect(cellA.getBoundingClientRect().right).toBeLessThanOrEqual( + Math.ceil(cellB.getBoundingClientRect().left) + ) + }) + + it('keeps the track list when the width setter is handed NaN', async () => { + const grid = makeGrid() + const screen = await mount(grid) + + grid.columns.setWidth('a', Number.NaN) + await expect.poll(() => tracksOf(screen.container, 0).length).toBe(2) + expect(grid.columns.style).not.toContain('NaN') + }) + + it('still applies a width it can draw', async () => { + const grid = makeGrid() + const screen = await mount(grid) + + grid.setState({ version: 1, columns: { widths: { a: 320 } } } as never) + await expect.poll(() => tracksOf(screen.container, 0)[0]).toBe('320px') + }) +}) diff --git a/src/tests/filter-row.svelte.test.ts b/src/tests/filter-row.svelte.test.ts index 18e287f..d877ae5 100644 --- a/src/tests/filter-row.svelte.test.ts +++ b/src/tests/filter-row.svelte.test.ts @@ -76,6 +76,27 @@ async function renderGrid(grid: GridState) { return screen } +/** What `GridFilterCell` waits before a typed value reaches the model. */ +const FILTER_DEBOUNCE_MS = 200 + +/** + * Types through the driver and reports the most writes a working debounce could + * have produced. + * + * Counting writes only means something against the time the typing took. The + * browser driver round-trips once per key, and on a loaded machine that round + * trip outlasts the window, so the debounce fires between keystrokes and a + * write per key is correct rather than a regression. A debounce can write once + * per window it is left alone for, plus once at the end, which is the bound + * this returns: exactly 1 when the typing fitted in one window, which is what + * an unloaded machine and CI do. + */ +async function typeWithin(keys: string) { + const started = performance.now() + await userEvent.keyboard(keys) + return Math.floor((performance.now() - started) / FILTER_DEBOUNCE_MS) + 1 +} + describe('the filter row', () => { it('draws one cell per column, and a field only where a filter is declared', async () => { const screen = await renderGrid(makeGrid()) @@ -596,12 +617,12 @@ describe('the data ops demo', () => { const cell = filterCell(screen.container, 0) cell.querySelector('[role="spinbutton"]')!.focus() - await userEvent.keyboard('01052026') + const windows = await typeWithin('01052026') await expect .poll(() => getFiltering(grid)!.columnFilters['joined'], { timeout: 2000 }) .toEqual({ kind: 'date', op: 'equals', value: '2026-01-05' }) - expect(writes).toBe(1) + expect(writes).toBeLessThanOrEqual(windows) await expect.poll(() => bodyRows(screen.container)).toBe(1) }) @@ -615,12 +636,12 @@ describe('the data ops demo', () => { 'input[role="spinbutton"]' )! input.focus() - await userEvent.keyboard('365') + const windows = await typeWithin('365') await expect .poll(() => getFiltering(grid)!.columnFilters['age'], { timeout: 2000 }) .toEqual({ kind: 'number', op: 'eq', value: 365 }) - expect(writes).toBe(1) + expect(writes).toBeLessThanOrEqual(windows) }) it('clears a picked date at once rather than after the wait', async () => {