Skip to content
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

Treat all parameters not matching a named route segment as query params #451

Merged
merged 3 commits into from
Aug 25, 2021

Conversation

bakerkretzmar
Copy link
Collaborator

This PR assumes that any object keys/values passed into route() as parameters that do not match any named route segments are treated as query parameters.

Before

// route 'home' at `/home`

route('home', ['foo']); // `/home?foo=`
route('home', { foo: 'bar' }); // `/home?foo=bar`
route('home', { _query: { foo: { bar: 'baz' } } }); // `/home?foo[bar]=baz`

route('home', { foo: { bar: 'baz' } }); // error

That last example causes an error because Ziggy sees that the value of the foo parameter passed in is an object, so it expects either an id key in that object or a key matching the binding for the foo route parameter—but there is no foo route parameter.

After

// route 'home' at `/home`

route('home', ['foo']); // `/home?foo=`
route('home', { foo: 'bar' }); // `/home?foo=bar`
route('home', { _query: { foo: { bar: 'baz' } } }); // `/home?foo[bar]=baz`

route('home', { foo: { bar: 'baz' } }); // `/home?foo[bar]=baz`

Closes #450.

@caseydwyer
Copy link

Any chance this might be merged in the next couple of days? I know you all are busy people, so no pressure—just trying to decide if I need to roll back to an earlier version for an event this weekend. Thanks for your work, Ziggy is a great little tool!

@bakerkretzmar
Copy link
Collaborator Author

@caseydwyer will do 👍🏻 I think I actually meant to tag you in this to take a look and see if it'll solve your issue, but completely forgot. Looking at it again thought I'm pretty sure it's good to go.

@caseydwyer
Copy link

Thanks @bakerkretzmar! Probably a dumb question...but should the changes to src/js/Router.js be available within /dist/index.js or one of the other dist files? ie, is an NPM build necessary for this PR? Again, pardon my ignorance. Ha

@bakerkretzmar
Copy link
Collaborator Author

Yep, I'll run another build after I merge this and it gets run when we publish to NPM too.

@bakerkretzmar bakerkretzmar merged commit 25712ad into main Aug 25, 2021
@bakerkretzmar bakerkretzmar deleted the jbk/fix-450-query-params-object branch August 25, 2021 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing route model binding key on query filters
2 participants