[query-params-new] don't serialize null or undefined to url #4571
Conversation
raytiley
changed the title
| @@ -1636,7 +1636,9 @@ if (Ember.FEATURES.isEnabled("query-params-new")) { | |||
| // urlKey isn't used here, but anyone overriding | |||
| // can use it to provide serialization specific | |||
| // to a certain query param. | |||
| if (defaultValueType === 'array') { | |||
| if (value === null || value === undefined) { | |||
There was a problem hiding this comment.
Doesn't have to be. The '' + value will coerce the boolean value into a string which is what we want. We want boolean values to be explicit in the url ?foo=false or ?foo=true
|
I was working through the same issue earlier today and came to a similar conclusion. This is also relevant to #4493. |
|
@machty - Thoughts here? |
|
We've ran into the same issue and agreed with the changeset - impartial to whether undefined should get swallowed into null. I think it would work as Our example is doing pagination and search - using refreshModel. I wouldn't even mind if the route's model hook received a real null value. We can filter those out before we hit the server as opposed to being out of luck with the string "undefined" or "null" - nothing is (or should) be stopping a user from typing that as a search query. Ironic as I typed "null" in Github to find this PR :P |
|
This fix works for me. |
|
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 |
nullandundefinedare no longer serialized into the url. This behavior might not be right though as its now ambiguous as to weather you want the default value for a query param, or you want the value to benullThere is more of a discussion in #4570.