Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/great-pugs-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/table-core': patch
---

Fix `getSortedRowModel().flatRows` listing sub-rows before their parent. This restores the parent-first order used by the v8 sorted model and aligns the flattened result with the sorted `rows` tree, core row model, and paginated row model.
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ function _createSortedRowModel<
changed = true
}

// Take this row's slot before descending, so a parent stays ahead of
// its own sub-rows in flatRows.
const flatIndex = sortedFlatRows.length
sortedFlatRows.push(row)

if (row.subRows.length) {
const sortedSubRows = sortData(row.subRows)

Expand All @@ -172,13 +177,9 @@ function _createSortedRowModel<
copyInstancePropertiesWithoutMemos(cloned, row)
cloned.subRows = sortedSubRows.rows
sortedData[i] = cloned
sortedFlatRows.push(cloned)
sortedFlatRows[flatIndex] = cloned
changed = true
} else {
sortedFlatRows.push(row)
}
} else {
sortedFlatRows.push(row)
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/table-core/src/worker/createWorkerRowModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export function createWorkerRowModel(
}
}
}
return rebuildRowModel(table, payload, stage !== 'filtered')
return rebuildRowModel(table, payload, stage)
},
})

Expand Down
28 changes: 20 additions & 8 deletions packages/table-core/src/worker/rebuildRowModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { TableFeatures } from '../types/TableFeatures'
import type { RowData } from '../types/type-utils'
import type {
TableWorkerRowNode,
TableWorkerStage,
TableWorkerStagePayload,
} from './tableWorkerProtocol'

Expand Down Expand Up @@ -38,15 +39,14 @@ export function rebuildRowModel<
>(
table: Table_Internal<TFeatures, TData>,
payload: TableWorkerDataPayload,
/**
* Whether a flat payload should rewrite row depth/parentId. Mirrors core:
* the grouped model's passthrough resets them, the filtered model never
* touches them. Without this distinction a filtered rebuild could zero the
* depths a grouped/sorted tree rebuild just assigned to shared row objects.
*/
resetDepths: boolean,
stage: TableWorkerStage,
): RowModel<TFeatures, TData> {
const core = table.getCoreRowModel()
// The grouped model's flat passthrough resets row relationships, while the
// 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'

if (payload.kind === 'flat') {
const { indices } = payload
Expand Down Expand Up @@ -84,6 +84,14 @@ export function rebuildRowModel<
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.
const flatIndex = flattenParentsFirst ? flatRows.length : -1
if (flattenParentsFirst) {
flatRows.push(undefined)
}

const subRows = rebuildRows(node.children, depth + 1, node.id)
const leafRows: Array<any> = []
collectLeafRows(subRows, leafRows)
Expand All @@ -107,7 +115,11 @@ export function rebuildRowModel<
hasOwn(aggregates, columnId) ? aggregates[columnId] : undefined,
})

flatRows.push(row)
if (flattenParentsFirst) {
flatRows[flatIndex] = row
} else {
flatRows.push(row)
}
rowsById[node.id] = row
rows[i] = row
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
rowSortingFeature,
} from '../../../../src'
import { testFeatures } from '../../../fixtures/features'
import type { ColumnDef } from '../../../../src'
import type { ColumnDef, Row } from '../../../../src'

type Person = {
firstName: string
Expand Down Expand Up @@ -42,6 +42,22 @@ function makeTable(
})
}

function flattenRows(rows: Array<Row<typeof features, Person>>) {
const flatRows: Array<Row<typeof features, Person>> = []

const visit = (nestedRows: Array<Row<typeof features, Person>>) => {
for (const row of nestedRows) {
flatRows.push(row)
if (row.subRows.length) {
visit(row.subRows)
}
}
}

visit(rows)
return flatRows
}

