search

gvergnaud / nextjs-dynamic-routes

[Deprecated] Super simple way to create dynamic routes with Next.js
MIT License
140 stars 7 forks source link

Support query parameters with the same name as url params? #17

Closed lydell closed 5 years ago

lydell commented 5 years ago

Context: https://github.com/gvergnaud/nextjs-dynamic-routes/pull/15#issuecomment-455150011

Currently, you can’t have query parameters with the same name as a URL param. The URL param takes precedence and the query param is discarded. For example, with the pattern /test/:id, the URL /test/1?id=a always gives you { id: "1" } and you can’t see that the user passed id=a.

In Next.js, if you pass the same query parameter multiple times, such as ?x=a&x=b you get a query object like this: { x: ["a", "b"] }

In other words, you get an array of all the values for the param.

Now to the question: Should we do something similar to support query params with the same name as URL params? For example, /test/1?id=a could result in { id: ["1", "a"] }. Or maybe the URL param should go at the end, since one might be used to take the last query param value when the user has given multiple but only one is supported? { id: ["a", "1"] }.

So – why should we do this? It makes the library a little bit more flexible and "complete", I guess. But maybe it would just make things easier to screw up. It’s easy to forget that you might get an array and forget to handle that, or you might take the wrong value from the array. And I guess it’s easier to just rename the URL param or query param to not conflict, anyway. Probably makes your code easier to understand, too.

What do you think?

gvergnaud commented 5 years ago

I personally don't really like the fact that a parameter can have two different types (string and array), even if it's pretty common for mvc web framework to handle redondant query params this way...

Not converting redondant query params to arrays doesn't block you from implementing your own logic, like separating your values by commas (e.g. id=a,b,c) which is clearer in my opinion. Do you see any usecases you couldn't handle without this feature? If not, I think we could let the lib as it is.

Anyway, thanks for bringing this up!

lydell commented 5 years ago

Yes, I agree – after discussion it feels like things are good the way they are!