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 |
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
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
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