fix(pagination): prevent onPaginationChange from firing when state is unchanged#6217
fix(pagination): prevent onPaginationChange from firing when state is unchanged#6217solssak wants to merge 1 commit intoTanStack:alphafrom
Conversation
… unchanged When data reference changes without actual value changes (e.g., RSC re-renders on URL navigation), _autoResetPageIndex triggers onPaginationChange even though pagination state is identical. Add shallow equality check in setPagination to skip the callback when the resolved new state matches the current state. Closes TanStack#6158
📝 WalkthroughWalkthroughThe changes optimize pagination state updates by preventing unnecessary callbacks when the pagination state hasn't changed. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
View your CI Pipeline Execution ↗ for commit 41f1092
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud has identified a possible root cause for your failed CI:
We determined these failures are pre-existing issues unrelated to the pagination fix — the sherif version conflict in @tanstack/react-devtools and the ESLint import ordering error in solid-table-devtools both exist independently of this PR's changes. Neither failing project is within our touched scope (@tanstack/table-core), and no code modified by this PR is referenced in either error.
No code changes were suggested for this issue.
You can trigger a rerun by pushing an empty commit:
git commit --allow-empty -m "chore: trigger rerun"
git push
🎓 Learn more about Self-Healing CI on nx.dev
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/table-core/tests/unit/features/row-pagination/rowPaginationFeature.utils.test.ts (1)
16-16: Fix array type annotation per ESLint rule.ESLint flags the use of
any[]syntax.🔧 Proposed fix
-function createPaginationTable(onPaginationChange?: (...args: any[]) => void) { +function createPaginationTable(onPaginationChange?: (...args: Array<any>) => void) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/table-core/tests/unit/features/row-pagination/rowPaginationFeature.utils.test.ts` at line 16, The function signature createPaginationTable currently types the callback parameter as onPaginationChange?: (...args: any[]) => void which violates the ESLint rule against any[]; change the parameter to use a safer non-any type such as onPaginationChange?: (...args: unknown[]) => void (or Array<unknown>) so the callback accepts unknown-typed varargs instead of any, and update any related usages/tests to handle unknown if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/table-core/tests/unit/features/row-pagination/rowPaginationFeature.utils.test.ts`:
- Line 16: The function signature createPaginationTable currently types the
callback parameter as onPaginationChange?: (...args: any[]) => void which
violates the ESLint rule against any[]; change the parameter to use a safer
non-any type such as onPaginationChange?: (...args: unknown[]) => void (or
Array<unknown>) so the callback accepts unknown-typed varargs instead of any,
and update any related usages/tests to handle unknown if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb6ec849-e349-4d4e-b7f6-1668c5dcdcb8
📒 Files selected for processing (3)
packages/table-core/src/features/row-pagination/rowPaginationFeature.utils.tspackages/table-core/src/utils.tspackages/table-core/tests/unit/features/row-pagination/rowPaginationFeature.utils.test.ts
|
I ended up applying a similar fix in alpha.37 before seeing this. Check if it's solved for you. We could explore a more broad set state prevention refactor when current state shallow equals the next state update if we want. |
|
@KevinVandy, thanks for the heads up! I just verified — yeah, you're right, it's already fixed in alpha.37. My local setup was apparently out of date, sorry for the noise on that. Really appreciate you taking the time to look into this and share the broader refactor idea too. A more general "skip update when next state shallow-equals current" pass sounds like a nice direction — happy to take a stab at that as a follow-up PR if it'd be useful. Thanks again for the consistently kind and thoughtful reviews 🙏 |
2 participants
Summary
Fixes #6158
Problem:
onPaginationChangefires on every URL navigation in React Server Components, even when pagination state hasn't changed. When a parent RSC re-renders (e.g., due torouter.pushorLinknavigation), thedataprop gets a new reference. This triggers_autoResetPageIndex→resetPageIndex→setPagination→onPaginationChange, even thoughpageIndexandpageSizeare identical. When pagination is persisted to URL params, this creates an infinite loop.Fix: Add a shallow equality check in
setPaginationbefore invokingonPaginationChange. If the resolved new pagination state is identical to the current state, the callback is skipped entirely.Changes
packages/table-core/src/utils.ts— AddedshallowEqualutility functionpackages/table-core/src/features/row-pagination/rowPaginationFeature.utils.ts—table_setPaginationnow checks if state actually changed before callingonPaginationChangepackages/table-core/tests/unit/features/row-pagination/rowPaginationFeature.utils.test.ts— Added 9 tests covering no-op updates fortable_setPagination,table_setPageIndex,table_resetPageIndex, andtable_autoResetPageIndexVerification
Summary by CodeRabbit
Refactor
Tests