Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

$http config.param expands array values properly. #1364

Closed
wants to merge 1 commit into from

Conversation

tdavis
Copy link
Contributor

@tdavis tdavis commented Sep 14, 2012

Today, calling e.g. $http(url, { params: { a: [1,2,3] } }) results in a query
string like "?a=%5B1%2C2%2C3%5D" which is undesirable. This commit enhances
buildURL to createa query string like "?a=1&a=2&a=3".

Fixes #1363.

Today, calling e.g. $http(url, { params: { a: [1,2,3] } }) results in a query
string like "?a=%5B1%2C2%2C3%5D" which is undesirable. This commit enhances
buildURL to createa query string like "?a=1&a=2&a=3".
@IgorMinar
Copy link
Contributor

Very nice pull request!

Can you please sign our CLA and I'll merge it in.

CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html

@ghost ghost assigned IgorMinar Oct 5, 2012
@tdavis
Copy link
Contributor Author

tdavis commented Oct 5, 2012

Thanks! I signed the CLA a couple days ago; am I waiting for some kind of
confirmation or are we set?

@bsparks
Copy link

bsparks commented Oct 9, 2012

hey can we make this a configurable hook? with jquery for example, the default is to do arrays like &foo[]=1&foo[]=2 and you pass a traditional: true flag in if you want &foo=1&foo=2 ... since I have some back end targets setup for the brackets it'd be nice if we could expose the whole buildUrl method to configuration.

@tdavis
Copy link
Contributor Author

tdavis commented Oct 15, 2012

My concern is that this dies on the vine while people discuss the relative
merits of supporting an arbitrary convention on top of an existing
standard, what configuration args may be named, etc.

As it stands, this patch does not prohibit you from passing { "foo[]":
[1,2,3] } as query args. Whereas without it, many REST services currently
require patched angular or manual URL construction. Perhaps this is best
suited to a follow-up enhancement discussion?

@bsparks
Copy link

bsparks commented Oct 16, 2012

OK, I'm fine with that. Can I also note though that ngResource also suffers from this as it doesn't seem to use $http to build its URLs. Perhaps that is best mentioned on that file...

@tdavis
Copy link
Contributor Author

tdavis commented Oct 16, 2012

Oh, that stinks. I imagine I'll work on a patch for that soon, then.

@IgorMinar
Copy link
Contributor

tweaked the commit message a bit and landed this as 79af2ba

thanks for the contribution. if you don't have our t-shirt yet please fill out this form: http://goo.gl/075Sj

@IgorMinar IgorMinar closed this Nov 24, 2012
@tdavis
Copy link
Contributor Author

tdavis commented Nov 26, 2012

Awesome, glad to see this go in! Looking forward to my tshirt ;)

@jmeiss
Copy link

jmeiss commented Apr 11, 2013

Nice fix guy!

I'm experiencing the same issue with $ressource is there the same bug?

@jmeiss
Copy link

jmeiss commented Apr 11, 2013

I've removed the 'use strict' and it works like a charm.

Sorry for the noise!

@lifeiscontent
Copy link

does this also fix the $resource object? seems this is still a problem: https://groups.google.com/forum/#!msg/angular/mAqaKXGm3Fg/fOazZvB8GBcJ

@tdavis
Copy link
Contributor Author

tdavis commented Jun 23, 2013

No. At least, not when it was merged.
On Jun 21, 2013 11:29 PM, "Aaron Reisman" [email protected] wrote:

does this also fix the $resource object?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1364#issuecomment-19849899
.

@lifeiscontent
Copy link

@tdavis does that mean its still broken or is it fixed in a later version?

@tdavis
Copy link
Contributor Author

tdavis commented Jun 25, 2013

Likely still broken. Unless $resource has been refactored to use the same
underlying function, which I cannot say. Though it seemed like a good idea
at the time ;-)
On Jun 25, 2013 2:34 AM, "Aaron Reisman" [email protected] wrote:

@tdavis https://github.com/tdavis does that mean its still broken or is
it fixed in a later version?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1364#issuecomment-19955854
.

@lifeiscontent
Copy link

@tdavis would you suggest using the bleeding edge of angular.js? I feel like its hard to use stable with all the build processes of angular.js and making sure the bug hasn't been patched in newer versions.

Also, if you end up finding a bug in unstable, just submit a patch right? :)

@cm325
Copy link

cm325 commented Sep 30, 2013

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$http config.params doesn't treat array values properly
6 participants