cloneDeep breaking change

[KamalAman]

Intended outcome:
Unchanged objects should not be modified when caching lastResults, and Symbols should be preserved with cloneDeep .

Version 2.4.3 supported copying symbols to new objects while 2.4.4 does not
This PR introduced the bug: #4052

While the effects of this bug being introduced seemed resolved by #4069 in 2.4.5 due to the cloned object only being used by lastResultSnapshot for isDifferentFromLastResult,the underlying bug still exists.

Furthermore, there is no reason to deep clone objects. Objects should be immutable, shallow copy object when modifying them. It seems like the strategy in this PR #4089

Actual outcome:
Object symbols are not copied over

How to reproduce the issue:

let obj = {};
obj[Symbol('a')] = 'a';

let clonedObject = cloneDeep(obj);

expect(clonedObject[Symbol('a')]).to.equal('a')

Versions
Introduced in apollo-client 2.4.4
Side-effects resolved in apollo-client 2.4.5, however bug still technically exists

[benjamn]

Symbol properties were not copied by the previous fclone implementation in [email protected] (i.e. [email protected]) and earlier, so this is only a breaking change with respect to a very temporary implementation that was found to be prohibitively slow, as discussed in #4039. In some sense, the act of copying symbols was a breaking change that we have now undone.

Furthermore, there is no reason to deep clone objects. Objects should be immutable, shallow copy object when modifying them.

That's a nice aspiration that you are welcome to pursue in your own code, but as the maintainers of a library written in a language that does not enforce immutability in the type system, it's important to appreciate that any object exposed to application code can and will be modified by someone.

I'm not interested in bugs that only technically exist. Everyone else is here to get help building real applications. Feel free to reopen this issue when you have an actual use case to discuss.