-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(Angular.js): not serializing null values #6964
Conversation
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
(arrayValue === true ? '' : '=' + encodeUriQuery(arrayValue, true))); | ||
}); | ||
} else { | ||
parts.push(encodeUriQuery(key, true) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this was indented like this already, but it should be fixed if this patch lands.
👍 I'm having this issue right now. I really don't understand why angular would "jsonize" something that he actually thinks is blank. I've done a fiddle to show how is that null will show as blank on templates, but will appear as a "null" string as a result of the angular.toJson :( http://jsfiddle.net/genuinefafa/CQ3dL/1/ Am I missing something here? |
02dc2aa
to
fd2d6c0
Compare
cad9560
to
f294244
Compare
e8dc429
to
e83fab9
Compare
4dd5a20
to
998c61c
Compare
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
@pkozlowski-opensource I just pushed a test that is failing because toKeyValue is serializing 'undefined'. Just wanted to make sure it was actually happening. Ill fix and push. Do you think there are better ways to solve this issue or maybe if this is a issue at all? or if we should serialize the same way jQuery is doing it? which is nullKey=&foo=bar&undefinedKey= like so http://jsfiddle.net/joshkurz/6r6ojcey/? |
Signed-off-by: Josh Kurz <jkurz25@gmail.com>
CLAs look good, thanks! |
Hi @joshkurz - thanks for working on this issue. This PR is actually failing on both Travis: https://travis-ci.org/angular/angular.js/jobs/42586543 and on Jenkins: https://ci.angularjs.org/view/AngularJS/job/angular.js-angular-master/3635/. @joshkurz - did you mean to merge this PR? It was not verified by any core team member. I am going to revert the commit and reopen the PR. Can you take a look at the failing tests; make sure that they are fixed then get a LGTM from a core member before merging to master? Thanks! |
Oh, I can't reopen the PR because it was "merged" rather than closed. @joshkurz - can you create a new PR please with fixe for the failing test? |
#10186's 2nd commit (wesleycho@c266080) fixes the issue (I have suggested to @wesleycho to submit as a separate PR; @joshkurz's submitting a new PR would be just as good, of course). Just mentioning it, so we don't double effort. |
toKeyValue should not serialize a value if that value is set as null.