-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($httpParamSerializerJQLike): Follow jQuery parameter serialization l... #11635
Conversation
@@ -2014,6 +2014,12 @@ describe('$http param serializers', function() { | |||
expect(jqrSer({a: 'b', foo: {'bar': 'barv', 'baz': 'bazv'}})).toEqual('a=b&foo%5Bbar%5D=barv&foo%5Bbaz%5D=bazv'); | |||
//a=b&foo[bar]=barv&foo[baz]=bazv | |||
}); | |||
|
|||
it('should serialize nested objects by repeating param name with [kay] suffix', function() { |
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.
kay --> key
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.
Done
57a82d8
to
089e246
Compare
if (!params) return ''; | ||
var parts = []; | ||
serialize(params, '', true); | ||
return parts.length > 0 ? parts.join('&') : ''; |
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.
Minor, but I think this can be just return parts.join('&');
In paramSerializer()
too.
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.
Done
089e246
to
551b63b
Compare
The travis failure looks like a flake |
I think we need to make a special case for date parameters: jQueryLikeParamSerializer({ dateFrom: new Date() }) === "" |
forEach(toSerialize, function(value) { | ||
serialize(value, prefix + '[]'); | ||
}); | ||
} else if (isObject(toSerialize)) { |
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.
Maybe } else if (isObject(toSerialize) && !isDate(toSerialize)) {
?
…n logic Follow jQuery parameter serialization logic for nested objects. Closes angular#11551
551b63b
to
e77080f
Compare
@gabrielmaldi you are right, fixed this case |
I thought this would make it into |
@@ -11,6 +11,8 @@ var JSON_PROTECTION_PREFIX = /^\)\]\}',?\n/; | |||
|
|||
function paramSerializerFactory(jQueryMode) { | |||
|
|||
return jQueryMode ? jQueryLikeParamSerializer : paramSerializer; |
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.
If we are going to do this then we might as wel ditch the paramSerializerFactory
altogether and just reference these two serializers directly in their respective providers.
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.
They only share serializeValue
, maybe move a little more logic to this function but keep it as is?
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.
It's already refactored in 2420a0a
One comment but otherwise LGTM |
...ogic
Follow jQuery parameter serialization logic for nested objects.
Closes #11551