-
-
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
Query params with values null or undefined get serialized into strings #4570
Comments
thanks @denisnazarov for tracking this one down. ping señor @machty |
@machty we could use your help on this |
So after struggling with this last night I'm not sure that this is totally undesirable. Say you have the following controller.
If you set This is similar to the problem in the previous implementation that it was ambiguous between falsy values and the actual value of false. Now it seems its ambiguous between a |
Here's another example using boolean query params: http://emberjs.jsbin.com/hamev/2/edit If the default value is set to |
@HeroicEric so what's the compelling reason for setting it to |
That might have come out wrong; I'm just curious what the use cases are for all these serialization corner cases. |
@machty I tried to show an example use case in the jsbin. For example, I'm displaying a list of users and I want to be able to filter them so that the list contains either All Users, Admin Users, or Non-Admin Users. When I want to see All Users I would basically just be removing the filter. Are situations like this not what query params are meant for? |
Ideally the URLs would be something like this: /users shows all the users |
@HeroicEric @machty I think the idea was that if the defaultValue on the controller wasn't defined ( I think if you want a property to be set to If we had this then you could set the type as |
I have this JSBin example, and I wonder if what I'm seeing is related to this issue: http://jsbin.com/dipajezi/1/edit Basically, I have this Query: When I click on the NextPage button, I transition again but this time I change the Query: Finally, when I click on the Query |
@raytiley Just wrote a note on raytiley@26a3f85#diff-0631ecfe6138cf2c2eb2d94369c3e846R1640. |
If I set a qp to Otherwise if you want to pass QPs that have values only, you have to do |
I think this is out of date but will reopen if someone can demonstrate the issue in a JSBin that uses the following ember.js: http://s3.amazonaws.com/machty/to_s3_uploads/ember-9fbe6c2a-c124-5c2e-0414-f5ed36c2a1a2.js |
In case anyone find this via Google. The issue is basically solved, but an edge case remain where if the query param isn't set on the controller it will serialize
|
Regarding @HeroicEric's use case, the controller won't know how to intelligently serialize based on what the value will be in its lifetime. This seems to be working in the most recent version (2.6):
Working ember twiddle: https://ember-twiddle.com/3afa1091106a91ce2c1734ae2998bc3f?openFiles=controllers.application.js%2C&route=%2F%3Fredevelopment%3Dtrue |
Since when do QPs allow setting the type? Or are you just proposing a new API? |
It seems to be undocumented. Looking here, it seems it can be overridden. |
Oh, that's nice! |
Thanks, @allthesignals, it's so helpful for your solution. |
…ined into url # Conflicts: # packages/ember/tests/routing/query_params_test.js # packages_es6/ember-routing/lib/system/route.js
…ined into url # Conflicts: # packages/ember/tests/routing/query_params_test.js # packages_es6/ember-routing/lib/system/route.js
…to url # Conflicts: # packages/ember/tests/routing/query_params_test.js # packages_es6/ember-routing/lib/system/route.js (cherry picked from commit c157b99)
…to url # Conflicts: # packages/ember/tests/routing/query_params_test.js # packages_es6/ember-routing/lib/system/route.js (cherry picked from commit c157b99)
How about this solution?
|
Good list! I'd vote for this: and possibly also:
|
ember.js/packages_es6/ember-routing/lib/system/route.js
Lines 1633 to 1641 in 4444525
The text was updated successfully, but these errors were encountered: