query-string icon indicating copy to clipboard operation
query-string copied to clipboard

Default arrayFormat causes arrays of length 1 to be serialised the same as a string

Open hedgepigdaniel opened this issue 4 years ago • 4 comments

The default arrayFormat value of none is problematic because arrays of length 1 are serialised to the same format as if the value was the string contained within the array.

For example:

qs.parse(qs.stringify({a: 'val'})); // {a: 'val'}
qs.parse(qs.stringify({a: ['val']})); // {a: 'val'} WRONG/UNEXPECTED
qs.parse(qs.stringify({a: ['val', 'val2']})); // {a: ['val', 'val2']}

Perhaps the default arrayFormat should be changed to bracket or index, which do not have this problem. The none option would make more sense IMO if it refused to stringify an array when stringifying, and ignored/crash for duplicate keys when parsing. I don't see how the comma option can be made to handle arrays consistently.

hedgepigdaniel avatar Sep 03 '19 03:09 hedgepigdaniel

While I agree with you it's not optimal, people have been using this for years without complaining, so it cannot be that big of an issue. Not sure it's worth doing a breaking change to change the default, but we should at least make the "catches" clearer in the docs.

// @zepod @rapzo @yaodingyd Mentioning you in case you have an opinion on this.

sindresorhus avatar Oct 02 '19 17:10 sindresorhus

I think better documentation would be fine, this is not worth a code change.

yaodingyd avatar Oct 02 '19 17:10 yaodingyd

I understand the importance of predictability and wouldn't condone at all any code breaking change.

rapzo avatar Oct 02 '19 18:10 rapzo

I think we could improve the situation by supporting optionally defining the type of one or more keys. See: https://github.com/sindresorhus/query-string/issues/228#issuecomment-575197530

sindresorhus avatar Jan 16 '20 15:01 sindresorhus