Repository: sindresorhus/query-string
Pull request #82 (2017-01-13 08:18:46)

Preserve appearance order while handling multiple values
Examples:
foo=bar&foo= -> ['bar', '']
foo=&foo=bar -> ['', 'bar']

doochik: This is regression in v4.3.0. v4.2.3 works fine
sindresorhus: // @rapzo
rapzo: Yeah, makes sense. It's sorting the array payload as well.
Although is there really a need to rename the function for that? I understand it only sort keys and all... Ok :-)
Plus, if going beyond 80 line chars is fine, seems legit.
Oh, and please fix _appearence_ to _appearance_.

But please add test cases for the other type of formats, just to make it a strong case, such as:

``` 
test('handle multiple values and preserve appearance order with brackets', t => {
	t.deepEqual(fn.parse('a[]=value&a[]=', {arrayFormat: 'bracket'}), {a: ['value', '']});
	t.deepEqual(fn.parse('a[]=&a[]=value', {arrayFormat: 'bracket'}), {a: ['', 'value']});
});

test('handle multiple values and preserve appearance order with indexes', t => {
	t.deepEqual(fn.parse('a[0]=value&a[1]=', {arrayFormat: 'index'}), {a: ['value', '']});
	t.deepEqual(fn.parse('a[1]=&a[0]=value', {arrayFormat: 'index'}), {a: ['value', '']});
});
```

And for `stringify` too, maybe.
doochik: @rapzo I've renamed function to make it more obviously :)

1) Reformat condition to fix long line
2) Add tests for brackets
3) `stringify` has such tests already https://github.com/sindresorhus/query-string/blob/master/test/stringify.js#L29


rapzo: I don't want to be the bad guy here but not being symmetric makes me hitch heheh, @sindresorhus should have a word on it though.

What i'd suggest would be:

```
var val = ret[key];

if (Boolean(val) && typeof val === 'object' && !Array.isArray(val)) {
  result[key] = sorter(val);
} else {
  result[key] = val;
}
```

The `Boolean` can also be replaced with `!!` but personally i find it confusing (`sudo !!` too much hehe). What do you think?
SamVerschueren: > The Boolean can also be replaced with !!

No, `Boolean` is better.

And yes, put it on one line.
doochik: Ok, i've rewrited condition
rapzo: 👍 

Patch:
diff --git a/index.js b/index.js
index e21c72b..a92a592 100644
--- a/index.js
+++ b/index.js
@@ -96,11 +96,11 @@ function encode(value, opts) {
 	return value;
 }
 
-function sorter(input) {
+function keysSorter(input) {
 	if (Array.isArray(input)) {
 		return input.sort();
 	} else if (typeof input === 'object') {
-		return sorter(Object.keys(input)).sort(function (a, b) {
+		return keysSorter(Object.keys(input)).sort(function (a, b) {
 			return Number(a) - Number(b);
 		}).map(function (key) {
 			return input[key];
@@ -148,10 +148,12 @@ exports.parse = function (str, opts) {
 	});
 
 	return Object.keys(ret).sort().reduce(function (result, key) {
-		if (Boolean(ret[key]) && typeof ret[key] === 'object') {
-			result[key] = sorter(ret[key]);
+		var val = ret[key];
+		if (Boolean(val) && typeof val === 'object' && !Array.isArray(val)) {
+			// Sort object keys, not values
+			result[key] = keysSorter(val);
 		} else {
-			result[key] = ret[key];
+			result[key] = val;
 		}
 
 		return result;


Test patch:
diff --git a/test/parse.js b/test/parse.js
index b3d8ab8..66eb363 100644
--- a/test/parse.js
+++ b/test/parse.js
@@ -53,6 +53,21 @@ test('handle multiple of the same key', t => {
 	t.deepEqual(fn.parse('foo=bar&foo=baz'), {foo: ['bar', 'baz']});
 });
 
+test('handle multiple values and preserve appearence order', t => {
+	t.deepEqual(fn.parse('a=value&a='), {a: ['value', '']});
+	t.deepEqual(fn.parse('a=&a=value'), {a: ['', 'value']});
+});
+
+test('handle multiple values and preserve appearance order with brackets', t => {
+	t.deepEqual(fn.parse('a[]=value&a[]=', {arrayFormat: 'bracket'}), {a: ['value', '']});
+	t.deepEqual(fn.parse('a[]=&a[]=value', {arrayFormat: 'bracket'}), {a: ['', 'value']});
+});
+
+test('handle multiple values and preserve appearance order with indexes', t => {
+	t.deepEqual(fn.parse('a[0]=value&a[1]=', {arrayFormat: 'index'}), {a: ['value', '']});
+	t.deepEqual(fn.parse('a[1]=&a[0]=value', {arrayFormat: 'index'}), {a: ['value', '']});
+});
+
 test('query strings params including embedded `=`', t => {
 	t.deepEqual(fn.parse('?param=http%3A%2F%2Fsomeurl%3Fid%3D2837'), {param: 'http://someurl?id=2837'});
 });