baruchiro / use-route-as-state

Use React Router route and query string as component state
https://baruchiro.github.io/use-route-as-state/
MIT License
42 stars 7 forks source link

Add support for repeating query params (arrays) #95

Closed dietergeerts closed 3 years ago

dietergeerts commented 3 years ago

Hi,

I was looking to be able to work with query params easily, and also be able to update only certain params. I first wrote something small myself, but wanted to see if there would be something better. This package looks good, but one feature we do require for our functionality is the usage of repeating query params, which resolves to arrays. I didn't find this in the code, and I think this feature is missing.

It's a pity that react-router itself doesn't have such API's, because updating some params is something done a lot while using a web application that use the route as main state to start off.

baruchiro commented 3 years ago

Hi, thanks for your message!

Yes, this is known limitation, I started this library with minimal features.

I think adding array support should not be so complicated, because this problem already solved in other libraries. We just need to enhance the QueryString builder and update the types.

unfortunately, currently I'm busy on publishing another open source library, and I'm not available for new features (but I'm here for bugs, of course, this is my responsibility).

I will be happy to help you to implement this feature.

cxlucas commented 3 years ago

Hello, I'm currently working on this feature.

My aim is to add support for Array Query Params using the following "pattern":

Example:

It's almost done, but I can't set a completion date yet.

baruchiro commented 3 years ago

@cxlucas Thanks!

When you're ready, please explain why you decided about this convention, and please add links to show your convention is commonly used. I didn't research it, but I see there are some conventions, so I don't want to invent a new one.

Also, consider using an existing QueryString parser, if there is any.

Update: After a 2-minutes-research, I found the NodeJS querystring module that supports parsing arrays in query-string. But its legacy, and instead, the suggestion is to use URLSearchParams. Please take a look and see if it support arrays, and if we can use it.

cxlucas commented 3 years ago

There are many ways to represent a query param as an array. After reading about it on the web, there's no convention of which methods are better, but I think that the most used are:

"?val=faa&val=boo" and "?val[]=faa&val[]=boo"

However, I was reading some PHP forums, and the second one is the right way to represent query params arrays in PHP. As well, both approaches work in Node (as I tested).

Finally, to easily maintain the library's behavior (undefined for variables without defaultValue and [] for empty array variables) I decided to use the second approach, but if you want to move to the first one, it can be easily changed.

dietergeerts commented 3 years ago

Might I suggest to use the "standard" as is done in the official web API's?

See https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

I know that there isn't an actual standard, and there are indeed many ways to do this, but as this is a library for web, it would make more sense to follow what is done in official APIs around this.

This will then be ?val=faa&val=boo

cxlucas commented 3 years ago

Hi @dietergeerts, it's a good point.

I have read the documentation of NodeJS and you are right, they use the approach ?val=foo&val=boo. I tried to add the support for query params using this standard but I faced some issues, mainly when the query param has just one value. As the library receive just a string (URL Params), if we have an array key with a single value, we can't identify if the key is an array or a string, and we can't return for example: ['foo'] (array with a single element) instead of 'foo' (string) to the state variable.

Example 1: https://codepen.io/lrmendes/pen/qBjKGKO (expected behavior when the array key has 2+ values)

Example 2: https://codepen.io/lrmendes/pen/PojavrQ (here is the issue, how could we identify that a variable is an array or string using just the input string URL)

But of course, if we can find a good solution to handle it, I can move to the "web standard" without problems, I had chosen the second way - using var[] - because it's very simple to identify if the key is a string or array (if an array, it always includes [] in key-name)

Example 3: https://codepen.io/lrmendes/pen/XWgYLpX (using val[]=foo&val[]=bar)

dietergeerts commented 3 years ago

I understand that if you only know the query string, it's impossible to know in the use-case of a single value that it should be an array or not. But you can also say that about a query param that is representing a boolean, what will it be when there is no value? In every app that needs to work with query params, which is almost every app out there, you always have some sort of configuration where you decide what the query params should be. Almost all libraries that work with query params expect a configuration like this in order to transform the query params correctly, and to also avoid extra code in the app itself to convert the strings to the correct values like a boolean, or an enum value etc.... So in essence, when you don't use a configuration, you just can't know, but if you do, you do know, and your library will be better for it, as the user of this library will have way less work for working with query params. It's the reason we decided to use another library instead, so we can just configure the query params and use as-is, and be sure to receive correct values.

baruchiro commented 3 years ago

I understand that if you only know the query string, it's impossible to know in the use-case of a single value that it should be an array or not. But you can also say that about a query param that is representing a boolean, what will it be when there is no value? In every app that needs to work with query params, which is almost every app out there, you always have some sort of configuration where you decide what the query params should be. Almost all libraries that work with query params expect a configuration like this in order to transform the query params correctly, and to also avoid extra code in the app itself to convert the strings to the correct values like a boolean, or an enum value etc.... So in essence, when you don't use a configuration, you just can't know, but if you do, you do know, and your library will be better for it, as the user of this library will have way less work for working with query params. It's the reason we decided to use another library instead, so we can just configure the query params and use as-is, and be sure to receive correct values.

@dietergeerts I'm not sure what you're suggesting here. Can you explain?

If you can give us an example of how you can "configure the query params", I will appreciate that. Thanks!

baruchiro commented 3 years ago

@cxlucas @dietergeerts BTW, see the next code:

> params = new URLSearchParams({
    str: 'foo',
    arr: ['bar', 'eggs']
})
URLSearchParams { 'str' => 'foo', 'arr' => 'bar,eggs' }

The URLSearchParams is parsing the array with toString, and it turns to be a comma-separated string. I think with this approach, we can keep the array data, and the user will decide to parse it back to an array or not.

Another way is to use JSON.stringify for all complicated types.

cxlucas commented 3 years ago

The first implementation that I tried was the stringified approach, which converts the array data as comma-separated-values, like: ?foo=single&data=value1,value2,value3 as you said @baruchiro.

But when we need to (convert) split the string into an array of values using a comma, it will get some wrong values if any value in the array has a comma in its value, like "va,lue1" (a valid string value).

baruchiro commented 3 years ago

Yes, I was thinking about that.

OK, let's see what we have:

I think I prefer to use the first solution somehow, because it is conventional. But after thinking, in our use-case, the query string is controlled all by our library, so we don't need to guide the developer about the format of the query string, so maybe the second option is good for us.

Maybe, to find our best approach, we should ask ourselves what we will do when someone will ask us to support objects, with my.foo=x&my.bar=y? Which array solution will support extending for objects?

baruchiro commented 3 years ago

@cxlucas I see, as you said, there is no real convention. The NodeJS "convention" is to use foo=bar&foo=eggs, but it is very common in StackOverflow to use the PHP convention foo[]=bar&foo[]=eggs.

If you don't have any new thoughts after my preview comment, let's continue with the PHP convention.

dietergeerts commented 3 years ago

At the point where you will use this hook to get the query params, it will be known which ones are an array. With the official API to work with query params, you also have to explicitly use the getAll method instead of get if you know you want all of them because it's a repeated one. So if I adapt the example in the readme, it could look like:

// URL: /:param?query=
import * as React from 'react'

import { useQueryParams } from 'use-route-as-state'

const Example = () => {
  const [{ query }, setQueryParams] = useQueryParams({ query: "whateverTellsThisIsAnArray" })

  return (
    <div>{query.map((value) => <span>{value}</span>)}</div>
  )
}

So it will be more of a configuration that you pass through combined with the default values, or separate of them. Take a look at use-query-params package, which uses a config that really helps in avoiding boilerplate code of converting the strings to actual values, like when you have an enum etc....

baruchiro commented 3 years ago

OK, I see. Let's keep it with what @cxlucas suggested, I think it is simpler (although it gives us fewer options), and we will be different from the use-query-params package, and the users will be allowed to choose it or us.

But I will consider adding support for HOC and RenderProps :-)

I will look at this library more, I'm sure I can learn from it, or even understand my library is not relevant.