fix(react-table): fix infinite rerender issue in useLegacyTable#6202
fix(react-table): fix infinite rerender issue in useLegacyTable#6202KevinVandy merged 1 commit intoalphafrom
Conversation
|
|
View your CI Pipeline Execution ↗ for commit 00feb71
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughTwo related state subscription updates: Subscribe.ts adds shallow equality comparison to store subscriptions, while useLegacyTable.ts refactors to pass state selectors directly to useTable, eliminating a separate state variable and simplifying the subscription pattern. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| getState: () => table.state, | ||
| }) as LegacyReactTable<TData>, | ||
| [table, state], | ||
| [table], |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 state store updates automatically when options changes.
There was a problem hiding this comment.
Anyway, I just tested the useLegacyTable example with this configuration and it seems working the same
react({
babel: {
plugins: ['babel-plugin-react-compiler'],
},
}),
| props: SubscribeProps<TFeatures, TData, TSelected>, | ||
| ): ReturnType<FunctionComponent> { | ||
| const selected = useStore(props.table.store, props.selector) | ||
| const selected = useStore(props.table.store, props.selector, shallow) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-table/src/useLegacyTable.ts`:
- Around line 18-19: The file imports unused symbols—remove the unused imports
useCallback, shallow, and useStore from the import lines in useLegacyTable.ts;
specifically delete useCallback from the 'react' import and remove shallow and
useStore from the '@tanstack/react-store' import so only actually used exports
remain, then run a quick lint/type-check to confirm no references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7b5b707-7ade-4df7-9d6c-dad090172eed
📒 Files selected for processing (2)
packages/react-table/src/Subscribe.tspackages/react-table/src/useLegacyTable.ts
| import { useCallback, useMemo } from 'react' | ||
| import { shallow, useStore } from '@tanstack/react-store' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused imports.
useCallback, shallow, and useStore are imported but not used in this file. These appear to be leftovers from the refactoring.
🧹 Proposed fix to remove unused imports
-import { useCallback, useMemo } from 'react'
-import { shallow, useStore } from '@tanstack/react-store'
+import { useMemo } from 'react'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { useCallback, useMemo } from 'react' | |
| import { shallow, useStore } from '@tanstack/react-store' | |
| import { useMemo } from 'react' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-table/src/useLegacyTable.ts` around lines 18 - 19, The file
imports unused symbols—remove the unused imports useCallback, shallow, and
useStore from the import lines in useLegacyTable.ts; specifically delete
useCallback from the 'react' import and remove shallow and useStore from the
'@tanstack/react-store' import so only actually used exports remain, then run a
quick lint/type-check to confirm no references remain.
2 participants
This pr removes the
useStorecall in useLegacyTable since it's already used by theuseTablehook with shallow selector. Table reference will change when that state is updated so we shouldn't need to call it twice in both hookshttps://github.com/TanStack/table/blob/alpha/packages/react-table/src/useTable.ts#L114-L122
I've also updated the Subscribe component to use the shallow selector to restore the
defaultbehavior we have in other adapters and previous tanstack store versions