produceWithPatches creates patch when assigning a reference to itself

[smirea]

🐛 Bug Report

When using produceWithPatches, if you mutate a reference and then assign it to itself, the resulting patch would be a replace on the parent, instead of a replace on the nested field

Link to repro

Example: https://codesandbox.io/s/immer-issue-producewithpatches-nested-change-assign-to-self-vmd5l?file=/src/index.ts

To Reproduce

const input = {
    obj: {
        value: 100,
    },
};

produceWithPatches(input, draft => {
    draft.obj.value = 200;
    draft.obj = draft.obj;
});

Observed behavior

The above returns a single patch of:

{ op: 'replace', path: ['obj'], value: { value: 200 } }

Expected behavior

It should not count assigning a reference to itself as a replace and just ignore it. The correct patch should be:

{ op: 'replace', path: ['obj', 'value'], value: 200 }

Environment

Tested on 7.0.7

• Immer version:
• I filed this report against the latest version of Immer
• Occurs with setUseProxies(true)
• Occurs with setUseProxies(false) (ES5 only)

Full Example

[mweststrate]

Per documentation, the patches generated by Immer are correct, but not necessarily the smallest patch possible. Will update the docs to reflect that.

[smirea]

If you don't mind checking this patch: #690

It seems to solve the issue example codesandbox

The only issue I see with the tests is handling the -0 use-case, thus the ugly check for typeof === number. I haven't spent too much time in the codebase to know if there's a better way. That is only an issue when the references are equal to start with, but still a perf hit

[mweststrate]

🎉 This issue has been resolved in version 8.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[smirea]

☝️ it hasn't actually been resolved, still an issue, it's just marked as as ignored in the readme

edit: nevermind, it actually has been fixed 🎉

[mweststrate]

@smirea so the PR wasn't a fix for this issue?

[smirea]

oh i completely missed that it got merged, sorry about that.

just tested it on 8.0.2 and it works fine, thank you 🎉