immerjs / immer

Create the next immutable state by mutating the current one
https://immerjs.github.io/immer/
MIT License
27.53k stars 852 forks source link

Does produce() perform strict equality check (===) when setting values in receipt? #1073

Closed d6u closed 12 months ago

d6u commented 12 months ago

🙋‍♂ Question

Thanks in advance!

I have below example:

const newData = produce(
  oldData,
  (draft) => {
    draft.tasks[0].title = draft.tasks[0].title;
  }
);

My questions are:

  1. Since draft.tasks[0].title is assigned to draft.tasks[0].title, will that cause oldData === newData to be true?
  2. How about when draft.tasks[0].title is "Hello" and I do draft.tasks[0].title = "Hello"?
  3. This is expected behavior that I can rely on or undefined behavior that I shouldn't rely on?

I expect that if I'm replacing the a object or array, produce is probably not going to perform deep comparison before applying the changes/patches. But I'm wondering if I can rely on immer to not generate a new object if primitive values are the same. Right now based on my observation, oldData === newData is indeed true.

Link to repro

N/A

Environment

We only accept questions against the latest Immer version.

markerikson commented 12 months ago

A correct immutable update will make copies of every level of nesting.

If you want to to update state.some.nested.field = 123, it needs to make:

So in a normal update, newData !== oldData, because the new object tree did have to make copies of tasks[0], tasks, and draft (aka data).

However, it looks in this example like you're just setting the exact same value. In that case, yes, I think Immer is smart enough to detect that nothing actually changed, and the overall result will be a no-op, no updates or copies performed.

d6u commented 12 months ago

A correct immutable update will make copies of every level of nesting.

If you want to to update state.some.nested.field = 123, it needs to make:

  • a copy of nested with a new field value
  • a copy of some that has the new nested value
  • a copy of state that has the new some value

So in a normal update, newData !== oldData, because the new object tree did have to make copies of tasks[0], tasks, and draft (aka data).

However, it looks in this example like you're just setting the exact same value. In that case, yes, I think Immer is smart enough to detect that nothing actually changed, and the overall result will be a no-op, no updates or copies performed.

Got it. Thanks for explaining. Can I rely on this behavior in future versions? In order words, is this a documented and expected behavior that the Immer maintainers will ensure to not break?

markerikson commented 12 months ago

As far as I know, yes.

mweststrate commented 12 months ago

Yes.

On Tue, 19 Sept 2023, 21:56 Daiwei Lu, @.***> wrote:

A correct immutable update will make copies of every level of nesting.

If you want to to update state.some.nested.field = 123, it needs to make:

  • a copy of nested with a new field value
  • a copy of some that has the new nested value
  • a copy of state that has the new some value

So in a normal update, newData !== oldData, because the new object tree did have to make copies of tasks[0], tasks, and draft (aka data).

However, it looks in this example like you're just setting the exact same value. In that case, yes, I think Immer is smart enough to detect that nothing actually changed, and the overall result will be a no-op, no updates or copies performed.

Got it. Thanks for explaining. Can I rely on this behavior in future versions? In order words, is this a documented and expected behavior that the Immer maintainer want to ensure?

— Reply to this email directly, view it on GitHub https://github.com/immerjs/immer/issues/1073#issuecomment-1726384317, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBBN6KGV2TZPVEQRTWDX3H2F5ANCNFSM6AAAAAA462DYGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

d6u commented 12 months ago

Thanks! That answered me question.

Akimotorakiyu commented 8 months ago

@markerikson please have a look. In above case, nothing change, ref is the same, but in below case,not meeting expectations. nothing change, but the ref is not the same.

const baseState = {
  a: 1,
};

const newState = produce(
  baseState,
  (draft) => {
    draft.a = 2;
    draft.a = 1;
  },
  (patch) => {
    // tobe: []
    console.log(patch);
  }
);

// tobe: false
console.log( newState === baseState);
mweststrate commented 8 months ago

Just try?

On Fri, 22 Dec 2023, 04:15 湫曗, @.***> wrote:

const baseState = { a: 1,}; const newState = produce( baseState, (draft) => { draft.a = 2; draft.a = 1; }, (patch, f) => { // tobe: [] console.log(patch, f); }); // tobe: falseconsole.log( newState === baseState);

— Reply to this email directly, view it on GitHub https://github.com/immerjs/immer/issues/1073#issuecomment-1867182076, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBECPJDGNKTKZUWRAGDYKT3OPAVCNFSM6AAAAAA462DYGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRXGE4DEMBXGY . You are receiving this because you commented.Message ID: @.***>