From 4df2ce99d671636aff6e09c9e2ed0cea308c4717 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Tue, 25 Aug 2026 16:42:43 -0500 Subject: [PATCH] fix(table-core): preserve grouped flat row preorder Co-authored-by: Lazizbek Ergashev --- .changeset/tidy-groups-walk.md | 5 + .../column-grouping/createGroupedRowModel.ts | 27 ++-- .../table-core/src/worker/rebuildRowModel.ts | 21 ++- .../row-models/rowModelFlatRowsOrder.test.ts | 121 ++++++++++++++++++ .../createGroupedRowModel.test.ts | 40 +++++- .../unit/worker/serializeRebuild.test.ts | 21 +++ perf-new.md | 13 +- 7 files changed, 225 insertions(+), 23 deletions(-) create mode 100644 .changeset/tidy-groups-walk.md create mode 100644 packages/table-core/tests/implementation/core/row-models/rowModelFlatRowsOrder.test.ts diff --git a/.changeset/tidy-groups-walk.md b/.changeset/tidy-groups-walk.md new file mode 100644 index 0000000000..7dffb5f23c --- /dev/null +++ b/.changeset/tidy-groups-walk.md @@ -0,0 +1,5 @@ +--- +'@tanstack/table-core': patch +--- + +Emit grouped and downstream worker `flatRows` in parent-first preorder. diff --git a/packages/table-core/src/features/column-grouping/createGroupedRowModel.ts b/packages/table-core/src/features/column-grouping/createGroupedRowModel.ts index 718fcc8a26..d78fcea280 100644 --- a/packages/table-core/src/features/column-grouping/createGroupedRowModel.ts +++ b/packages/table-core/src/features/column-grouping/createGroupedRowModel.ts @@ -107,17 +107,11 @@ function _createGroupedRowModel< return rows.map((row) => { row.depth = depth - // Every row is pushed into flatRows/rowsById exactly once, by its - // parent frame: rows returned here are pushed by the caller (the - // parent group's loop or the root loop), so only descendants below - // the terminal depth are pushed here. + groupedFlatRows.push(row) + groupedRowsById[row.id] = row + if (row.subRows.length) { row.subRows = groupUpRecursively(row.subRows, depth + 1, row.id) - for (let i = 0; i < row.subRows.length; i++) { - const subRow = row.subRows[i]! - groupedFlatRows.push(subRow) - groupedRowsById[subRow.id] = subRow - } } return row @@ -135,6 +129,10 @@ function _createGroupedRowModel< let id = `${columnId}:${groupingValue}` id = parentId ? `${parentId}>${id}` : id + // Reserve this group's position before its descendants are built. + const flatIndex = groupedFlatRows.length + groupedFlatRows.push(undefined as unknown as Row) + // First, Recurse to group sub rows before aggregation const subRows = groupUpRecursively(groupedRows, depth + 1, id) @@ -206,10 +204,8 @@ function _createGroupedRowModel< }, }) - subRows.forEach((subRow) => { - groupedFlatRows.push(subRow) - groupedRowsById[subRow.id] = subRow - }) + groupedFlatRows[flatIndex] = row + groupedRowsById[id] = row return row }, @@ -220,11 +216,6 @@ function _createGroupedRowModel< const groupedRows = groupUpRecursively(rowModel.rows, 0) - groupedRows.forEach((subRow) => { - groupedFlatRows.push(subRow) - groupedRowsById[subRow.id] = subRow - }) - return { rows: groupedRows, flatRows: groupedFlatRows, diff --git a/packages/table-core/src/worker/rebuildRowModel.ts b/packages/table-core/src/worker/rebuildRowModel.ts index 2eeed692ab..8122ef9d00 100644 --- a/packages/table-core/src/worker/rebuildRowModel.ts +++ b/packages/table-core/src/worker/rebuildRowModel.ts @@ -78,7 +78,8 @@ 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 === 'filtered' || stage === 'sorted' + const flattenParentsFirst = + stage === 'filtered' || stage === 'grouped' || stage === 'sorted' if (payload.kind === 'flat') { const { indices } = payload @@ -207,5 +208,23 @@ export function rebuildRowModel< const rows = rebuildRows(payload.children, 0, undefined) + if (stage === 'expanded') { + // Expanded rows are serialized inline as well as beneath their parents. + // Rebuild flatRows from the finished tree so each row appears once and + // parents retain their pipeline-wide preorder contract. + flatRows.length = 0 + const seen = new Set() + const flattenRows = (nestedRows: Array) => { + for (let i = 0; i < nestedRows.length; i++) { + const row = nestedRows[i] + if (seen.has(row.id)) continue + seen.add(row.id) + flatRows.push(row) + flattenRows(row.subRows) + } + } + flattenRows(rows) + } + return { rows, flatRows, rowsById } } diff --git a/packages/table-core/tests/implementation/core/row-models/rowModelFlatRowsOrder.test.ts b/packages/table-core/tests/implementation/core/row-models/rowModelFlatRowsOrder.test.ts new file mode 100644 index 0000000000..07a4bffd94 --- /dev/null +++ b/packages/table-core/tests/implementation/core/row-models/rowModelFlatRowsOrder.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' +import { + columnFilteringFeature, + columnGroupingFeature, + constructTable, + createExpandedRowModel, + createFilteredRowModel, + createGroupedRowModel, + createPaginatedRowModel, + createSortedRowModel, + filterFns, + globalFilteringFeature, + rowAggregationFeature, + rowExpandingFeature, + rowPaginationFeature, + rowSortingFeature, +} from '../../../../src' +import { testFeatures } from '../../../fixtures/features' +import type { ColumnDef, Row, RowModel } from '../../../../src' + +interface PipelineRow { + group: string + name: string + subRows?: Array +} + +const features = testFeatures({ + columnFilteringFeature, + columnGroupingFeature, + globalFilteringFeature, + rowAggregationFeature, + rowExpandingFeature, + rowPaginationFeature, + rowSortingFeature, + expandedRowModel: createExpandedRowModel(), + filteredRowModel: createFilteredRowModel(), + groupedRowModel: createGroupedRowModel(), + paginatedRowModel: createPaginatedRowModel(), + sortedRowModel: createSortedRowModel(), + filterFns, +}) + +const data: Array = [ + { + group: 'b', + name: 'keep-b', + subRows: [ + { + group: 'b', + name: 'keep-b2', + subRows: [{ group: 'b', name: 'keep-b2a' }], + }, + { group: 'b', name: 'keep-b1' }, + ], + }, + { + group: 'a', + name: 'keep-a', + subRows: [{ group: 'a', name: 'keep-a1' }], + }, +] + +const columns: Array> = [ + { accessorKey: 'group', id: 'group' }, + { accessorKey: 'name', id: 'name' }, +] + +function preorderIds(rows: Array>) { + const result: Array = [] + const seen = new Set() + + const visit = (nestedRows: Array>) => { + for (let i = 0; i < nestedRows.length; i++) { + const row = nestedRows[i]! + if (seen.has(row.id)) continue + seen.add(row.id) + result.push(row.id) + visit(row.subRows) + } + } + + visit(rows) + return result +} + +function expectPreorder(model: RowModel) { + const flatIds = model.flatRows.map((row) => row.id) + expect(flatIds).toEqual(preorderIds(model.rows)) + expect(new Set(flatIds).size).toBe(flatIds.length) +} + +describe('row-model pipeline flatRows ordering', () => { + it('keeps parents before descendants through every hierarchical stage', () => { + const table = constructTable({ + features, + columns, + data, + getSubRows: (row) => row.subRows, + initialState: { + columnFilters: [{ id: 'name', value: 'keep' }], + expanded: true, + grouping: ['group'], + pagination: { pageIndex: 0, pageSize: 1 }, + sorting: [{ id: 'name', desc: false }], + }, + }) + + const models = [ + table.getCoreRowModel(), + table.getFilteredRowModel(), + table.getGroupedRowModel(), + table.getSortedRowModel(), + table.getExpandedRowModel(), + table.getPaginatedRowModel(), + ] + + for (let i = 0; i < models.length; i++) { + expectPreorder(models[i]!) + } + }) +}) diff --git a/packages/table-core/tests/implementation/features/column-grouping/createGroupedRowModel.test.ts b/packages/table-core/tests/implementation/features/column-grouping/createGroupedRowModel.test.ts index da511b8d92..a51b8dae06 100644 --- a/packages/table-core/tests/implementation/features/column-grouping/createGroupedRowModel.test.ts +++ b/packages/table-core/tests/implementation/features/column-grouping/createGroupedRowModel.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it, vi } from 'vitest' import { - rowAggregationFeature, aggregationFns, columnGroupingFeature, constructTable, createGroupedRowModel, + rowAggregationFeature, } from '../../../../src' import { testFeatures } from '../../../fixtures/features' import { generateTestData } from '../../../fixtures/data/generateTestData' @@ -67,6 +67,15 @@ describe('createGroupedRowModel flatRows contain every row exactly once', () => expect(rowModel.flatRows.length).toBe(7) expectUniqueFlatRowIds(rowModel) expect(Object.keys(rowModel.rowsById).length).toBe(7) + expect(rowModel.flatRows.map((row) => row.id)).toEqual([ + 'status:a', + '0', + '1', + '2', + 'status:b', + '3', + '4', + ]) }) it('two-level grouping over flat data', () => { @@ -89,6 +98,17 @@ describe('createGroupedRowModel flatRows contain every row exactly once', () => expect(ids.has('status:a>firstName:x')).toBe(true) expect(ids.has('status:a>firstName:y')).toBe(true) expect(ids.has('status:b>firstName:x')).toBe(true) + expect(rowModel.flatRows.map((row) => row.id)).toEqual([ + 'status:a', + 'status:a>firstName:x', + '0', + '1', + 'status:a>firstName:y', + '2', + 'status:b', + 'status:b>firstName:x', + '3', + ]) }) it('single-level grouping over tree data keeps descendants below the terminal depth exactly once', () => { @@ -124,6 +144,24 @@ describe('createGroupedRowModel flatRows contain every row exactly once', () => expect(rowModel.rowsById['0']!.depth).toBe(1) expect(rowModel.rowsById['0.0']!.depth).toBe(2) expect(rowModel.rowsById['0.0.0']!.depth).toBe(3) + expect(rowModel.flatRows.map((row) => row.id)).toEqual([ + 'status:single', + '0', + '0.0', + '0.0.0', + '0.0.1', + '0.1', + '0.1.0', + '0.1.1', + 'status:complicated', + '1', + '1.0', + '1.0.0', + '1.0.1', + '1.1', + '1.1.0', + '1.1.1', + ]) }) it('groups rows with undefined grouping values exactly once', () => { diff --git a/packages/table-core/tests/unit/worker/serializeRebuild.test.ts b/packages/table-core/tests/unit/worker/serializeRebuild.test.ts index b6b8acd052..5c177e8511 100644 --- a/packages/table-core/tests/unit/worker/serializeRebuild.test.ts +++ b/packages/table-core/tests/unit/worker/serializeRebuild.test.ts @@ -4,12 +4,14 @@ import { columnFilteringFeature, columnGroupingFeature, constructTable, + createExpandedRowModel, createFilteredRowModel, createGroupedRowModel, createSortedRowModel, filterFns, globalFilteringFeature, rowAggregationFeature, + rowExpandingFeature, rowSortingFeature, sortFns, } from '../../../src' @@ -44,7 +46,9 @@ const features = testFeatures({ columnFilteringFeature, columnGroupingFeature, globalFilteringFeature, + rowExpandingFeature, rowSortingFeature, + expandedRowModel: createExpandedRowModel(), filteredRowModel: createFilteredRowModel(), groupedRowModel: createGroupedRowModel(), sortedRowModel: createSortedRowModel(), @@ -268,6 +272,7 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { expect(payload.kind).toBe('tree') expect(ids(rebuilt.rows)).toEqual(ids(model.rows)) expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(rebuilt.flatRows[0]).toBe(rebuilt.rows[0]) 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), @@ -507,6 +512,9 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { const firstLeaf = firstSubGroup.subRows[0]! expect(firstLeaf.depth).toBe(2) expect(firstLeaf.parentId).toBe(firstSubGroup.id) + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + expect(rebuilt.flatRows[0]).toBe(firstGroup) + expect(rebuilt.flatRows[1]).toBe(firstSubGroup) }) it('round-trips parent-first sorted flatRows through nested groups', () => { @@ -523,6 +531,19 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => { expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) }) + it('round-trips parent-first expanded flatRows through groups', () => { + const data = makeData(12) + const workerTable = makeTable(data) + const mainTable = makeTable(data) + workerTable.baseAtoms.grouping.set(['status']) + workerTable.baseAtoms.expanded.set(true) + + const model = workerTable.getExpandedRowModel() + const { rebuilt } = roundTrip(workerTable, mainTable, model, 'expanded') + + expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows)) + }) + it('does not reset depths when rebuilding a filtered payload (regression)', () => { const data = makeData(12) const workerTable = makeTable(data) diff --git a/perf-new.md b/perf-new.md index e76d7fe171..ddb26ad7d5 100644 --- a/perf-new.md +++ b/perf-new.md @@ -55,7 +55,7 @@ medians under `warmups: 0` — measurement noise (see N5). The two real non-wins **Implementation note:** Fixed 2026-07-03 with the exactly-once push scheme described below: the terminal-branch push (:86–87) was removed and replaced with a post-recursion push of the reassigned `row.subRows` (covers descendants below terminal depth); the parent-group and root -loops are unchanged, so every row is pushed by its parent (or the root loop) exactly once. +loops were unchanged, so every row was pushed by its parent (or the root loop) exactly once. Design review verified the scheme for single/multi-level grouping, flat and tree data, undefined grouping values, AND a bonus instance the original finding missed: grouping on only-nonexistent column ids (`existingGrouping.length === 0` after filtering) reached the terminal branch at depth @@ -66,10 +66,17 @@ in-repo consumer reads that order; the sorted model already emits postorder), an forwards grouped flatRows by reference) — correct, flagged for the release note. Regression coverage in `tests/implementation/features/column-grouping/createGroupedRowModel.test.ts` (single-level flat 12→7, two-level flat 13→9, tree-below-terminal 18→16, undefined grouping -values 8→5, nonexistent-column grouping 6→3 flatRows; all with duplicate-id checks). The +values 8→5, nonexistent-column grouping 6→3 flatRows; all with duplicate-id checks). A +2026-08-25 follow-up changed terminal rows to push before descending and synthetic group rows to +reserve their flat-array position before their descendants are built. This removes accepted +delta (a): group rows and tree data now appear before their descendants, matching core, +filtered, sorted, and paginated `flatRows`, without adding another traversal. Exact single-level, +multi-level, tree-data, and worker round-trip ordering tests cover the corrected contract. The +worker expanded-stage rebuild also deduplicates and restores preorder so the corrected grouping +order survives the downstream pipeline. Accepted delta (b), the exactly-once row count, remains. The benchmark comparison layer gained a known-delta allowlist annotating the intentional v8↔v9 `outputFlatRows` mismatch for grouping scenarios. -**Location:** `packages/table-core/src/features/column-grouping/createGroupedRowModel.ts:86–87` (terminal-branch push) and `:179–182` (parent group's subRows push); v8 has the identical double-push in `table-v8/packages/table-core/src/utils/getGroupedRowModel.ts` +**Location:** `packages/table-core/src/features/column-grouping/createGroupedRowModel.ts`; v8 has the identical double-push in `table-v8/packages/table-core/src/utils/getGroupedRowModel.ts` **Category:** `bug`, `allocation`, `big-o` **Benchmark evidence:** for R=400,000 flat rows grouped into 20 groups, `outputFlatRowsMedian`