New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change strictQueryParams default to false? #137

Closed
Keats opened this Issue May 22, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@Keats
Copy link
Contributor

Keats commented May 22, 2017

The vast majority of the time, you want to match routes on their path, whether they have query params or not. I would think the number of people wanting strict matching on query params to be very small.

This non-intuitive default means that by default / and /?ga_id=some-analytics-stuff (for example) will not match, leading to hard to diagnose issues.

Now I could be completly off-base and the majority of people actually wants strict query parameters but I can't even think of a case where I would want that.

If changing it is not possible, I suggest moving the "Strict query parameters" paragraph from http://router5.github.io/docs/router-options.html to the top of the page, doubling its font-size and putting a 10px red border to make sure it is visible.

@troch

This comment has been minimized.

Copy link
Member

troch commented May 23, 2017

Good suggestion for the docs. That's also fair to say strictQueryParans could be false by defaul`, but that will be a breaking change and need a major version bump.

I also need to document a trick you can use: the router has a setRootNodePath function you can use for "global" query params.

// List query params you expect to be used
router.setRootNodePath('?ga_id');
@Keats

This comment has been minimized.

Copy link
Contributor Author

Keats commented May 23, 2017

I agree that it would be a breaking change. I personally think it would be worth the new version but otherwise it can be scheduled in the next version when there are more breaking changes

@troch

This comment has been minimized.

Copy link
Member

troch commented May 23, 2017

I'm going to start working on a couple of new features which will create breaking changes on route-node (dependency) so it will be a good opportunity to introduce that change.

@troch troch referenced this issue Jun 14, 2017

Merged

V5: monorepo #147

17 of 19 tasks complete
@troch

This comment has been minimized.

Copy link
Member

troch commented Jun 25, 2017

Default changed with v5

@troch troch closed this Jun 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment