Skip to content

Commit 538b180

Browse files
committed
fix: deep array merge
deep-extend does not support array merge. There was special code added to merge top-level arrays, but that was a shallow merge. Use deepmerge instead of deep-extend to merge arrays also. Default merge settings seem to work well - all tests pass. Extend all-of merge test case based on swagger-api/swagger-ui#7618 Fixes: 2f5bb86 ("Fix and test for swagger-ui #3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
1 parent 45d2375 commit 538b180

File tree

4 files changed

+31
-57
lines changed

4 files changed

+31
-57
lines changed

package-lock.json

Lines changed: 2 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@
109109
"btoa": "^1.2.1",
110110
"cookie": "~0.4.1",
111111
"cross-fetch": "^3.1.4",
112-
"deep-extend": "~0.6.0",
112+
"deepmerge": "~4.2.2",
113113
"fast-json-patch": "^3.0.0-1",
114114
"form-data-encoder": "^1.4.3",
115115
"formdata-node": "^4.0.0",

src/specmap/lib/index.js

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as jsonPatch from 'fast-json-patch';
2-
import deepExtend from 'deep-extend';
3-
import cloneDeep from 'lodash/cloneDeep';
2+
import deepmerge from 'deepmerge';
43

54
export default {
65
add,
@@ -40,42 +39,8 @@ function applyPatch(obj, patch, opts) {
4039
jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]);
4140
} else if (patch.op === 'mergeDeep') {
4241
const currentValue = getInByJsonPath(obj, patch.path);
43-
44-
// Iterate the properties of the patch
45-
// eslint-disable-next-line no-restricted-syntax, guard-for-in
46-
for (const prop in patch.value) {
47-
const propVal = patch.value[prop];
48-
const isArray = Array.isArray(propVal);
49-
if (isArray) {
50-
// deepExtend doesn't merge arrays, so we will do it manually
51-
const existing = currentValue[prop] || [];
52-
currentValue[prop] = existing.concat(propVal);
53-
} else if (isObject(propVal) && !isArray) {
54-
// If it's an object, iterate it's keys and merge
55-
// if there are conflicting keys, merge deep, otherwise shallow merge
56-
let currentObj = { ...currentValue[prop] };
57-
// eslint-disable-next-line no-restricted-syntax
58-
for (const key in propVal) {
59-
if (Object.prototype.hasOwnProperty.call(currentObj, key)) {
60-
// if there is a single conflicting key, just deepExtend the entire value
61-
// and break from the loop (since all future keys are also merged)
62-
// We do this because we can't deepExtend two primitives
63-
// (currentObj[key] & propVal[key] may be primitives).
64-
//
65-
// we also deeply assign here, since we aren't in control of
66-
// how deepExtend affects existing nested objects
67-
currentObj = deepExtend(cloneDeep(currentObj), propVal);
68-
break;
69-
} else {
70-
Object.assign(currentObj, { [key]: propVal[key] });
71-
}
72-
}
73-
currentValue[prop] = currentObj;
74-
} else {
75-
// It's a primitive, just replace existing
76-
currentValue[prop] = propVal;
77-
}
78-
}
42+
const newValue = deepmerge(currentValue, patch.value);
43+
obj = jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]).newDocument;
7944
} else if (patch.op === 'add' && patch.path === '' && isObject(patch.value)) {
8045
// { op: 'add', path: '', value: { a: 1, b: 2 }}
8146
// has no effect: json patch refuses to do anything.

test/specmap/all-of.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ describe('allOf', () => {
436436
});
437437
});
438438

439-
test('merges arrays inside of an `allOf`', () =>
439+
test('deepmerges arrays inside of an `allOf`', () =>
440440
mapSpec({
441441
plugins: [plugins.refs, plugins.allOf],
442442
showDebug: true,
@@ -450,6 +450,12 @@ describe('allOf', () => {
450450
{
451451
type: 'object',
452452
required: ['a', 'b'],
453+
properties: {
454+
nested: {
455+
type: 'object',
456+
required: ['e'],
457+
},
458+
},
453459
},
454460
],
455461
},
@@ -458,6 +464,12 @@ describe('allOf', () => {
458464
{
459465
type: 'object',
460466
required: ['c', 'd'],
467+
properties: {
468+
nested: {
469+
type: 'object',
470+
required: ['f'],
471+
},
472+
},
461473
},
462474
],
463475
},
@@ -470,10 +482,22 @@ describe('allOf', () => {
470482
one: {
471483
type: 'object',
472484
required: ['c', 'd', 'a', 'b'],
485+
properties: {
486+
nested: {
487+
type: 'object',
488+
required: ['f', 'e'],
489+
},
490+
},
473491
},
474492
two: {
475493
type: 'object',
476494
required: ['c', 'd'],
495+
properties: {
496+
nested: {
497+
type: 'object',
498+
required: ['f'],
499+
},
500+
},
477501
},
478502
},
479503
});

0 commit comments

Comments
 (0)