search

gvergnaud / nextjs-dynamic-routes

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

Query param precedence #15

Closed lydell closed 5 years ago

lydell commented 5 years ago

See the individual commit messages for more information.

gvergnaud commented 5 years ago

Thanks for contributing! I merged your 3 other PRs but this one is conflicting. You can probably get rid of the diff in package-lock, package.json and yarn.lock, since it was already added in one of your other PR.

I'll publish a new version in the mean time and a new minor one after we merge this PR. Cheers!

lydell commented 5 years ago

Rebased.

One thing I came to think of is that since #12 was merged, ?id=1&id=2 sets query.id to ["1", "2"]. For the pattern /test/:id, should /test/0?id=1&id=2 then become ["0", "1", "2"]? Right now this PR gives query.id === "0". That’s better than status quo, but maybe we want to support query params with the same name? Not sure. What do you think?

lydell commented 5 years ago

@gvergnaud What do you think about merging this PR as-is and opening an issue about whether we should change to the array idea?

lydell commented 5 years ago

@gvergnaud Friendly ping :)