Unexpected JSON patches when calling array.replace()

[philllies]

When replacing an object of type types.array why does mobx generate add/remove patches rather than a single replace patch?

• [X] I've checked documentation and searched for existing issues
• [X] I tried the spectrum channel

Given the example below, why does MST create JSON patches to remove and add elements individually when calling array.replace rather than creating a single replace patch?

const Foo = types.model({
  Id: types.identifier
});

const Bar = types
  .model({
    Foos: types.array(Foo)
  })
  .actions(self => ({
    SetFoos(value): void {
      self.Foos.replace(value);
    }
  }));

const bar = Bar.create({
  Foos: [{ Id: "0" }, { Id: "1" }]
});

onPatch(bar, patch => console.log(patch));

bar.SetFoos([{ Id: "2" }, { Id: "3" }]);

// generated patches:
// {"op":"remove","path":"/Foos/1"} 
// {"op":"remove","path":"/Foos/0"} 
// {"op":"add","path":"/Foos/0","value":{"Id":"2"}} 
// {"op":"add","path":"/Foos/1","value":{"Id":"3"}} 

// expected patch
// {op:"replace", path:"/Foos", value:[{ Id: "2" }, { Id: "3" }] }

types.map behaves analogously.

https://codesandbox.io/s/trusting-hooks-fkfbp?file=/src/index.ts

[rgranger]

I encountered the same issue.

The behavior is the same for the following :

• self.array = newArray
• applySnapshot(self.array, newArray)
• self.array.replace(newArray)

The issue is the following : when replacing an array of length N by another array of length M, N JSON patches "remove", and M JSON patches "add" are emitted, whereas there could be a single "replace".

I am using many simple arrays of valueType (for exemple array<number>). These arrays can have a big size (1000 items for example). I have actions which can replace the entire array (for example Reset to the default value).

When this action is triggered, 2000 JSON patches are emitted (1000 "remove", 1000 "add") which really slows down the application unnecessarily.

Is there a way to generate a single "replace" JSON patch for this operation instead ?

[rgranger]

As a workaround, i'm using the type frozen instead : https://mobx-state-tree.js.org/API/#frozen

The downside is this type can't be mutated. But a replacement of the value triggers a single "replace" JSON patch, as expected.

[cannc4]

Were you able to get to the bottom of this @rgranger ?

[rgranger]

No, nothing new since the last time :

As a workaround, i'm using the type frozen instead : https://mobx-state-tree.js.org/API/#frozen The downside is this type can't be mutated. But a replacement of the value triggers a single "replace" JSON patch, as expected.

[philllies]

I found that replacing the array treenode with a new one rather than assigning a snapshot leads to a single emitted patch only:

const Foo = types.model({
  Id: types.identifier
});

const Bar = types
  .model({
    Foos: types.array(Foo)
  })
  .actions((self) => ({
    SetFoos(value): void {
      self.Foos = getType(self.Foos).create(value);
    }
  }));

const bar = Bar.create({
  Foos: [{ Id: "0" }, { Id: "1" }]
});

onPatch(bar, (patch) => console.log(patch));

bar.SetFoos([{ Id: "2" }, { Id: "3" }]);

https://codesandbox.io/s/great-bush-2rc4j?file=/src/index.ts

[rgranger]

This is a great solution, thanks !

[jrmyio]

I think this issue is also related to: https://github.com/mobxjs/mobx-state-tree/issues/799

The issue there is that remove/add events are triggered while the items within the array actually stayed the same. When you are not using types.map it is causing unexpected behavior:

https://codesandbox.io/s/hardcore-brahmagupta-h84gc?file=/src/index.ts

[coolsoftwaretyler]

Hey folks - sorry it's been so long since we had movement on this bug, but I appreciate all of the reproductions here.

I think like #2006, we're going to mark this as a bug, and see where/when it ends up on a roadmap to fix. Open for assistance if anyone is interested in pitching in. I'd be happy to help folks get started!

[BrianHung]

The primary cause of the issue is that mobx array.replace is a shallow function over splice

    replace(newItems: any[]) {
        const adm: ObservableArrayAdministration = this[$mobx]
        return adm.spliceWithArray_(0, adm.values_.length, newItems)
    },

https://github.com/mobxjs/mobx/blob/d813746cfaa18d80daddee3724562fed6b307c0a/packages/mobx/src/types/observablearray.ts#L427-L430

What we could do in MST is optimize patches so that when the entire contents of an array are replaced, we just produce one patch. We can do that here: https://github.com/mobxjs/mobx-state-tree/blob/94bb7d02de6ed3e2c3f92a6a6ddadc16644597f0/packages/mobx-state-tree/src/types/complex-types/array.ts#L228-L263

[BrianHung]

There is a separate issue from re-applying patches to an array that were just generated from onPatch. Expectation is that if values are deeply equal, no patches should be emitted. However, during reconciliation, only identity is checked.

https://github.com/mobxjs/mobx-state-tree/blob/94bb7d02de6ed3e2c3f92a6a6ddadc16644597f0/packages/mobx-state-tree/src/core/node/object-node.ts#L530

[BrianHung]

Looking further into this issue, array clear and replace can be optimized to emit a single patch. However, map merge and replace cannot because mobx itself doesn't emit granular enough events for it, that is optimizing patches for map will require changing mobx upstream first.

That being said, onPatch does not respect transaction boundaries, that is patches are not compressed. To compress patches, you can either use immer or a radix trie. https://github.com/mobxjs/mobx-state-tree/discussions/1976#discussioncomment-6610371

A larger PR which could fix this would be to inline the generatePatches function from immer and diff the previous snapshot with the current snapshot in onSnapshot: https://github.com/immerjs/immer/blob/main/src/plugins/patches.ts