Conversation
|
@cenobitedk is attempting to deploy a commit to the 47ng Team on Vercel. A member of the Team first needs to authorize it. |
|
The only challenge I've found is that onPaginationChange: (p) => {
setPagination(p);
},Here's the full example: const [pagination, setPagination] = useQueryStates(
{
pageIndex: parseAsPageIndex.withDefault(0),
pageSize: parseAsInteger.withDefault(15),
},
{
urlKeys: {
pageIndex: 'page',
pageSize: 'size',
},
},
);
const table = useReactTable({
columns,
data,
getCoreRowModel: getCoreRowModel(),
getPaginationRowModel: getPaginationRowModel(),
onPaginationChange: (p) => {
setPagination(p);
},
state: {
//...
pagination,
},
}); |
parseAsPageIndex parserparseAsPageIndex parser
franky47
left a comment
There was a problem hiding this comment.
LGTM, thanks!
nitpick: What do you think about naming it parseAsIndex? The zero-based approach might work for other things than pagination, though I agree it's likely to be the main use-case. parseAsIndex might be less clear of what it does though (but that's what documentation is for).
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Fine with me! :) |
|
@franky47 The contribution doc is a little vague on what to add before a feature is ready. I have added the parser and test, and I have the documentation ready as well. But what about e2e test, should I update those as well? |
|
@franky47 Hi François. I've updated e2e tests for Next.js. As far as I can tell, everything should be ready to go 👍 |
|
Thank you! The e2e part wasn't really necessary as parsers can easily be unit-tested, I need to clean the e2e-next project and remove duplicates. The e2e envs are mostly to test the adapters and framework behaviours, I'll update the contribution docs to reflect that. I see the Prettier linter step is failing on the parsers file, would you mind taking a look? Thanks! |
Sure! I tried to address it in the last commit. However the output from the check was not clear as to what was wrong and when I run
But I suspect it has to do with semicolon and other small things. |
|
Ok I might need to improve that part a bit more too then, so all tests in CI can be run locally. If you had Prettier format the files locally and it doesn't pass CI linting, maybe we have a config issue where user-set global Prettier config takes precedence and conflicts with defaults. |
franky47
left a comment
There was a problem hiding this comment.
LGTM, thanks! I added a negative index check to the parser, to avoid out-of-bounds access.
| const [value, setValue] = useQueryState('page', parseAsIndex) | ||
| return ( | ||
| <DemoContainer demoKey="page"> | ||
| <input |
There was a problem hiding this comment.
suggestion: Add a minimum of zero for the input, as the parser now checks for positive-only values.
There was a problem hiding this comment.
A minimum of zero makes sense for page index, but as a general index, a value -1 or below could be utilised to iterate backwards on an array. Something like .withOptions({ min: 0}) would make sense. Was that what you had in mind?
There was a problem hiding this comment.
Yeah I thought about going negative to do python-like backwards indexing (using .at), but it would mean:
?page=0maps to the last element (index -1)?page=-1maps to the second to last element (index -2)
This seems even more confusing.
I'll remove the positive check and we can enforce runtime values with validation once #446 (or a variant) lands.
Note: TanStack Table doesn't seem to do a bounds check on negative indices: https://table.sadmn.com/?flags=advancedTable&page=-1
|
Awesome! Thanks for merging 🙏🏻 happy to contribute |
|
🎉 This PR is included in version 2.4.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |


Hi! Thanks for a great package!
I have suggestion for a (probably very) common use-case where you add the current page (eg. of a table) to the url. At least if you are doing pagination client-side. Typically you have the page as 0-index yet you want the url to be human readable starting from 1. Here
parseAsPageIndexcomes in super handy, taking care of the +1 offset automatically.For example, let's say you use
@tanstack/react-tableand want to have the pagination as part of the url:I was also thinking about
parseAsIntegerWithOffsetwith any offset number, but I could only think of very few situations where it would be beneficial.