describe('createSortedRowModel', () => {
it('does not crash when the sorting state references a column that no longer exists', () => {
const table = makeTable([{ id: 'thisColumnDoesNotExist', desc: false }])
Expand Down Expand Up @@ -210,6 +226,84 @@ describe('createSortedRowModel', () => {
).toEqual(['child-a', 'child-b'])
})

it('flattens each parent ahead of its own sub-rows', () => {
const table = makeTable(
[{ id: 'age', desc: false }],
[
{
firstName: 'older-parent',
age: 20,
subRows: [
{
firstName: 'child-b',
age: 15,
subRows: [
{ firstName: 'grandchild-b', age: 4 },
{ firstName: 'grandchild-a', age: 2 },
],
},
{ firstName: 'child-a', age: 10 },
],
},
{
firstName: 'younger-parent',
age: 5,
subRows: [{ firstName: 'child-c', age: 1 }],
},
],
)

expect(
table.getSortedRowModel().flatRows.map((row) => row.original.firstName),
).toEqual([
'younger-parent',
'child-c',
'older-parent',
'child-a',
'child-b',
'grandchild-a',
'grandchild-b',
])
})

it('flattens nested branch clones rather than the rows they replaced', () => {
const table = makeTable(
[{ id: 'age', desc: false }],
[
{
firstName: 'parent',
age: 20,
subRows: [
{
firstName: 'child-b',
age: 15,
subRows: [
{ firstName: 'grandchild-b', age: 4 },
{ firstName: 'grandchild-a', age: 2 },
],
},
{ firstName: 'child-a', age: 10 },
],
},
],
)

const preSortedRowModel = table.getPreSortedRowModel()
const sortedRowModel = table.getSortedRowModel()
const recursivelyFlattenedRows = flattenRows(sortedRowModel.rows)

expect(sortedRowModel.flatRows).toHaveLength(
recursivelyFlattenedRows.length,
)
for (let i = 0; i < recursivelyFlattenedRows.length; i++) {
expect(sortedRowModel.flatRows[i]).toBe(recursivelyFlattenedRows[i])
}
expect(sortedRowModel.rows[0]).not.toBe(preSortedRowModel.rows[0])
expect(sortedRowModel.rows[0]!.subRows[1]).not.toBe(
preSortedRowModel.rows[0]!.subRows[0],
)
})

describe('sortUndefined', () => {
type MaybePerson = {
firstName: string
Expand Down
65 changes: 54 additions & 11 deletions packages/table-core/tests/unit/worker/serializeRebuild.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { describe, expect, it } from 'vitest'
import {
aggregationFns,
rowAggregationFeature,
columnFilteringFeature,
columnGroupingFeature,
constructTable,
Expand All @@ -10,6 +9,7 @@ import {
createSortedRowModel,
filterFns,
globalFilteringFeature,
rowAggregationFeature,
rowSortingFeature,
sortFns,
} from '../../../src'
Expand All @@ -18,6 +18,7 @@ import { serializeRowModel } from '../../../src/worker/serializeRowModel'
import { rebuildRowModel } from '../../../src/worker/rebuildRowModel'
import { makeObjectMap } from '../../../src/utils'
import type { ColumnDef, RowModel, Table } from '../../../src'
import type { TableWorkerStage } from '../../../src/worker/tableWorkerProtocol'

type Person = {
firstName: string
Expand Down Expand Up @@ -83,7 +84,7 @@ function roundTrip(
workerTable: Table<typeof features, Person>,
mainTable: Table<typeof features, Person>,
model: RowModel<typeof features, Person>,
resetDepths = true,
stage: TableWorkerStage,
) {
const transfer: Array<Transferable> = []
const payload = serializeRowModel(
Expand All @@ -97,7 +98,7 @@ function roundTrip(
}
return {
payload,
rebuilt: rebuildRowModel(mainTable, payload, resetDepths),
rebuilt: rebuildRowModel(mainTable, payload, stage),
}
}

Expand All @@ -111,10 +112,16 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
workerTable.baseAtoms.sorting.set([{ id: 'age', desc: true }])

const model = workerTable.getSortedRowModel()
const { payload, rebuilt } = roundTrip(workerTable, mainTable, model)
const { payload, rebuilt } = roundTrip(
workerTable,
mainTable,
model,
'sorted',
)

expect(payload.kind).toBe('flat')
expect(ids(rebuilt.rows)).toEqual(ids(model.rows))
expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows))
// rows are the main table's own core rows, not clones
expect(rebuilt.rows[0]!).toBe(
mainTable.getCoreRowModel().flatRows[Number(model.rows[0]!.id)],
Expand All @@ -129,12 +136,18 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
workerTable.baseAtoms.columnFilters.set([{ id: 'status', value: 'single' }])

const model = workerTable.getFilteredRowModel()
const { payload, rebuilt } = roundTrip(workerTable, mainTable, model)
const { payload, rebuilt } = roundTrip(
workerTable,
mainTable,
model,
'filtered',
)

expect(payload.kind).toBe('flat')
expect(model.rows.length).toBeGreaterThan(0)
expect(model.rows.length).toBeLessThan(data.length)
expect(ids(rebuilt.rows)).toEqual(ids(model.rows))
expect(ids(rebuilt.flatRows)).toEqual(ids(model.flatRows))
})

it('round-trips a grouped model as a tree with aggregates', () => {
Expand All @@ -144,7 +157,12 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
workerTable.baseAtoms.grouping.set(['status'])

const model = workerTable.getGroupedRowModel()
const { payload, rebuilt } = roundTrip(workerTable, mainTable, model)
const { payload, rebuilt } = roundTrip(
workerTable,
mainTable,
model,
'grouped',
)

expect(payload.kind).toBe('tree')
expect(ids(rebuilt.rows)).toEqual(ids(model.rows))
Expand Down Expand Up @@ -190,10 +208,16 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
workerTable.baseAtoms.sorting.set([{ id: 'visits', desc: false }])

const model = workerTable.getSortedRowModel()
const { payload, rebuilt } = roundTrip(workerTable, mainTable, model)
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))
// leaves within each group are in the worker's sorted order
for (let i = 0; i < model.rows.length; i++) {
expect(ids(rebuilt.rows[i]!.subRows)).toEqual(ids(model.rows[i]!.subRows))
Expand All @@ -207,7 +231,7 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
workerTable.baseAtoms.grouping.set(['status', 'age'])

const model = workerTable.getGroupedRowModel()
const { rebuilt } = roundTrip(workerTable, mainTable, model)
const { rebuilt } = roundTrip(workerTable, mainTable, model, 'grouped')

expect(ids(rebuilt.rows)).toEqual(ids(model.rows))
const firstGroup = rebuilt.rows[0]!
Expand All @@ -219,6 +243,20 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {
expect(firstLeaf.parentId).toBe(firstSubGroup.id)
})

it('round-trips parent-first sorted flatRows through nested groups', () => {
const data = makeData(18)
const workerTable = makeTable(data)
const mainTable = makeTable(data)
workerTable.baseAtoms.grouping.set(['status', 'age'])
workerTable.baseAtoms.sorting.set([{ id: 'visits', desc: false }])

const model = workerTable.getSortedRowModel()
const { rebuilt } = roundTrip(workerTable, mainTable, model, 'sorted')

expect(ids(rebuilt.rows)).toEqual(ids(model.rows))
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)
Expand All @@ -227,17 +265,22 @@ describe('serializeRowModel -> rebuildRowModel round trip', () => {

// Tree rebuild assigns leaf depths on the main table's shared row objects
const grouped = workerTable.getGroupedRowModel()
const { rebuilt: rebuiltTree } = roundTrip(workerTable, mainTable, grouped)
const { rebuilt: rebuiltTree } = roundTrip(
workerTable,
mainTable,
grouped,
'grouped',
)
const someLeaf = rebuiltTree.rows[0]!.subRows[0]!
expect(someLeaf.depth).toBe(1)

// A filtered (flat) rebuild afterward must not zero those depths
const filtered = workerTable.getFilteredRowModel()
roundTrip(workerTable, mainTable, filtered, false)
roundTrip(workerTable, mainTable, filtered, 'filtered')
expect(someLeaf.depth).toBe(1)

// ...while a grouped-or-later flat passthrough does reset them
roundTrip(workerTable, mainTable, filtered, true)
roundTrip(workerTable, mainTable, filtered, 'grouped')
expect(someLeaf.depth).toBe(0)
})
})
Loading
Loading