Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions lib/mergeDeep.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,17 @@ class MergeDeep {

// If the target is empty, then all the source is added to additions
if (t === undefined || t === null || (this.isEmpty(t) && !this.isEmpty(s))) {
additions = Object.assign(additions, s)
return ({ additions, modifications, hasChanges: true })
if (firstInvocation && Array.isArray(s)) {
// Match the shape of the non-empty-target path, which unwinds the
// `__array` wrapper before returning: a top-level array diff must come
// back as an array, not as the index-keyed object Object.assign builds.
additions = s.slice()
} else {
Comment thread
tdabasinskas marked this conversation as resolved.
// Assign into the container passed by recursive callers so they observe
// the additions through their own reference.
additions = Object.assign(additions, s)
Comment thread
tdabasinskas marked this conversation as resolved.
Outdated
}
return ({ additions, modifications, deletions, hasChanges: true })
}

// Compare the entries in the objects or elements of the array
Expand All @@ -92,8 +101,8 @@ class MergeDeep {
// So any property in the target that is not in the source is not treated as a deletion
for (const key in source) {
// Skip prototype pollution vectors
if (key === "__proto__" || key === "constructor") {
continue;
if (key === '__proto__' || key === 'constructor') {
continue
}
// Logic specific for Github
// API response includes urls for resources, or other ignorable fields; we can ignore them
Expand Down
25 changes: 25 additions & 0 deletions test/unit/lib/mergeDeep.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,7 @@ entries:
]
},
modifications: {},
deletions: {},
hasChanges: true
}
const ignorableFields = []
Expand Down Expand Up @@ -1882,4 +1883,28 @@ branches:
expect(same.additions).toEqual({})
expect(same.modifications).toEqual({})
})

it('CompareDeep returns top-level array additions in the same shape for empty and non-empty targets', () => {
const mergeDeep = new MergeDeep(log, jest.fn(), [])
const ruleset = { name: 'first-ruleset', enforcement: 'active' }

// Target already has an entry: additions come back as an array
const withExisting = mergeDeep.compareDeep(
[{ name: 'other', enforcement: 'active' }],
[{ name: 'other', enforcement: 'active' }, ruleset]
)
expect(withExisting.additions).toEqual([ruleset])

// Empty target (e.g. a repo receiving its first ruleset) must produce the
// same shape, not an index-keyed object like { 0: { ... } }
const firstEntry = mergeDeep.compareDeep([], [ruleset])
expect(firstEntry.additions).toEqual([ruleset])
expect(firstEntry.hasChanges).toEqual(true)
})

it('CompareDeep always includes deletions in its result', () => {
const mergeDeep = new MergeDeep(log, jest.fn(), [])
const emptyTarget = mergeDeep.compareDeep([], [{ name: 'a' }])
expect(emptyTarget).toHaveProperty('deletions')
})
})