fix: preserve data table state across prop updates - #106
Merged
Conversation
Paginating, sorting or searching a DataTable was undone whenever the consumer re-rendered. Page to 2 and click a row and the table jumps back to page 1 -- clicking a row typically sets state to highlight it, and that re-render was enough, though nothing about the table's data changed. Two independent causes, both required: 1. store.ts -- reset()'s client branch set `state: getTanstackTableState(updatedParams)`, rebuilding the whole TableState from props-time defaults (pageIndex 0, empty sizing, initialState filters and sorting). DataTable keys its effect on [props], a fresh rest-spread object each render, so this ran on every render. 2. DataTableControls.tsx -- the effect was keyed on [onSearchChange, searchValue]. Callers write onSearchChange inline, so it is a new function every render and the effect replayed the handler with an unchanged search value. A handler that writes filter state then handed Tanstack a new columnFilters identity, tripping autoResetPageIndex. Cause 2 is why 976be28 (released as 6.9.2) looked ineffective and was reverted: it addressed only cause 1. reset() no longer touches live table state; initialState now applies once, at store creation. Column pinning is the one slice derived from props, so reset reconciles just that via the extracted getColumnPinningWithActions, shared with getTanstackTableState so the create and update paths cannot drift. The search handler is held in a ref so its effect keys on searchValue alone. Memoizing columns is not an alternative: cell closures capture consumer state, so columns is legitimately new each render and skipping the update would leave stale cells rendering. The update was always correct; only its side effect on state was wrong. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🎉 This PR is included in version 6.9.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paginating, sorting or searching a
DataTablewas undone as soon as the consumer re-rendered. Most visibly: page to 2 and click a row, and the table jumps back to page 1. Clicking a row typically sets state to highlight it, and that re-render was enough — nothing about the table's data changed.Cause
Two independent causes, both required:
store.ts—reset()'s client branch setstate: getTanstackTableState(updatedParams), which builds aTableStatefrom scratch (pageIndex: 0, empty sizing,initialStatefilters and sorting). Every call replaced pagination, sorting, filters and column sizing with props-time defaults.DataTablekeys its effect on[props], a fresh rest-spread object each render, so this ran on every render. The server branch never did this.DataTableControls.tsx— the effect was keyed on[onSearchChange, searchValue]. Callers writeonSearchChangeinline, so it is a new function every render and the effect replayed the handler with an unchanged search value. A handler that writes filter state (the common shape,setFilterValue(prev => ({ ...prev, searchString }))) then handed Tanstack a newcolumnFiltersidentity, trippingautoResetPageIndex.Cause 2 is why the earlier fix (976be28, released as 6.9.2) looked ineffective and was reverted — it addressed only cause 1, so row clicks still reset the page in any table using
onSearchChange.Change
store.ts—reset()no longer touches live table state. OnesetOptionscall for both modes updates the genuine inputs (columns,data,meta, pluspageCountfor server mode).initialStatenow applies once, at store creation.utils.tsx— extractedgetColumnPinningWithActions. Column pinning is the one slice of state derived from props, soresetreconciles just that via asetTableStateupdater; sharing the helper withgetTanstackTableStatekeeps the create and update paths from drifting.DataTableControls.tsx— the handler is held in a ref so the effect is keyed onsearchValuealone. Mount-time invocation is unchanged.Memoizing
columnsis not an alternative:cellclosures capture consumer state, socolumnsis legitimately new each render and skipping the update would leave stale cells rendering. The update was always correct; only its side effect on state was wrong.Behaviour changes
columnVisibilitynow persists. Previously any consumer re-render restored all columns to visible, sotogglesComponentselections were silently reverted. They now stick.initialStateis now genuinely initial. Changes to it after mount are ignored. Previously prop-timecolumnFilters/sortingwere reapplied on every render.rowActionsnow unpins the actions column in server mode too; the old server branch never reconciled pinning.Columns changing under us
Verified safe against
@tanstack/table-core@8.21.3— every column-keyed state slice tolerates ids that no longer exist:getSortedRowModel.ts:22— filters out sortings for non-existent columnsgetFilteredRowModel.ts:32—if (!column) returnColumnPinning.ts:302-320—.map(...).filter(Boolean)on left/right/centercolumnSizingis fully recomputed byupdateColumnSizing()on every reset, so no stale entries survive. Removing a sorted column, removing a filtered column, and droppingrowActionsmid-life were each exercised and behave correctly. StalecolumnFilters/sorting/columnVisibilityentries do persist for removed columns and will reapply if that column id returns — generally the desired behaviour.Verification
177/177 tests pass,
tscandeslintclean. Three regression tests added in__tests__/DataTable.spec.tsx— page survives a row click; survives it with an inlineonSearchChangethat writes filter state; search survives it — all three fail before the fix.Known gaps (follow-ups, not regressions)
datashrinks. On page 3 of 50 rows, swapping in 3 rows renders 0 rows for one commit beforeautoResetPageIndex's queued microtask (table.ts:334) lands;DataTableBody.tsx:20shows the empty state at zero rows. Pre-fix it showed 3 rows immediately. A synchronous clamp to page 0 inreset()closes this with no semantic change, sinceresetPageIndex()lands on 0 anyway (RowPagination.ts:273).data. With an unmemoized array (data={rows.map(toDisplay)}), a row click still bounces to page 1 —dataidentity change tripsautoResetPageIndex. This is unchanged from before and not a regression, but it limits the fix's reach. Properly addressing it meansautoResetPageIndex: falseplus explicit page-reset policy in the store, which also changes what happens whendatais swapped wholesale — a deliberate change deserving its own PR.