Repository navigation
fix(core): strip currentPage in useTable getCurrentQueryParams - #7630
fncreator22 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: f33ca9f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const getCurrentQueryParams = (): object => { | ||
| // We get QueryString parameters that are uncontrolled by refine. | ||
| const { sorters, filters, pageSize, current, ...rest } = | ||
| const { sorters, filters, pageSize, currentPage, ...rest } = |
There was a problem hiding this comment.
Nice fix. I think this should keep stripping the legacy current key as well, rather than replacing it with currentPage because the React Router parser preserves arbitrary query keys in parsedParams.params, so opening an old link such as ?current=2 and then using createLinkForSyncWithLocation for page 3 will produce a link containing both current=2 and currentPage=3. The Ant Design pagination path calls this helper directly so it's quite important.
Could you, please, destructure both current and currentPage, and add a regression test with a legacy current query parameter just in case? Thanks!
There was a problem hiding this comment.
Great catch, @sviat-barbutsa! Preserving legacy ?current= from old bookmarks while introducing ?currentPage= would indeed pollute the query string.
I've updated getCurrentQueryParams to strip both current and currentPage, and added a dedicated regression test verifying that legacy current is cleanly stripped alongside currentPage. Thank you for the thoughtful review!
There was a problem hiding this comment.
@fncreator22 thank you for your reaction and update. Glad to help!
…CurrentQueryParams
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
In
useTable(packages/core/src/hooks/useTable/index.ts),getCurrentQueryParamsstill destructures the legacy v4currentproperty instead ofcurrentPage:Because
getCurrentQueryParams()is spread after{ currentPage, pageSize }insidecreateLinkForSyncWithLocation:currentPageis present inparsedParams.params(e.g.?currentPage=2),rest.currentPageoverwrites the targetcurrentPagepassed tocreateLinkForSyncWithLocation.currentPagequery param is in the URL, router bindings (@refinedev/react-router,@refinedev/remix-router,@refinedev/nextjs-router) still definecurrentPage: undefinedonparsedParams.params, which overwrites the targetcurrentPagewithundefined.pagination.modeis"off",currentPagefromparsedParams.paramsleaks into the link query via...getCurrentQueryParams().What is the new behavior?
getCurrentQueryParamsnow stripscurrentPageinstead ofcurrent(matchinguseMetaand the v5 pagination changes in #6948 and #7015), socreateLinkForSyncWithLocationuses the providedcurrentPagewhile preserving uncontrolled query parameters.Notes for reviewers
packages/core/src/hooks/useTable/index.spec.tscoveringcreateLinkForSyncWithLocationboth with pagination enabled and withpagination: { mode: "off" }.