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

[fix] parsing a field that holds array of arrays #335

Merged
merged 1 commit into from Oct 6, 2019

Conversation

@Om4ar
Copy link
Contributor

commented Oct 4, 2019

fixes #327

…rrays

Fixes #327.
@@ -87,6 +87,10 @@ var parseValues = function parseQueryStringValues(str, options) {
val = val.split(',');
}

if(part.indexOf('[]=') > -1){

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 4, 2019

Owner
Suggested change
if(part.indexOf('[]=') > -1){
if (part.indexOf('[]=') > -1) {

what happens with foo[][]=bar, or just []=a?

This comment has been minimized.

Copy link
@Om4ar

Om4ar Oct 4, 2019

Author Contributor

@ljharb

foo[][]=bar will be parsed to { foo: ['bar'] }

[]=a will be parsed to { 0: 'a' }

my code doesn't affect these results, it will be the same result

lib/parse.js Outdated Show resolved Hide resolved
lib/parse.js Outdated Show resolved Hide resolved
@@ -1,7 +1,10 @@
'use strict';

var has = Object.prototype.hasOwnProperty;
var isArray = Array.isArray;

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 6, 2019

Owner

whoops, turns out we already depended on this being available ¯\_(ツ)_/¯

This comment has been minimized.

Copy link
@Om4ar

Om4ar Oct 6, 2019

Author Contributor

@ljharb yes it is being used in stringify.js file too =D

This comment has been minimized.

Copy link
@Om4ar

Om4ar Oct 6, 2019

Author Contributor

@ljharb
should i revert my changes on utils file and make another pr to replace Array.isArray with the polyfill across all the files ?

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 6, 2019

Owner

nah for now just use Array.isArray the same way it's used in stringify.js.

This comment has been minimized.

Copy link
@Om4ar

Om4ar Oct 6, 2019

Author Contributor

okay, will revert my is changes for using this pollyfill

@Om4ar Om4ar requested a review from ljharb Oct 6, 2019
@ljharb ljharb force-pushed the Om4ar:fix-comma-bracket branch from 3da589c to f884e2d Oct 6, 2019
@ljharb
ljharb approved these changes Oct 6, 2019
@ljharb ljharb merged commit f884e2d into ljharb:master Oct 6, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.