-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BUGFIX beta] Don't throw an error, when not all query params are passed to routerService.transitionTo #15613
Conversation
…ined into url # Conflicts: # packages/ember/tests/routing/query_params_test.js # packages_es6/ember-routing/lib/system/route.js
Restarted sauce labs |
Failed again, any idea what's wrong? |
Thank you @CvX! I ran into this while upgrading as well. |
Alright, dunno who did what, but sauce labs tests are now passing! 🎉 |
Null/undefined fix isn't needed for the router service exception fix, but it just feels wrong to write a test that expects the url to have "queryParams=null". 😅 (see snippet in the original comment of this PR) |
Is there anything blocking at least the fix for the first issue from being merged? It is keeping us from upgrading to 2.15 |
ping @rwjblue ^^ any further debate or discussion required on this PR boss? |
We're waiting for this to be released, any chance this will show up in 2.16? |
Any updates here? What are the specifics for not allowing the type coercion? This issue prevents our app from using the router service in several places. It just doesn't feel right that doing the same transition from a template |
Any word from ember core team here? I know ember v2.16.0 shipped without this fix ... |
Alright, to make it easier to move this forward I'm splitting the PR into two:
|
Apologies for not getting this in for 2.16, I'll get the changes into the next 2.17 beta release and once some folks can confirm all is working well for us I'll pull down into 2.16 for the LTS. |
@CvX - Thank you very much for working so hard on this! |
This PR attempts to fix two issues:
Using
transitionTo
via router service fail with an error, unless all query parameters of the target route are passed into the method. Here's the failing test: https://github.com/CvX/ember.js/blob/1c8c6ecad42228e94deffb7672fc6de905fca007/packages/ember/tests/routing/router_service_test/transitionTo_test.js#L259-L277.And for googling folks, the error message was
Error: Assertion Failed: You passed the `false` query parameter during a transition into [route-name-here]}, please update to [query-param-here]
.null
, i.e.cc: @rwjblue