current mutates the original objects unexpectedly

[francescotescari]

Hello!
There is a new bug in the current coming from this PR that actually changes the original objects unexpectedly, breaking:

• Use of frozen objects in the draft (this actually makes my app crash)
• Weird behavior when using nested drafts.

Here are two failing tests:

    it("doesn't assign values to frozen object", () => {
      const frozen = Object.freeze({ test: 42 })
      const base = { k: null };
      produce(base, (draft) => {
        draft.k = frozen;
        const c = current(draft); // Boom! tries to set a value in the frozen object
        expect(c.k).toBe(frozen);
      });
    });

    it("nested drafts work after current", () => {
      const base = { k1: {}, k2: {} };
      const result = produce(base, (draft) => {
        const obj = { x: draft.k2 };
        draft.k1 = obj;
        current(draft); // try to comment me
        obj.x.abc = 100;
        draft.k2.def = 200;
      });
      expect(result).toEqual({ k1: { x: { abc: 100, def: 200 }}, k2: { abc: 100, def: 200 } });
    });

The first should be solvable by not assigning the value to the parent if the getCurrent(child value) has same identity as the child value (or if the object is frozen).
For the second one, I don't see ways of escaping the shallow copy.

[unadlib]

hi @francescotescari, based on your previous PR, indeed comparing the values after traversal can solve these two issue.

Regarding the second use case, does it not meet your expectations? result.k1.x and result.k2 share the draft, so they should actually be the same reference.

The second use case should be like this:

it('nested drafts work after current', () => {
  const base = { k1: {}, k2: {} };
  const result = create(base, (draft) => {
    const obj = { x: draft.k2 };
    draft.k1 = obj;
    // current(draft); // try to comment me
    obj.x.abc = 100;
    draft.k2.def = 200;
  });
  expect(result.k1.x).toBe(result.k2);
  expect(result).toEqual({
    k1: { x: { abc: 100, def: 200 } },
    k2: { abc: 100, def: 200 },
  });
});

[francescotescari]

Regarding the second use case, does it not meet your expectations? result.k1.x and result.k2 share the draft, so they should actually be the same reference.

Indeed, the failing test is with current uncommented: obj gets mutated by current and obj.x (which was a reference to draft.k2) becomes (unexpectedly) a shallow copy of draft.k2 instead. I think current should not have any side-effect on the draft or the child objects (e.g. obj). The test shows how the result of create is different if you invoke current or not in the middle, which is unexpected.

[unadlib]

I think current should not have any side-effect on the draft or the child objects (e.g. obj)

I completely agree. This issue has been fixed in v1.0.8.