diff --git a/.changeset/fuzzy-tables-filter.md b/.changeset/fuzzy-tables-filter.md new file mode 100644 index 0000000000..c57414375f --- /dev/null +++ b/.changeset/fuzzy-tables-filter.md @@ -0,0 +1,5 @@ +--- +'@tanstack/table-core': patch +--- + +Fix hierarchical filtered row models so `flatRows` lists parents before descendants in both filtering modes, preserves filter metadata on cloned rows, and round-trips nested data correctly through worker-backed row models. diff --git a/packages/table-core/src/features/column-filtering/filterRowsUtils.ts b/packages/table-core/src/features/column-filtering/filterRowsUtils.ts index e8f3e09336..061e812bd7 100644 --- a/packages/table-core/src/features/column-filtering/filterRowsUtils.ts +++ b/packages/table-core/src/features/column-filtering/filterRowsUtils.ts @@ -51,7 +51,7 @@ function filterRowModelFromLeafs< Partial> = [] // Filter from children up first - for (let row of rowsToFilter) { + for (const row of rowsToFilter) { const newRow = constructRow( table, row.id, @@ -63,30 +63,18 @@ function filterRowModelFromLeafs< ) as Row & Partial> newRow.columnFilters = row.columnFilters + newRow.columnFiltersMeta = row.columnFiltersMeta if (row.subRows.length && depth < maxDepth) { newRow.subRows = recurseFilterRows(row.subRows, depth + 1) - row = newRow - if (filterRow(row) && !newRow.subRows.length) { - filteredRows.push(row) - newFilteredRowsById[row.id] = row - newFilteredFlatRows.push(row) - continue - } - - if (filterRow(row) || newRow.subRows.length) { - filteredRows.push(row) - newFilteredRowsById[row.id] = row - newFilteredFlatRows.push(row) - continue + if (newRow.subRows.length || filterRow(newRow)) { + filteredRows.push(newRow) } } else { - row = newRow - if (filterRow(row)) { - filteredRows.push(row) - newFilteredRowsById[row.id] = row - newFilteredFlatRows.push(row) + if (filterRow(newRow)) { + newRow.subRows = row.subRows + filteredRows.push(newRow) } } } @@ -94,8 +82,11 @@ function filterRowModelFromLeafs< return filteredRows } + const rows = recurseFilterRows(rowsToFilter) + addSubRowsToFlatArrays(rows, newFilteredFlatRows, newFilteredRowsById) + return { - rows: recurseFilterRows(rowsToFilter), + rows, flatRows: newFilteredFlatRows, rowsById: newFilteredRowsById, } @@ -123,7 +114,7 @@ function filterRowModelFromRoot< const filteredRows: Array> = [] // Apply the filter to any subRows - for (let row of rowsToFilter) { + for (const row of rowsToFilter) { const pass = filterRow(row) if (pass) { @@ -136,26 +127,35 @@ function filterRowModelFromRoot< row.depth, undefined, row.parentId, - ) + ) as Row & + Partial> + const filterData = row as Row & + Partial> + newRow.columnFilters = filterData.columnFilters + newRow.columnFiltersMeta = filterData.columnFiltersMeta + + filteredRows.push(newRow) + newFilteredFlatRows.push(newRow) + newFilteredRowsById[newRow.id] = newRow + newRow.subRows = recurseFilterRows(row.subRows, depth + 1) - row = newRow - } + } else { + filteredRows.push(row) + newFilteredFlatRows.push(row) + newFilteredRowsById[row.id] = row - filteredRows.push(row) - newFilteredFlatRows.push(row) - newFilteredRowsById[row.id] = row - - // When maxLeafRowFilterDepth stops the recursion, the kept row's - // subtree stays visible through row.subRows, so those descendants - // must still enter flatRows and rowsById to keep the flat - // representation (and anything derived from it, like facet counts) - // consistent with the visible tree - if (row.subRows.length && depth >= maxDepth) { - addSubRowsToFlatArrays( - row.subRows, - newFilteredFlatRows, - newFilteredRowsById, - ) + // When maxLeafRowFilterDepth stops the recursion, the kept row's + // subtree stays visible through row.subRows, so those descendants + // must still enter flatRows and rowsById to keep the flat + // representation (and anything derived from it, like facet counts) + // consistent with the visible tree + if (row.subRows.length && depth >= maxDepth) { + addSubRowsToFlatArrays( + row.subRows, + newFilteredFlatRows, + newFilteredRowsById, + ) + } } } } diff --git a/packages/table-core/src/worker/createTableWorker.ts b/packages/table-core/src/worker/createTableWorker.ts index 514e593917..77421db944 100644 --- a/packages/table-core/src/worker/createTableWorker.ts +++ b/packages/table-core/src/worker/createTableWorker.ts @@ -4,6 +4,7 @@ import { tableWorkerPipeline, tableWorkerStageStateDeps, } from './tableWorkerProtocol' +import { applyFilterDataToCoreRows } from './rebuildRowModel' import type { Table_Internal } from '../types/Table' import type { TableFeature } from '../types/TableFeatures' import type { TableWorkerDataPayload } from './rebuildRowModel' @@ -236,6 +237,9 @@ function handleResult( [TableWorkerStage, TableWorkerStagePayload] >) { if (payload.kind === 'unchanged') continue + if (stage === 'filtered') { + applyFilterDataToCoreRows(table.getCoreRowModel().flatRows, payload) + } bridge.results[stage] = payload bridge.stageVersions[stage] = (bridge.stageVersions[stage] ?? 0) + 1 anyChanged = true diff --git a/packages/table-core/src/worker/initTableWorker.ts b/packages/table-core/src/worker/initTableWorker.ts index cf7c7fca6d..dd707e7766 100644 --- a/packages/table-core/src/worker/initTableWorker.ts +++ b/packages/table-core/src/worker/initTableWorker.ts @@ -147,8 +147,10 @@ export function initTableWorker< stages[stage] = serializeRowModel( model, coreIndexById, + table.getCoreRowModel().flatRows, aggregateColumnIds, transfer, + stage, ) } diff --git a/packages/table-core/src/worker/rebuildRowModel.ts b/packages/table-core/src/worker/rebuildRowModel.ts index 630d476669..2eeed692ab 100644 --- a/packages/table-core/src/worker/rebuildRowModel.ts +++ b/packages/table-core/src/worker/rebuildRowModel.ts @@ -1,15 +1,47 @@ import { constructRow } from '../core/rows/constructRow' -import { hasOwn } from '../utils' +import { copyInstancePropertiesWithoutMemos, hasOwn } from '../utils' import type { RowModel } from '../core/row-models/coreRowModelsFeature.types' import type { Table_Internal } from '../types/Table' import type { TableFeatures } from '../types/TableFeatures' import type { RowData } from '../types/type-utils' import type { + TableWorkerFilterData, TableWorkerRowNode, TableWorkerStage, TableWorkerStagePayload, } from './tableWorkerProtocol' +function applyFilterData(row: any, filterData?: TableWorkerFilterData) { + if (filterData) { + row.columnFilters = filterData.columnFilters + row.columnFiltersMeta = filterData.columnFiltersMeta + } +} + +export function applyFilterDataToCoreRows( + coreFlatRows: Array, + payload: TableWorkerDataPayload, +) { + if (payload.kind === 'flat') { + if (!payload.filterData) return + for (let i = 0; i < payload.indices.length; i++) { + applyFilterData(coreFlatRows[payload.indices[i]!], payload.filterData[i]) + } + return + } + + const applyToNodes = (nodes: Array) => { + for (const node of nodes) { + if (typeof node === 'number') continue + if (!('groupingColumnId' in node)) { + applyFilterData(coreFlatRows[node.index], node.filterData) + } + applyToNodes(node.children) + } + } + applyToNodes(payload.children) +} + /** Payloads that carry data; `unchanged` never reaches the rebuilder. */ export type TableWorkerDataPayload = Exclude< TableWorkerStagePayload, @@ -46,7 +78,7 @@ export function rebuildRowModel< // filtered model never touches them. Without this distinction a filtered // rebuild could zero depths assigned by a grouped/sorted tree rebuild. const resetDepths = stage !== 'filtered' - const flattenParentsFirst = stage === 'sorted' + const flattenParentsFirst = stage === 'filtered' || stage === 'sorted' if (payload.kind === 'flat') { const { indices } = payload @@ -57,6 +89,7 @@ export function rebuildRowModel< row.depth = 0 row.parentId = undefined } + applyFilterData(row, payload.filterData?.[i]) rows[i] = row } return { rows, flatRows: rows, rowsById: core.rowsById } @@ -84,6 +117,52 @@ export function rebuildRowModel< continue } + if (!('groupingColumnId' in node)) { + const coreRow: any = core.flatRows[node.index]! + const flatIndex = flattenParentsFirst ? flatRows.length : -1 + if (flattenParentsFirst) { + flatRows.push(undefined) + } + + const subRows = rebuildRows(node.children, depth + 1, coreRow.id) + let row = coreRow + const subRowsChanged = + subRows.length !== coreRow.subRows.length || + subRows.some((subRow, index) => subRow !== coreRow.subRows[index]) + + if (stage === 'filtered' && coreRow.subRows.length) { + row = constructRow( + table, + coreRow.id, + coreRow.original, + coreRow.index, + coreRow.depth, + undefined, + coreRow.parentId, + ) + row.subRows = subRows + } else if (subRowsChanged) { + row = Object.create(Object.getPrototypeOf(coreRow)) + copyInstancePropertiesWithoutMemos(row, coreRow) + row.subRows = subRows + } + + applyFilterData(row, node.filterData) + + row.depth = depth + row.parentId = parentId + if (flattenParentsFirst) { + flatRows[flatIndex] = row + } else { + flatRows.push(row) + } + if (row !== coreRow) { + rowsById[row.id] = row + } + rows[i] = row + continue + } + // Sorted flatRows preserve the recursive rows order. Reserve the // synthetic parent's position before rebuilding its descendants, then // fill it once the row can be constructed from those descendants. diff --git a/packages/table-core/src/worker/serializeRowModel.ts b/packages/table-core/src/worker/serializeRowModel.ts index 6f086a035b..a3d0240bd0 100644 --- a/packages/table-core/src/worker/serializeRowModel.ts +++ b/packages/table-core/src/worker/serializeRowModel.ts @@ -1,6 +1,8 @@ import type { RowModel } from '../core/row-models/coreRowModelsFeature.types' import type { + TableWorkerFilterData, TableWorkerRowNode, + TableWorkerStage, TableWorkerStagePayload, } from './tableWorkerProtocol' @@ -14,14 +16,36 @@ function isCloneSafe(value: unknown): boolean { function serializeRows( rows: Array, coreIndexById: Record, + coreFlatRows: Array, aggregateColumnIds: Array, + stage: TableWorkerStage, ): Array { const nodes = new Array(rows.length) for (let i = 0; i < rows.length; i++) { const row = rows[i] if (row.groupingColumnId == null) { - // Data row: its position in `options.data` is all the main thread needs. - nodes[i] = coreIndexById[row.id]! + const index = coreIndexById[row.id]! + const coreRow = coreFlatRows[index]! + // A true leaf needs only its core-row position. Branch rows must carry + // their row-model children because filtering and sorting can replace or + // reorder (or remove all of) the core subtree. Filtered rows also carry + // the flags and metadata computed by the worker. + nodes[i] = + stage === 'filtered' || row.subRows.length || coreRow.subRows.length + ? { + index, + children: serializeRows( + row.subRows, + coreIndexById, + coreFlatRows, + aggregateColumnIds, + stage, + ), + ...(stage === 'filtered' + ? { filterData: serializeFilterData(row) } + : {}), + } + : index continue } // Synthetic group row: compute aggregates eagerly (the expensive per-group @@ -50,7 +74,13 @@ function serializeRows( groupingValue: row.groupingValue, index: row.index, aggregates, - children: serializeRows(row.subRows, coreIndexById, aggregateColumnIds), + children: serializeRows( + row.subRows, + coreIndexById, + coreFlatRows, + aggregateColumnIds, + stage, + ), } } return nodes @@ -59,22 +89,51 @@ function serializeRows( export function serializeRowModel( model: RowModel, coreIndexById: Record, + coreFlatRows: Array, aggregateColumnIds: Array, transfer: Array, + stage: TableWorkerStage, ): TableWorkerStagePayload { // Flat fast path: no synthetic rows anywhere (flatRows === rows for flat // data). A Uint32Array permutation transfers at zero-copy cost. - if (model.flatRows.length === model.rows.length) { + const canUseFlatPayload = + model.flatRows.length === model.rows.length && + model.rows.every((row) => { + const coreRow = coreFlatRows[coreIndexById[row.id]!]! + return row.groupingColumnId == null && !coreRow.subRows.length + }) + + if (canUseFlatPayload) { const indices = new Uint32Array(model.rows.length) + const filterData = + stage === 'filtered' + ? new Array(model.rows.length) + : undefined for (let i = 0; i < indices.length; i++) { indices[i] = coreIndexById[model.rows[i]!.id]! + if (filterData) { + filterData[i] = serializeFilterData(model.rows[i]!) + } } transfer.push(indices.buffer) - return { kind: 'flat', indices } + return { kind: 'flat', indices, filterData } } return { kind: 'tree', - children: serializeRows(model.rows, coreIndexById, aggregateColumnIds), + children: serializeRows( + model.rows, + coreIndexById, + coreFlatRows, + aggregateColumnIds, + stage, + ), + } +} + +function serializeFilterData(row: any): TableWorkerFilterData { + return { + columnFilters: row.columnFilters, + columnFiltersMeta: row.columnFiltersMeta, } } diff --git a/packages/table-core/src/worker/tableWorkerProtocol.ts b/packages/table-core/src/worker/tableWorkerProtocol.ts index 4625452342..279a29aeda 100644 --- a/packages/table-core/src/worker/tableWorkerProtocol.ts +++ b/packages/table-core/src/worker/tableWorkerProtocol.ts @@ -29,11 +29,25 @@ export const tableWorkerStageStateDeps: Record< expanded: ['expanded'], } +export interface TableWorkerFilterData { + columnFilters: Record + columnFiltersMeta: Record +} + +/** + * A serialized data branch whose row-model hierarchy must be preserved. True + * leaf rows still serialize as a bare core-row index. + */ +export interface TableWorkerDataNode { + index: number + children: Array + filterData?: TableWorkerFilterData +} + /** - * A serialized synthetic (group) row. Leaf rows serialize as their position in - * `options.data`; group rows carry what `constructRow` + the grouped row model - * need to rebuild them, plus eagerly computed aggregate values so no - * aggregation runs on the main thread. + * A serialized synthetic (group) row. Group rows carry what `constructRow` + + * the grouped row model need to rebuild them, plus eagerly computed aggregate + * values so no aggregation ever runs on the main thread. */ export interface TableWorkerGroupNode { id: string @@ -44,7 +58,8 @@ export interface TableWorkerGroupNode { children: Array } -export type TableWorkerRowNode = number | TableWorkerGroupNode +export type TableWorkerRowNode = + number | TableWorkerDataNode | TableWorkerGroupNode /** * A stage result. Flat models (every row is a data row) travel as a @@ -58,7 +73,11 @@ export type TableWorkerRowNode = number | TableWorkerGroupNode * its previous result and skips the rebuild entirely. */ export type TableWorkerStagePayload = - | { kind: 'flat'; indices: Uint32Array } + | { + kind: 'flat' + indices: Uint32Array + filterData?: Array + } | { kind: 'tree'; children: Array } | { kind: 'unchanged' } diff --git a/packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts b/packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts index 67637123f7..cd5653a78a 100644 --- a/packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts +++ b/packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts @@ -7,6 +7,7 @@ import { globalFilteringFeature, } from '../../../../src' import { testFeatures } from '../../../fixtures/features' +import { filterRows } from '../../../../src/features/column-filtering/filterRowsUtils' import type { ColumnDef, FilterFn } from '../../../../src' interface TestRow { @@ -188,11 +189,11 @@ describe('createFilteredRowModel', () => { expect(rowNames(model.flatRows)).not.toContain('keep-b1') }) - it('should include cloned rows and exclude dropped rows in flatRows and rowsById', () => { + it('should include cloned rows and exclude dropped rows in flatRows and rowsById in pre-order', () => { const table = makeNestedTable() const model = table.getFilteredRowModel() - expect(rowNames(model.flatRows).sort()).toEqual([ + expect(rowNames(model.flatRows)).toEqual([ 'keep-a', 'keep-a1', 'keep-c', @@ -222,6 +223,20 @@ describe('createFilteredRowModel', () => { expect(rowNames(dropB.subRows)).toEqual(['keep-b1']) }) + it('should flatten rows in pre-order with each parent ahead of its sub-rows', () => { + const table = makeNestedTable({ filterFromLeafRows: true }) + const { flatRows } = table.getFilteredRowModel() + + expect(rowNames(flatRows)).toEqual([ + 'keep-a', + 'keep-a1', + 'drop-b', + 'keep-b1', + 'keep-c', + 'keep-d', + ]) + }) + it('should keep a matching parent that has no matching children', () => { const table = makeNestedTable({ filterFromLeafRows: true }) const { rows } = table.getFilteredRowModel() @@ -254,11 +269,7 @@ describe('createFilteredRowModel', () => { expect(rowNames(rows)).toEqual(['drop-x']) expect(rowNames(rows[0]!.subRows)).toEqual(['drop-x1']) expect(rowNames(rows[0]!.subRows[0]!.subRows)).toEqual(['keep-x1a']) - expect(rowNames(flatRows).sort()).toEqual([ - 'drop-x', - 'drop-x1', - 'keep-x1a', - ]) + expect(rowNames(flatRows)).toEqual(['drop-x', 'drop-x1', 'keep-x1a']) }) it('should prune matching subRows from a matching parent while filtering', () => { @@ -270,6 +281,32 @@ describe('createFilteredRowModel', () => { expect(rowNames(keepA.subRows)).toEqual(['keep-a1']) expect(keepA.subRows[0]!.subRows).toEqual([]) }) + + it('should skip the parent predicate when matching descendants retain it', () => { + const table = makeNestedTable({ + filterFromLeafRows: true, + data: [ + { name: 'drop-parent', subRows: [{ name: 'keep-child' }] }, + { name: 'keep-parent', subRows: [{ name: 'drop-child' }] }, + ], + }) + const predicate = vi.fn((row: { original: NestedRow }) => + row.original.name.includes('keep'), + ) + + const model = filterRows( + table.getCoreRowModel().rows, + predicate as any, + table as any, + ) + + expect(rowNames(model.rows)).toEqual(['drop-parent', 'keep-parent']) + expect(predicate.mock.calls.map(([row]) => row.original.name)).toEqual([ + 'keep-child', + 'drop-child', + 'keep-parent', + ]) + }) }) describe('maxLeafRowFilterDepth', () => { @@ -291,11 +328,25 @@ describe('createFilteredRowModel', () => { filterFromLeafRows: true, maxLeafRowFilterDepth: 0, }) - const { rows } = table.getFilteredRowModel() + const model = table.getFilteredRowModel() // drop-b is dropped even though keep-b1 matches, because descendants - // are never consulted at depth 0 - expect(rowNames(rows)).toEqual(['keep-a', 'keep-c', 'keep-d']) + // are never consulted at depth 0. Descendants of matching roots remain + // visible without being filtered. + expect(rowNames(model.rows)).toEqual(['keep-a', 'keep-c', 'keep-d']) + expect(rowNames(model.rows[0]!.subRows)).toEqual(['keep-a1', 'drop-a2']) + expect(rowNames(model.rows[0]!.subRows[0]!.subRows)).toEqual(['drop-a1a']) + expect(rowNames(model.flatRows)).toEqual([ + 'keep-a', + 'keep-a1', + 'drop-a1a', + 'drop-a2', + 'keep-c', + 'keep-d', + 'drop-d1', + ]) + const descendant = model.rows[0]!.subRows[0]! + expect(model.rowsById[descendant.id]).toBe(descendant) }) it('should include unfiltered descendants of kept rows in flatRows and rowsById (from root, depth 0)', () => { @@ -324,13 +375,12 @@ describe('createFilteredRowModel', () => { const model = table.getFilteredRowModel() // Depth-1 children are still filtered (drop-a2 removed), while the - // depth-2 subtree of keep-a1 is kept as-is and joins flatRows. The - // pre-existing flatRows order pushes recursed children before their - // parent. + // depth-2 subtree of keep-a1 is kept as-is and joins flatRows in + // pre-order traversal (parent before children). expect(rowNames(model.flatRows)).toEqual([ + 'keep-a', 'keep-a1', 'drop-a1a', - 'keep-a', 'keep-c', 'keep-d', ]) @@ -512,6 +562,54 @@ describe('createFilteredRowModel', () => { expect(preRows[0]!.columnFiltersMeta.name).toEqual({ globalHit: 'keep' }) expect(preRows[1]!.columnFiltersMeta.name).toEqual({ globalHit: 'drop' }) }) + + for (const filterFromLeafRows of [false, true]) { + it(`should preserve filter flags and metadata on nested ${ + filterFromLeafRows ? 'leaf-first' : 'root-first' + } clones`, () => { + const metaFilterFn: FilterFn = ( + row, + columnId, + filterValue, + addMeta, + ) => { + const value = row.getValue(columnId) + addMeta?.({ inspected: value }) + return value.includes(filterValue as string) + } + const table = constructTable({ + features, + columns: [ + { accessorKey: 'name', id: 'name', filterFn: metaFilterFn }, + ], + data: [ + { + name: 'keep-parent', + subRows: [{ name: 'keep-child' }], + }, + ], + getSubRows: (row) => row.subRows, + filterFromLeafRows, + initialState: { + columnFilters: [{ id: 'name', value: 'keep' }], + }, + }) + + const model = table.getFilteredRowModel() + const preRowsById = table.getPreFilteredRowModel().rowsById + + for (const row of model.flatRows) { + const preRow = preRowsById[row.id]! + expect(row.columnFilters).toBe(preRow.columnFilters) + expect(row.columnFiltersMeta).toBe(preRow.columnFiltersMeta) + expect(row.columnFilters.name).toBe(true) + expect(row.columnFiltersMeta.name).toEqual({ + inspected: row.original.name, + }) + expect(model.rowsById[row.id]).toBe(row) + } + }) + } }) describe('row.columnFilters flags', () => { @@ -716,4 +814,66 @@ describe('createFilteredRowModel', () => { expect(table.getFilteredRowModel()).toBe(table.getPreFilteredRowModel()) }) }) + + describe('pre-order flatRows traversal', () => { + const complexNestedData: Array = [ + { + name: 'parent-1', + subRows: [ + { + name: 'child-1.1', + subRows: [ + { name: 'grandchild-1.1.1' }, + { name: 'grandchild-1.1.2' }, + ], + }, + { name: 'child-1.2' }, + ], + }, + { + name: 'parent-2', + subRows: [{ name: 'child-2.1' }], + }, + ] + + it('flattens rows depth-first with each parent preceding its sub-rows (root filtering)', () => { + const table = constructTable({ + features, + columns: nestedColumns, + data: complexNestedData, + getSubRows: (row) => row.subRows, + initialState: { + columnFilters: [{ id: 'name', value: '1' }], + }, + }) + + expect(rowNames(table.getFilteredRowModel().flatRows)).toEqual([ + 'parent-1', + 'child-1.1', + 'grandchild-1.1.1', + 'grandchild-1.1.2', + 'child-1.2', + ]) + }) + + it('flattens rows depth-first with each parent preceding its sub-rows (leaf filtering)', () => { + const table = constructTable({ + features, + columns: nestedColumns, + data: complexNestedData, + getSubRows: (row) => row.subRows, + filterFromLeafRows: true, + initialState: { + columnFilters: [{ id: 'name', value: '.2' }], + }, + }) + + expect(rowNames(table.getFilteredRowModel().flatRows)).toEqual([ + 'parent-1', + 'child-1.1', + 'grandchild-1.1.2', + 'child-1.2', + ]) + }) + }) }) diff --git a/packages/table-core/tests/unit/worker/createTableWorker.test.ts b/packages/table-core/tests/unit/worker/createTableWorker.test.ts index cd42f70efd..c9842ab85f 100644 --- a/packages/table-core/tests/unit/worker/createTableWorker.test.ts +++ b/packages/table-core/tests/unit/worker/createTableWorker.test.ts @@ -234,6 +234,45 @@ describe('createTableWorker bridge', () => { expect(modelB).toBe(modelA) }) + it('applies filtered metadata before rebuilding a downstream stage', () => { + const tableWorker = createTableWorker({ + createWorker: () => new (globalThis as any).Worker(), + }) + const table = makeTable(tableWorker) + table.baseAtoms.columnFilters.set([{ id: 'firstName', value: 'person' }]) + + // Request only the downstream getter. Its fallback initializes the + // upstream filtered stage while the first request is in flight. + table.getSortedRowModel() + const worker = FakeWorker.instances[0]! + worker.emitResult({ + stages: { sorted: { kind: 'flat', indices: reversedIndices(8) } }, + }) + + const filterData = data.map((_, index) => ({ + columnFilters: { firstName: true }, + columnFiltersMeta: { firstName: { rank: index } }, + })) + worker.emitResult({ + stages: { + filtered: { + kind: 'flat', + indices: Uint32Array.from({ length: 8 }, (_, index) => index), + filterData, + }, + // Downstream payloads deliberately remain metadata-free. + sorted: { kind: 'flat', indices: reversedIndices(8) }, + }, + }) + + const sorted = table.getSortedRowModel() + expect(sorted.rows[0]!.id).toBe('7') + expect(sorted.rows[0]!.columnFilters).toEqual({ firstName: true }) + expect(sorted.rows[0]!.columnFiltersMeta).toEqual({ + firstName: { rank: 7 }, + }) + }) + it('clears pending and stops posting after a worker error', async () => { const tableWorker = createTableWorker({ createWorker: () => new (globalThis as any).Worker(), diff --git a/packages/table-core/tests/unit/worker/serializeRebuild.test.ts b/packages/table-core/tests/unit/worker/serializeRebuild.test.ts index 47aae6ff57..b6b8acd052 100644 --- a/packages/table-core/tests/unit/worker/serializeRebuild.test.ts +++ b/packages/table-core/tests/unit/worker/serializeRebuild.test.ts @@ -25,6 +25,7 @@ type Person = { age: number visits: number status: 'single' | 'complicated' | 'relationship' + subRows?: Array } const STATUSES = ['single', 'complicated', 'relationship'] as const @@ -62,11 +63,20 @@ const columns: Array> = [ // Columns with an explicit aggregation, mirroring initTableWorker's selection. const aggregateColumnIds = ['age', 'visits'] -function makeTable(data: Array): Table { +function makeTable( + data: Array, + options?: { + filterFromLeafRows?: boolean + maxLeafRowFilterDepth?: number + }, +): Table { return constructTable({ data, columns, features, + getSubRows: (row) => row.subRows, + filterFromLeafRows: options?.filterFromLeafRows, + maxLeafRowFilterDepth: options?.maxLeafRowFilterDepth, }) } @@ -90,8 +100,10 @@ function roundTrip( const payload = serializeRowModel( model, coreIndexMap(workerTable), + workerTable.getCoreRowModel().flatRows, aggregateColumnIds, transfer, + stage, ) if (payload.kind === 'unchanged') { throw new Error('expected a data payload, got "unchanged"') @@ -136,6 +148,7 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }]) const model = workerTable.getFilteredRowModel() + model.rows[0]!.columnFiltersMeta.status = { rank: 1 } const { payload, rebuilt } = roundTrip( workerTable, mainTable, @@ -148,6 +161,217 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { expect(model.rows.length).toBeLessThan(data.length) expect(ids(rebuilt.rows)).toEqual(ids(model.rows)) expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(rebuilt.rows[0]!.columnFilters).toEqual(model.rows[0]!.columnFilters) + expect(rebuilt.rows[0]!.columnFiltersMeta).toEqual( + model.rows[0]!.columnFiltersMeta, + ) + }) + + it('does not restore children removed from a matching filtered parent', () => { + const data: Array = [ + { + firstName: 'kept-parent', + age: 40, + visits: 1, + status: 'single', + subRows: [ + { + firstName: 'dropped-child', + age: 20, + visits: 2, + status: 'complicated', + }, + ], + }, + ] + const workerTable = makeTable(data) + const mainTable = makeTable(data) + workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }]) + + const model = workerTable.getFilteredRowModel() + const { payload, rebuilt } = roundTrip( + workerTable, + mainTable, + model, + 'filtered', + ) + + expect(payload.kind).toBe('tree') + expect(rebuilt.rows).toHaveLength(1) + expect(rebuilt.rows[0]!.subRows).toEqual([]) + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + }) + + it('round-trips a parent-first hierarchical filtered model from roots', () => { + const data: Array = [ + { + firstName: 'kept-parent', + age: 40, + visits: 1, + status: 'single', + subRows: [ + { + firstName: 'kept-child', + age: 20, + visits: 2, + status: 'single', + subRows: [ + { + firstName: 'kept-grandchild', + age: 5, + visits: 3, + status: 'single', + }, + { + firstName: 'dropped-grandchild', + age: 6, + visits: 4, + status: 'complicated', + }, + ], + }, + { + firstName: 'dropped-child', + age: 21, + visits: 5, + status: 'complicated', + }, + ], + }, + { + firstName: 'dropped-parent', + age: 41, + visits: 6, + status: 'complicated', + subRows: [ + { + firstName: 'unreachable-match', + age: 22, + visits: 7, + status: 'single', + }, + ], + }, + ] + const workerTable = makeTable(data) + const mainTable = makeTable(data) + workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }]) + + const model = workerTable.getFilteredRowModel() + const { payload, rebuilt } = roundTrip( + workerTable, + mainTable, + model, + 'filtered', + ) + + expect(payload.kind).toBe('tree') + expect(ids(rebuilt.rows)).toEqual(ids(model.rows)) + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(ids(rebuilt.rows[0]!.subRows)).toEqual(ids(model.rows[0]!.subRows)) + expect(ids(rebuilt.rows[0]!.subRows[0]!.subRows)).toEqual( + ids(model.rows[0]!.subRows[0]!.subRows), + ) + expect(rebuilt.rows[0]).not.toBe(mainTable.getCoreRowModel().rows[0]) + for (const row of rebuilt.flatRows) { + expect(rebuilt.rowsById[row.id]).toBe(row) + } + }) + + it('round-trips a parent-first hierarchical filtered model from leaves', () => { + const data: Array = [ + { + firstName: 'retained-parent', + age: 40, + visits: 1, + status: 'complicated', + subRows: [ + { + firstName: 'retained-child', + age: 20, + visits: 2, + status: 'complicated', + subRows: [ + { + firstName: 'matching-grandchild', + age: 5, + visits: 3, + status: 'single', + }, + ], + }, + { + firstName: 'matching-child', + age: 21, + visits: 4, + status: 'single', + }, + ], + }, + ] + const options = { filterFromLeafRows: true } + const workerTable = makeTable(data, options) + const mainTable = makeTable(data, options) + workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }]) + + const model = workerTable.getFilteredRowModel() + const { rebuilt } = roundTrip(workerTable, mainTable, model, 'filtered') + + expect(ids(rebuilt.rows)).toEqual(ids(model.rows)) + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(ids(rebuilt.rows[0]!.subRows)).toEqual(ids(model.rows[0]!.subRows)) + expect(ids(rebuilt.rows[0]!.subRows[0]!.subRows)).toEqual( + ids(model.rows[0]!.subRows[0]!.subRows), + ) + for (const row of rebuilt.flatRows) { + expect(rebuilt.rowsById[row.id]).toBe(row) + } + expect(rebuilt.rows[0]).not.toBe(mainTable.getCoreRowModel().rows[0]) + expect(rebuilt.rows[0]!.subRows[0]).not.toBe( + mainTable.getCoreRowModel().rows[0]!.subRows[0], + ) + }) + + it('round-trips unfiltered descendants kept past max filter depth', () => { + const data: Array = [ + { + firstName: 'kept-parent', + age: 40, + visits: 1, + status: 'single', + subRows: [ + { + firstName: 'unfiltered-child', + age: 20, + visits: 2, + status: 'complicated', + subRows: [ + { + firstName: 'unfiltered-grandchild', + age: 5, + visits: 3, + status: 'relationship', + }, + ], + }, + ], + }, + ] + const options = { maxLeafRowFilterDepth: 0 } + const workerTable = makeTable(data, options) + const mainTable = makeTable(data, options) + workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }]) + + const model = workerTable.getFilteredRowModel() + const { rebuilt } = roundTrip(workerTable, mainTable, model, 'filtered') + + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(ids(rebuilt.rows[0]!.subRows[0]!.subRows)).toEqual( + ids(model.rows[0]!.subRows[0]!.subRows), + ) + for (const row of rebuilt.flatRows) { + expect(rebuilt.rowsById[row.id]).toBe(row) + } }) it('round-trips a grouped model as a tree with aggregates', () => { @@ -224,6 +448,48 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { } }) + it('round-trips a parent-first sorted hierarchy of data rows', () => { + const data: Array = [ + { + firstName: 'parent', + age: 40, + visits: 0, + status: 'single', + subRows: [ + { + firstName: 'later-child', + age: 20, + visits: 2, + status: 'single', + }, + { + firstName: 'earlier-child', + age: 21, + visits: 1, + status: 'single', + }, + ], + }, + ] + const workerTable = makeTable(data) + const mainTable = makeTable(data) + workerTable.baseAtoms.sorting.set([{ id: 'visits', desc: false }]) + + const model = workerTable.getSortedRowModel() + const { payload, rebuilt } = roundTrip( + workerTable, + mainTable, + model, + 'sorted', + ) + + expect(payload.kind).toBe('tree') + expect(ids(rebuilt.rows)).toEqual(ids(model.rows)) + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(ids(rebuilt.rows[0]!.subRows)).toEqual(ids(model.rows[0]!.subRows)) + expect(rebuilt.rowsById[rebuilt.rows[0]!.id]).toBe(rebuilt.rows[0]) + }) + it('round-trips multi-column grouping (nested tree)', () => { const data = makeData(18) const workerTable = makeTable(data) diff --git a/perf-done.md b/perf-done.md index 131d8aa0a1..1284336f67 100644 --- a/perf-done.md +++ b/perf-done.md @@ -7,8 +7,8 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 59 -- **Source findings:** 57 +- **Entries:** 60 +- **Source findings:** 58 - **Cross-cutting sweeps:** 2 - 2026-07-03: #102 (C9) moved here from perf-todo.md after the row-model benchmark confirmed and the fix landed. - 2026-07-07: #68 (A4) moved here from perf-todo.md after implementation. @@ -16,6 +16,7 @@ Entries are sorted by adjusted effectiveness score descending. - 2026-07-07: #85 (A6) moved here from perf-todo.md after implementation. - 2026-07-07: #92 (C11) moved here from perf-todo.md after implementation. - 2026-07-29: #65 (F1) moved here from perf-todo.md after the Svelte selector layer was removed. +- 2026-08-25: #29 moved here from perf-todo.md with the filtered-row pre-order fix. ## Score 9 @@ -60,7 +61,7 @@ if (!filterableIds.length) { **Big-O:** O(R) → O(1) for the excluded-own-filter-only case: ~100k iterations + ~100k object-map insertions + 2 array allocations avoided per keystroke per faceted column (~10-30ms at 100k rows). Verified bonus: `getFacetedUniqueValues`/`getFacetedMinMaxValues` memoDeps key on `.flatRows` identity, so the stable `preRowModel` reference also converts the downstream O(R) facet-map rebuild into a memo hit per keystroke. -**Risk:** Returns `preRowModel` by reference instead of a fresh model with identical contents; same referential behavior as the existing empty-filter branch. For hierarchical data there is a flatRows-order note: the current all-pass `filterRows` emits child-before-parent flatRows while `preRowModel` is parent-first, so facet `Map` insertion order can shift (content identical; the existing no-filter branch already returns parent-first, so precedent exists). The early return never reads tags, so B17's ordering constraint does not apply. +**Risk:** Returns `preRowModel` by reference instead of a fresh model with identical contents; same referential behavior as the existing empty-filter branch. PR #6568 later aligned hierarchical `filterRows` with the parent-first order already used by `preRowModel`, removing the former facet `Map` insertion-order delta. The early return never reads tags, so B17's ordering constraint does not apply. **Verification:** CONFIRMED, raised 8 → 9; verifier added the downstream facet-map memo-hit win and the hierarchical flatRows-order note. --- @@ -2476,6 +2477,32 @@ Swap `.map()` and `for...of` for indexed loops. Called for every row in the row --- +## 29. `filterRowModelFromLeafs` duplicates predicate work — Score: 4 + +**Status:** `[x]` done +**Implementation note:** Collapsed the two overlapping branch predicates into one `newRow.subRows.length || filterRow(newRow)` check while restructuring filtered `flatRows` into parent-first order. The child-first short circuit also avoids the tag scan entirely for parents retained by matching descendants. Added nested leaf-first filtering coverage that exercises matching parents, retained non-matching ancestors, pre-order flattening, row identity, and preserved filter metadata. + +**Location:** `packages/table-core/src/features/column-filtering/filterRowsUtils.ts:43–101` +**Category:** `micro` + +`filterRow(row)` is called twice in some branches. Cache the boolean and the `hasVisibleSubRows` flag, branch once. + +**Scale impact** (duplicate `filterRow` invocations saved — dimension: rows in subtree-bearing branches per filter pass): + +| Rows in subtree-bearing branches | Before (`filterRow` calls) | After | Saved | +| -------------------------------- | -------------------------- | ------ | ------ | +| 10 | 20 | 10 | 10 | +| 100 | 200 | 100 | 100 | +| 1,000 | 2,000 | 1,000 | 1,000 | +| 10,000 | 20,000 | 10,000 | 10,000 | + +**Risk:** Logic is subtle; focused regression coverage now verifies parent-first flattening, retained ancestors, filter metadata, and unfiltered descendants at the maximum leaf-filter depth. + +**2026-07-01 audit (B6, score 4 — verified simplification):** The from-leafs branch calls `filterRow(newRow)` (an O(F) tag scan) twice per passing branch row, and the first of two identical-bodied branches is provably subsumed by the second (`(A && !B)` implies `(A || B)`). Fix: single `if (newRow.subRows.length || filterRow(newRow))` — checking subRows first also skips the predicate entirely for parents kept alive by children. Boolean-identical simplification, verified airtight; opt-in `filterFromLeafRows` path only. +**Verification:** Verified (2026-07-01 audit); boolean equivalence proven. + +--- + ## 39. `row_getVisibleCells` builds Sets for the small `left`/`right` arrays — Score: 4 **Status:** `[x]` done diff --git a/perf-todo.md b/perf-todo.md index 745e3e6ec3..b3091c8e52 100644 --- a/perf-todo.md +++ b/perf-todo.md @@ -7,8 +7,8 @@ Entries are sorted by adjusted effectiveness score descending. ## Counts -- **Entries:** 74 -- **Source findings:** 74 +- **Entries:** 73 +- **Source findings:** 73 - **Cross-cutting sweeps:** 0 - 2026-07-03: #102 (C9) completed and moved to perf-done.md. - 2026-07-07: #68 (A4) completed and moved to perf-done.md. @@ -16,6 +16,7 @@ Entries are sorted by adjusted effectiveness score descending. - 2026-07-07: #85 (A6) completed and moved to perf-done.md. - 2026-07-07: #92 (C11) completed and moved to perf-done.md. - 2026-07-29: #65 (F1) completed and moved to perf-done.md after the Svelte selector layer was removed. +- 2026-08-25: #29 completed and moved to perf-done.md with the filtered-row pre-order fix. ## Score 8 @@ -1451,32 +1452,6 @@ Today's dep is `table.options.data`. If a consumer recreates the options object --- -## 29. `filterRowModelFromLeafs` duplicates predicate work — Score: 4 - -**Status:** `[ ]` not started -**Implementation note:** _(none)_ - -**Location:** `src/features/column-filtering/filterRowsUtils.ts:43–101` -**Category:** `micro` - -`filterRow(row)` is called twice in some branches. Cache the boolean and the `hasVisibleSubRows` flag, branch once. - -**Scale impact** (duplicate `filterRow` invocations saved — dimension: rows in subtree-bearing branches per filter pass): - -| Rows in subtree-bearing branches | Before (`filterRow` calls) | After | Saved | -| -------------------------------- | -------------------------- | ------ | ------ | -| 10 | 20 | 10 | 10 | -| 100 | 200 | 100 | 100 | -| 1,000 | 2,000 | 1,000 | 1,000 | -| 10,000 | 20,000 | 10,000 | 10,000 | - -**Risk:** Logic is subtle; needs unit-test coverage when refactored. - -**2026-07-01 audit (B6, score 4 — verified simplification):** The from-leafs branch calls `filterRow(row)` (an O(F) tag scan) twice per passing branch row, and the first of two identical-bodied branches is provably subsumed by the second (`(A && !B)` implies `(A || B)`). Fix: single `if (newRow.subRows.length || filterRow(row))` — checking subRows first also skips the predicate entirely for parents kept alive by children. Boolean-identical simplification, verified airtight; opt-in `filterFromLeafRows` path only. -**Verification:** Verified (2026-07-01 audit); boolean equivalence proven. - ---- - ## 32. `groupBy` uses `Array.prototype.reduce` (broadened by B11: createGroupedRowModel loop fusion) — Score: 4 **Status:** `[ ]` not started