Skip to content
Open
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/stale-accessor-values.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/table-core': patch
---

Clear row value caches when column definitions are replaced, so a column whose `accessorFn` changes no longer serves the value produced by the previous accessor.
31 changes: 31 additions & 0 deletions packages/table-core/src/core/table/coreTablesFeature.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,34 @@ export function table_mergeOptions<
) as TableOptions<TFeatures, TData>
}

/**
* Clears every built row's accessor-value caches. Both are keyed by column id
* and rows outlive column-definition swaps, so a replaced `accessorFn` would
* otherwise never be read again. Emptied in place because sorted and grouped
* row clones share these objects by reference.
*/
function table_clearRowValueCaches<
TFeatures extends TableFeatures,
TData extends RowData,
>(table: Table_Internal<TFeatures, TData>): void {
if (!table._rowModels.coreRowModel) {
return
}

const rows = table.getCoreRowModel().flatRows

for (let i = 0; i < rows.length; i++) {
const row = rows[i]!

for (const key in row._valuesCache) {
delete row._valuesCache[key]
}
for (const key in row._uniqueValuesCache) {
delete row._uniqueValuesCache[key]
}
}
}

/**
* Updates the table options object.
*
Expand Down Expand Up @@ -213,6 +241,9 @@ export function table_setOptions<
table.options as TableOptions<TFeatures, TData>,
)
const mergedOptions = table_mergeOptions(table, newOptions)
if (mergedOptions.columns !== table.options.columns) {
table_clearRowValueCaches(table)
}

if (table.optionsStore) {
table.optionsStore.set(() => mergedOptions)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ function makeNestedTable(
})
}

function makeAccessorTable(accessorFn: (person: Person) => string) {
return constructTable({
features,
data: generateTestData(1),
columns: [{ id: 'name', accessorFn }],
})
}

describe('row_getValue', () => {
it('should read and cache the accessor value', () => {
const table = makeTable(1)
Expand All @@ -113,6 +121,62 @@ describe('row_getValue', () => {

expect(row_getValue(row, 'not-a-column')).toBeUndefined()
})

it('should re-read the value when the column accessor is replaced', () => {
const table = makeAccessorTable((person) => `${person.firstName} last`)
const row = table.getCoreRowModel().rows[0]!
const original = row_getValue(row, 'name')

table.setOptions((old) => ({
...old,
columns: [
{
id: 'name',
accessorFn: (person: Person) => `${person.firstName} replaced`,
},
],
}))

expect(table.getCoreRowModel().rows[0]!).toBe(row)
expect(row_getValue(row, 'name')).not.toBe(original)
expect(row_getValue(row, 'name')).toBe(`${row.original.firstName} replaced`)
})

it('should not expose stale values to subscribers notified during the options update', () => {
const table = makeAccessorTable(() => 'original')
const row = table.getCoreRowModel().rows[0]!
row_getValue(row, 'name')

const observed: Array<unknown> = []
const subscription = table.optionsStore!.subscribe(() => {
observed.push(row_getValue(row, 'name'))
})

table.setOptions((old) => ({
...old,
columns: [{ id: 'name', accessorFn: () => 'replaced' }],
}))
subscription.unsubscribe()

expect(observed).toEqual(['replaced'])
})

it('should call the replacement accessor exactly once per read cycle', () => {
const table = makeAccessorTable(() => 'original')
const row = table.getCoreRowModel().rows[0]!
row_getValue(row, 'name')

const replacement = vi.fn(() => 'replaced')
table.setOptions((old) => ({
...old,
columns: [{ id: 'name', accessorFn: replacement }],
}))

row_getValue(row, 'name')
row_getValue(row, 'name')

expect(replacement).toHaveBeenCalledTimes(1)
})
})

describe('row_getUniqueValues', () => {
Expand All @@ -139,6 +203,20 @@ describe('row_getUniqueValues', () => {
expect(getUniqueValues).toHaveBeenCalledTimes(1)
})

it('should re-read unique values when the column accessor is replaced', () => {
const table = makeAccessorTable(() => 'original')
const row = table.getCoreRowModel().rows[0]!

expect(row_getUniqueValues(row, 'name')).toEqual(['original'])

table.setOptions((old) => ({
...old,
columns: [{ id: 'name', accessorFn: () => 'replaced' }],
}))

expect(row_getUniqueValues(row, 'name')).toEqual(['replaced'])
})

it('should return undefined for unknown columns', () => {
const table = makeTable(1)
const row = table.getRowModel().rows[0]!
Expand Down