-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(react-table): fix infinite rerender issue in useLegacyTable #6202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,8 +15,8 @@ import { | |||||||
| sortFns, | ||||||||
| stockFeatures, | ||||||||
| } from '@tanstack/table-core' | ||||||||
| import { useMemo } from 'react' | ||||||||
| import { useStore } from '@tanstack/react-store' | ||||||||
| import { useCallback, useMemo } from 'react' | ||||||||
| import { shallow, useStore } from '@tanstack/react-store' | ||||||||
|
Comment on lines
+18
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Remove unused imports.
🧹 Proposed fix to remove unused imports-import { useCallback, useMemo } from 'react'
-import { shallow, useStore } from '@tanstack/react-store'
+import { useMemo } from 'react'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| import { useTable } from './useTable' | ||||||||
| import type { | ||||||||
| Cell, | ||||||||
|
|
@@ -419,20 +419,21 @@ export function useLegacyTable<TData extends RowData>( | |||||||
| } | ||||||||
|
|
||||||||
| // Call useTable with the v9 API, subscribing to all state changes | ||||||||
| const table = useTable<StockFeatures, TData, TableState<StockFeatures>>({ | ||||||||
| ...restOptions, | ||||||||
| _features: stockFeatures, | ||||||||
| _rowModels, | ||||||||
| } as TableOptions<StockFeatures, TData>) | ||||||||
|
|
||||||||
| const state = useStore(table.store, (state) => state) | ||||||||
| const table = useTable<StockFeatures, TData, TableState<StockFeatures>>( | ||||||||
| { | ||||||||
| ...restOptions, | ||||||||
| _features: stockFeatures, | ||||||||
| _rowModels, | ||||||||
| } as TableOptions<StockFeatures, TData>, | ||||||||
| (state) => state, | ||||||||
| ) | ||||||||
|
|
||||||||
| return useMemo( | ||||||||
| () => | ||||||||
| ({ | ||||||||
| ...table, | ||||||||
| getState: () => state, | ||||||||
| getState: () => table.state, | ||||||||
| }) as LegacyReactTable<TData>, | ||||||||
| [table, state], | ||||||||
| [table], | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to remember why I even did had a dedicated useStore call here, I think it might have had something to do with the react compiler. I'm fine shipping this though
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmh could it be because previously there were some updates done in useEffect with queueMicrotask to update the baseStore? That's something we removed after
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyway, I just tested the useLegacyTable example with this configuration and it seems working the same |
||||||||
| ) | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, this was needed.