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
Update dependency array on setPage #2942
base: canary
Are you sure you want to change the base?
Conversation
This fixes an issue where search parameters were not updated correctly when changing pages in the pagination component. The root cause was the missing onChange dependency in the setPage callback function, which prevented the correct state from being passed on page changes. Changes: Add the onChange function to the dependency array of the setPage callback to ensure it always reflects the latest state. Impact: This fix ensures that the pagination component accurately reflects the current state when triggering onChange.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@jesuzon is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update in the Changes
Recent Review DetailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Fix to this open bug issue: #2417 |
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.
why not onChangeActivePage?
I supposed they are equivalent in this context, one calls the other anyway - I have tested and that works as well. |
@jesuzon |
Changing the active page the intended documented way (using setPage rather than changing the internal state directly using setActivePage which you technically can’t do directly I think) triggers an onChange by calling onChangeActivePage:
Do you want me to make a different commit adding onChangeActivePage instead of onChange? Either way, it should fix the issue I tagged above |
Changes:
Add the onChange function to the dependency array of the setPage callback to ensure it always reflects the latest state.
Impact:
This fix ensures that the pagination component accurately reflects the current state when triggering onChange.
Closes #
📝 Description
This fixes an issue where search parameters were not updated correctly when changing pages in the pagination component. The root cause was the missing onChange dependency in the setPage callback function, which prevented the correct state from being passed on page changes.
⛳️ Current behavior (updates)
Currently, state is memoized during onChange after page changes, meaning that writing onChange handlers do not always contain current state. This was causing issues with hooks such as useSearchParams, where the search params would not be fully up-to-date
🚀 New behavior
Adding onChange to the callback dependencies ensures state is refreshed.
💣 Is this a breaking change (Yes/No):
Should not break anything.
📝 Additional Information
Summary by CodeRabbit