Skip to content
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

Adjusted query string array representation to remove indices. #8342

Merged
merged 2 commits into from Feb 10, 2017

Conversation

@hwillson
Copy link
Member

@hwillson hwillson commented Feb 9, 2017

Hi guys - after some discussion about the implementation provided in PR #8261, it looks like we should adjust the implemented array handling a bit. Right now arrays are encoded with indices like:

some_array[0]=value1&some_array[1]=value2&some_array[2]=value3

While this implementation is somewhat supported by external languages and frameworks, it's not as well supported (by default) as the following query string representation for arrays:

some_array[]=value1&some_array[]=value2&some_array[]=value3

For example, PHP and Rails both expect no indices for arrays.

This PR adjusts the url package to make sure query string encoded arrays do not include indices. Thanks!

Copy link
Member

@benjamn benjamn left a comment

Could you also increment the version in package.js as you see fit?

@hwillson
Copy link
Member Author

@hwillson hwillson commented Feb 10, 2017

Shoot - sure thing @benjamn, thanks!

…ackwards compatiable.
@benjamn benjamn merged commit 0830967 into meteor:devel Feb 10, 2017
3 checks passed
3 checks passed
CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
abernix added a commit that referenced this pull request Feb 10, 2017
* Adjusted query string array representation to remove indices.

* Increased major version since the new _encodeParams() result is not backwards compatiable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants