Skip to content

Commit

Permalink
[Fix] ensure utils.merge handles merging two arrays.
Browse files Browse the repository at this point in the history
  • Loading branch information
ljharb committed Jul 22, 2016
1 parent 98f59a6 commit 988dbe4
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
24 changes: 20 additions & 4 deletions lib/utils.js
@@ -1,5 +1,7 @@
'use strict';

var has = Object.prototype.hasOwnProperty;

var hexTable = (function () {
var array = [];
for (var i = 0; i < 256; ++i) {
Expand All @@ -10,7 +12,7 @@ var hexTable = (function () {
}());

exports.arrayToObject = function (source, options) {
var obj = options.plainObjects ? Object.create(null) : {};
var obj = options && options.plainObjects ? Object.create(null) : {};
for (var i = 0; i < source.length; ++i) {
if (typeof source[i] !== 'undefined') {
obj[i] = source[i];
Expand Down Expand Up @@ -46,6 +48,21 @@ exports.merge = function (target, source, options) {
mergeTarget = exports.arrayToObject(target, options);
}

if (Array.isArray(target) && Array.isArray(source)) {
source.forEach(function (item, i) {
if (has.call(target, i)) {
if (target[i] && typeof target[i] === 'object') {
target[i] = exports.merge(target[i], item, options);
} else {
target.push(item);
}
} else {
target[i] = item;
}
});
return target;
}

return Object.keys(source).reduce(function (acc, key) {
var value = source[key];

Expand Down Expand Up @@ -143,10 +160,9 @@ exports.compact = function (obj, references) {
}

var keys = Object.keys(obj);
for (var j = 0; j < keys.length; ++j) {
var key = keys[j];
keys.forEach(function (key) {
obj[key] = exports.compact(obj[key], refs);
}
});

return obj;
};
Expand Down
13 changes: 13 additions & 0 deletions test/utils.js
Expand Up @@ -5,5 +5,18 @@ var utils = require('../lib/utils');

test('merge()', function (t) {
t.deepEqual(utils.merge({ a: 'b' }, { a: 'c' }), { a: ['b', 'c'] }, 'merges two objects with the same key');

var oneMerged = utils.merge({ foo: 'bar' }, { foo: { first: '123' } });
t.deepEqual(oneMerged, { foo: ['bar', { first: '123' }] }, 'merges a standalone and an object into an array');

var twoMerged = utils.merge({ foo: ['bar', { first: '123' }] }, { foo: { second: '456' } });
t.deepEqual(twoMerged, { foo: { 0: 'bar', 1: { first: '123' }, second: '456' } }, 'merges a standalone and two objects into an array');

var sandwiched = utils.merge({ foo: ['bar', { first: '123', second: '456' }] }, { foo: 'baz' });
t.deepEqual(sandwiched, { foo: ['bar', { first: '123', second: '456' }, 'baz'] }, 'merges an object sandwiched by two standalones into an array');

var nestedArrays = utils.merge({ foo: ['baz'] }, { foo: ['bar', 'xyzzy'] });
t.deepEqual(nestedArrays, { foo: ['baz', 'bar', 'xyzzy'] });

t.end();
});

2 comments on commit 988dbe4

@ysangkok
Copy link

@ysangkok ysangkok commented on 988dbe4 Dec 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem like this fix affects the consumer of the library in any way. I am testing with strings like a[0][0][]=1&a[][0][]=2. Can this be true?

@ljharb
Copy link
Owner Author

@ljharb ljharb commented on 988dbe4 Dec 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added tests would fail prior to the fix.

It definitely affects qs.parse. Compare JSON.stringify(qs.parse('foo[]=bar&foo[][first]=123&foo[second]=456')) which produces '{"foo":{"0":["bar",{"first":"123"}],"second":"456"}}' before, and produces '{"foo":{"0":"bar","1":{"first":"123"},"second":"456"}}' after.

Please sign in to comment.