infinum / react-nuts-and-bolts

A collection of commonly used tools and components that we use everyday at Infinum.
MIT License
1 stars 0 forks source link

Create useQueryParams #21

Open fvoska opened 11 months ago

fvoska commented 11 months ago

In JustTodoIt, we require table state like pagination, sorting and filtering to be kept in the URL. This is a great practice for writing a custom hook that people often end up calling useQueryParams. I would still like people to keep implementing this hook as part of the onboarding project, but for real project work I think it would be good to have this hook available in our library (we have a similar thing in ngx-nuts-and-bolts).

Oddly enough, there is no such custom hook developed in the community (at least not from my quick search); nothing on useHooks. There is use-query-params npm package, but it assumes use of react-router. I guess it makes sense that this sort of a hook would have to assume how routing is done and whether there is SSR or now. Since we are mostly using NextJS, I think it would make sense for us to create useQueryParams hook that assumes use of NextJS routing and have it tested well.

Erbenos commented 11 months ago

There is https://nextjs.org/docs/app/api-reference/functions/use-search-params

On Fri, Nov 24, 2023, 11:19 Filip Voska @.***> wrote:

In JustTodoIt https://github.com/infinum/JS-justTodoIt, we require table state like pagination, sorting and filtering to be kept in the URL. This is a great practice for writing a custom hook that people often end up calling useQueryParams. I would still like people to keep implementing this hook as part of the onboarding project, but for real project work I think it would be good to have this hook available in our library (we have a similar thing in ngx-nuts-and-bolts https://infinum.github.io/ngx-nuts-and-bolts/docs/table-state).

Oddly enough, there is no such custom hook developed in the community (at least not from my quick search); nothing on useHooks https://usehooks.com/. There is use-query-params https://www.npmjs.com/package/use-query-params npm package, but it assumes use of react-router. I guess it makes sense that this sort of a hook would have to assume how routing is done and whether there is SSR or now. Since we are mostly using NextJS, I think it would make sense for us to create useQueryParams hook that assumes use of NextJS routing.

— Reply to this email directly, view it on GitHub https://github.com/infinum/react-nuts-and-bolts/issues/21, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABN5UV6KLGE5B6LPQ5HIAJ3YGBYDRAVCNFSM6AAAAAA7Y44JHCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGAYDSNBWHEZTMNQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

fvoska commented 11 months ago

@arielbk can you maybe provide some insight, based on what you had to do during your onboarding?

fvoska commented 11 months ago

@Erbenos I am not sure if what you linked works with the new pages router?

Erbenos commented 11 months ago

I see no indication that it would not work, however the behavior is a bit different compared to just reading the queryParams from page prop

arielbk commented 11 months ago

I think the useSearchParams hook from Next.js is only available with the new app router (the onboarding project currently uses the old pages router) and it just returns a readonly URLSearchParams object. A custom hook makes dealing with nontrivial use cases (like multiple params) simpler.

What I found helpful to have in the hook during onboarding was:

Erbenos commented 11 months ago

I mean it has #get/#getAll so I don't see the issue with handling multiple params.

As to the generic-ness/typesafety, think any such solution would have to cast / guess the values there as well given that the address bar/URL itself is source of truth - thats untyped and best you can do is read string records.

There is no point in having "setParam" function - you can either navigate via link or by the router methods. Don't think there is much value in building abstraction on top of it.

arielbk commented 11 months ago

I mean it has #get/#getAll so I don't see the issue with handling multiple params.

It's not a problem reading params, but it can become cumbersome when you want to route to new ones with the existing ones intact.

It's easier to work with an object of parsed params and pass that around than a URLParams object — especially a readonly one. Next's Link component (and the router) accepts query params as an object, which makes something like this really easy:

<Link
  href={{
    query: {
      ...params,
      page: page + 1,
    },
  }}
>

As to the generic-ness/typesafety, think any such solution would have to cast / guess the values there as well given that the address bar/URL itself is source of truth - thats untyped and best you can do is read string records.

Definitely — the URL itself should be the source of truth. The types should not give us false confidence about what we get, because we don't have control over that part.

But types can constrain what we can do with the query params in a particular part of our application. We can decide which URL params we care about, not assuming they're there, but still getting nice autocomplete.

There is no point in having "setParam" function - you can either navigate via link or by the router methods. Don't think there is much value in building abstraction on top of it.

True, this could be an unnecessary abstraction. On the onboarding project this was for a search input that updates the URL params, removes it if it was empty, and routes the user. This could be better to have on a case by case basis, although it is nice to have that logic encapsulated and avoid repetition.

arielbk commented 11 months ago

Maybe having a custom hook with abstractions does push us away from standards. The hook from the new app router makes it easier with URLSearchParams since we don't have to construct it ourselves now.

There's a section in the new Next Learn tutorial that just uses Router and Links: https://nextjs.org/learn/dashboard-app/adding-search-and-pagination

Erbenos commented 11 months ago

I think you may be miscategorizing the problem a bit. Problem isn't working with query parameters URLSearchParams and generic navigation or Link already does that well enough. It sounds to me like you desire a separate abstraction, one, that would interpret/typecast whatever is in query parameters so that the data would be more semantic, not just mere strings, so that you can reason about it better. Such an abstraction wouldn't be called "useQueryParams" though.

From top of my head I could imagine custom hook factory createQueryParamsReader which would accept Zod validator and return a hook which wraps around useQueryParams, reads the params, lets it go through the validator which optionally could use https://github.com/colinhacks/zod#catch to provide default values and return the parsed data - which is now typed and as such should be easier and safer for you to work with.

Erbenos commented 11 months ago

It could perhaps also be called createQueryParamsReaderWriter return a tuple of reader and writer, and you could use the writer function to build args for your navigation abstraction from the values read by reader. Naming could be improved.