From 8f8d16545ce688f4b518fa0d293f62e9b5d4541b Mon Sep 17 00:00:00 2001 From: nguyenlongdang0412 Date: Wed, 2 Sep 2026 14:15:11 +0700 Subject: [PATCH 1/3] fix(core): refuse a column width the layout cannot draw A `NaN` or `Infinity` width reached the CSS custom property as `NaNpx`. A custom property accepts that, but `grid-template-columns: var(...)` then resolves to an invalid value, the declaration is dropped at computed-value time, and every column folds into a single track with the cells stacked down the page. Nothing threw and nothing was logged: the grid simply looked broken, and `widthOverrides` still read as a plausible record while the layout was already gone. Two ways in, both closed. The snapshot boundary now reads values as carefully as it already read keys, and `setWidth`/`setWidths` refuse a width that is not finite, where `clamp` and `Math.round` had been carrying `NaN` straight through. An app computing a width from an empty input field reached the second without going near a snapshot. `setState` also stopped throwing on a corrupt `columns` slice: an `order` that was not an array reached `.filter` inside the caller's own call. Only string ids order the columns now, only real booleans hide one or fold a group, and a slice that is not an object is read as nothing at all. The fields are typed as unknown while they are read, because naming the type there would be the same promise that let the broken snapshot in. The regression test asserts the computed `grid-template-columns` in the browser, not the override record. Backing the fix out turns it red with one track where there should be two. Closes #43 --- src/lib/core/columns/column-model.svelte.ts | 8 +- src/lib/core/columns/column-model.test.ts | 30 +++++++ src/lib/core/grid/snapshot.test.ts | 61 +++++++++++++ src/lib/core/grid/snapshot.ts | 76 ++++++++++++---- .../column-width-snapshot.svelte.test.ts | 88 +++++++++++++++++++ 5 files changed, 246 insertions(+), 17 deletions(-) create mode 100644 src/tests/column-width-snapshot.svelte.test.ts 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/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/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') + }) +}) From f8a037ec2f5cb558f385738c33992a40dc393760 Mon Sep 17 00:00:00 2001 From: nguyenlongdang0412 Date: Wed, 2 Sep 2026 14:15:23 +0700 Subject: [PATCH 2/3] fix(core): say so when two rows share an id The row index keeps the last row for a repeated id, so an edit addressed to the row the user opened was written to a different one, and a selection stood for two rows at once. Nothing reported it. The id belongs to the app and the grid cannot mend it, but failing at it silently, in the data, is the worst of the available behaviours. A development build now names the ids that collided. Production pays one integer comparison for the whole data set, `index.size` against `nodes.length`; working out which ids repeated costs a second pass and only a build that will print it pays for that. This is the library's only `console` statement, and the lint rule that forbids them is disabled on exactly that line. An error would take an app down over data the grid can still draw, and there is no logger to route it to. Closes #44 --- CHANGELOG.md | 21 ++++++++ src/lib/core/grid/row-node.test.ts | 78 ++++++++++++++++++++++++++++++ src/lib/core/grid/row-node.ts | 40 +++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 src/lib/core/grid/row-node.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a3a23ac..7d4338d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- 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 both at the snapshot boundary and in `setWidth`/`setWidths`, + where `clamp` and `Math.round` had been carrying `NaN` straight through. +- 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 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 } From c39848478bbd9f30f1a24fa8de470a82b2bb3868 Mon Sep 17 00:00:00 2001 From: nguyenlongdang0412 Date: Wed, 2 Sep 2026 14:24:43 +0700 Subject: [PATCH 3/3] fix(core): refuse a non-finite track where it becomes CSS Closing the width setters and the snapshot boundary closed the routes the issue named, not the failure itself. Three more reach the same line without passing either: a container measured as `NaN`, which makes every flex column `NaNpx` at once, and a column definition written with `width: NaN` or `flex: NaN`. They all converge on `buildColumnCssVars`, so the check belongs there. A value that is not finite falls back to the column's minimum, and an unusable minimum or flex weight falls back to a width that can be drawn. A track of the wrong size is a much smaller failure than a grid that will not lay out at all. The earlier guards stay. They keep unusable values out of the model rather than papering over them at the end, and they let `setWidth` return an honest answer about the width a column still has. --- CHANGELOG.md | 7 ++- src/lib/core/columns/column-sizing.test.ts | 56 ++++++++++++++++++++++ src/lib/core/columns/column-sizing.ts | 30 ++++++++++-- 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d4338d..29f8456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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 both at the snapshot boundary and in `setWidth`/`setWidths`, - where `clamp` and `Math.round` had been carrying `NaN` straight through. + 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 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(' ')