Skip to content

Commit 7390858

Browse files
fix(table-core): clear row value caches when column defs are replaced
Rows outlive column-definition swaps, so a column whose accessorFn changed kept serving the previous accessor's value. Fixes #5363
1 parent d01c01b commit 7390858

3 files changed

Lines changed: 114 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/table-core': patch
3+
---
4+
5+
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.

packages/table-core/src/core/table/coreTablesFeature.utils.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,34 @@ export function table_mergeOptions<
186186
) as TableOptions<TFeatures, TData>
187187
}
188188

189+
/**
190+
* Clears every built row's accessor-value caches. Both are keyed by column id
191+
* and rows outlive column-definition swaps, so a replaced `accessorFn` would
192+
* otherwise never be read again. Emptied in place because sorted and grouped
193+
* row clones share these objects by reference.
194+
*/
195+
function table_clearRowValueCaches<
196+
TFeatures extends TableFeatures,
197+
TData extends RowData,
198+
>(table: Table_Internal<TFeatures, TData>): void {
199+
if (!table._rowModels.coreRowModel) {
200+
return
201+
}
202+
203+
const rows = table.getCoreRowModel().flatRows
204+
205+
for (let i = 0; i < rows.length; i++) {
206+
const row = rows[i]!
207+
208+
for (const key in row._valuesCache) {
209+
delete row._valuesCache[key]
210+
}
211+
for (const key in row._uniqueValuesCache) {
212+
delete row._uniqueValuesCache[key]
213+
}
214+
}
215+
}
216+
189217
/**
190218
* Updates the table options object.
191219
*
@@ -213,6 +241,9 @@ export function table_setOptions<
213241
table.options as TableOptions<TFeatures, TData>,
214242
)
215243
const mergedOptions = table_mergeOptions(table, newOptions)
244+
if (mergedOptions.columns !== table.options.columns) {
245+
table_clearRowValueCaches(table)
246+
}
216247

217248
if (table.optionsStore) {
218249
table.optionsStore.set(() => mergedOptions)

packages/table-core/tests/unit/core/rows/coreRowsFeature.utils.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ function makeNestedTable(
9696
})
9797
}
9898

99+
function makeAccessorTable(accessorFn: (person: Person) => string) {
100+
return constructTable({
101+
features,
102+
data: generateTestData(1),
103+
columns: [{ id: 'name', accessorFn }],
104+
})
105+
}
106+
99107
describe('row_getValue', () => {
100108
it('should read and cache the accessor value', () => {
101109
const table = makeTable(1)
@@ -113,6 +121,62 @@ describe('row_getValue', () => {
113121

114122
expect(row_getValue(row, 'not-a-column')).toBeUndefined()
115123
})
124+
125+
it('should re-read the value when the column accessor is replaced', () => {
126+
const table = makeAccessorTable((person) => `${person.firstName} last`)
127+
const row = table.getCoreRowModel().rows[0]!
128+
const original = row_getValue(row, 'name')
129+
130+
table.setOptions((old) => ({
131+
...old,
132+
columns: [
133+
{
134+
id: 'name',
135+
accessorFn: (person: Person) => `${person.firstName} replaced`,
136+
},
137+
],
138+
}))
139+
140+
expect(table.getCoreRowModel().rows[0]!).toBe(row)
141+
expect(row_getValue(row, 'name')).not.toBe(original)
142+
expect(row_getValue(row, 'name')).toBe(`${row.original.firstName} replaced`)
143+
})
144+
145+
it('should not expose stale values to subscribers notified during the options update', () => {
146+
const table = makeAccessorTable(() => 'original')
147+
const row = table.getCoreRowModel().rows[0]!
148+
row_getValue(row, 'name')
149+
150+
const observed: Array<unknown> = []
151+
const subscription = table.optionsStore!.subscribe(() => {
152+
observed.push(row_getValue(row, 'name'))
153+
})
154+
155+
table.setOptions((old) => ({
156+
...old,
157+
columns: [{ id: 'name', accessorFn: () => 'replaced' }],
158+
}))
159+
subscription.unsubscribe()
160+
161+
expect(observed).toEqual(['replaced'])
162+
})
163+
164+
it('should call the replacement accessor exactly once per read cycle', () => {
165+
const table = makeAccessorTable(() => 'original')
166+
const row = table.getCoreRowModel().rows[0]!
167+
row_getValue(row, 'name')
168+
169+
const replacement = vi.fn(() => 'replaced')
170+
table.setOptions((old) => ({
171+
...old,
172+
columns: [{ id: 'name', accessorFn: replacement }],
173+
}))
174+
175+
row_getValue(row, 'name')
176+
row_getValue(row, 'name')
177+
178+
expect(replacement).toHaveBeenCalledTimes(1)
179+
})
116180
})
117181

118182
describe('row_getUniqueValues', () => {
@@ -139,6 +203,20 @@ describe('row_getUniqueValues', () => {
139203
expect(getUniqueValues).toHaveBeenCalledTimes(1)
140204
})
141205

206+
it('should re-read unique values when the column accessor is replaced', () => {
207+
const table = makeAccessorTable(() => 'original')
208+
const row = table.getCoreRowModel().rows[0]!
209+
210+
expect(row_getUniqueValues(row, 'name')).toEqual(['original'])
211+
212+
table.setOptions((old) => ({
213+
...old,
214+
columns: [{ id: 'name', accessorFn: () => 'replaced' }],
215+
}))
216+
217+
expect(row_getUniqueValues(row, 'name')).toEqual(['replaced'])
218+
})
219+
142220
it('should return undefined for unknown columns', () => {
143221
const table = makeTable(1)
144222
const row = table.getRowModel().rows[0]!

0 commit comments

Comments
 (0)