mergeDeep behaviour with `undefined` values changed in 10.6.2

[angrykoala]

trafficstars

Issue workflow progress

Progress of the issue based on the Contributor Workflow

• [ ] 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

  Make sure to fork this template and run yarn generate in the terminal.

  Please make sure the GraphQL Tools package versions under package.json matches yours.

• [ ] 2. A failing test has been provided
• [ ] 3. A local solution has been provided
• [ ] 4. A pull request is pending review

---

Describe the bug

In version 10.6.1 of @graphql-tools/utils, undefined values are ignored from mergeDeep. For example:

mergeDeep([{a:"dsa"}, {a:"dd", b: 1}, undefined])

Returns { a: 'dd', b: 1 }

In version 10.6.2 and beyond, the same input returns undefined

To Reproduce Steps to reproduce the behavior:

In version 10.6.2:

mergeDeep([{a:"dsa"}, {a:"dd", b: 1}, undefined])

Returns undefined

Expected behavior

To return the same as 10.6.1:

{ a: 'dd', b: 1 }

[angrykoala]

In addition to the aforementioned problem the following inputs are also different between 10.6.1 and 10.6.2

Input	10.6.1	10.6.2
mergeDeep([undefined])	{}	undefined
mergeDeep([])	{}	[]
mergeDeep([{}])	{}	undefined

[ardatan]

Thanks for the feedback! I created the PR for fixing the behavior! https://github.com/ardatan/graphql-tools/pull/7012

[angrykoala]

Thanks for the quick turnaround.

One question: per the PR I see that some cases (e.g. "no sources should return undefined") are not really what I expected, that behavior is different to how it was in 10.6.1 (always returning an empty object) and I expected a change like that to be considered a breaking change (i.e. released in a major release)

Here I assuming graphql-tools follow semantic versioning or a similar versioning

Thanks

[ardatan]

We follow semantic versioning, right.

The behavior was broken and not intended as you can see in the tests. There is no change in the API but the result is corrected in the PR. So it is a bug fix not a major breaking change in my opinion. mergeDeep takes an array of values to be merged. So if there is no value in the passed array, it should be undefined not an empty object. If an array with undefined values passed, nullish values should be ignored like you expected in the original issue, and the result should be undefined. I am not sure if relying on a broken result is a good idea. Let me know if I am missing something here.

[angrykoala]

Well, that behavior has been how this tool has been working for a long time if I'm not mistaken, and as this particular case was not documented afaik, we assumed that to be the expected behavior (always return an object) and relied on that for our tool as we don't have any checks for the result after mergeDeep

I agree that relying on unintended behavior is not ideal, and we are not massively affected by this as we can always add some extra checks on our end before updating, but wanted to warn about the potential breaking of other tools by this fix