Use MobX Computed Struct for Computed Array

[BrianHung]

I have an array of cell references that are computed in the views prop of a row. Right now, anytime that a new cell is created (and added to a row), because cells are added to a global cells map, the views prop of all rows is recomputed.

And as a result, all other dependents of row.cells are also recomputed as well.

Is it possible to use comparer.shallow or custom equals function in views to return the same array of cell references if it is the same?

For example, in the codesandbox, adding a cell to the second row should not affect the cells of the first row.

export const Cell = types
  .model("Cell", {
    id: types.optional(types.identifier, () => crypto.randomUUID()),
    row: types.reference(types.late(() => Row)),
    value: types.string
  });

export const Row = types
  .model("Row", {
    id: types.optional(types.identifier, () => crypto.randomUUID())
  })
  .views((self) => ({
    get cells() {
      const root = getRoot(self);
      console.log("cells computed", self.id);
      return values(root.cells).filter((cell) => cell.row === self);
    }
  }))
  .actions((self) => ({
    afterCreate() {
      trace(self, "cells");
    }
  }));

export const Table = types
  .model("Table", {
    cells: types.map(Cell),
    rows: types.map(Row)
  })
  .views((self) => ({
    get values() {
      return values(self.rows).map((row) =>
        row.cells.map((cell) => cell.value)
      );
    }
  }))
  .actions((self) => ({
    addCell(cell) {
      self.cells.put(cell);
    }
  }));

https://codesandbox.io/s/mobx-state-tree-table-mx37kj?file=/models/Row.js

---

Trying to dig into mobx-state-tree source code:

mobx-state-tree/packages/mobx-state-tree/src/types/complex-types/model.ts

Line 493 in 0c07dd4

makeObservable(self, { [key]: computed } as any)

Noticing that getters defined in views are annotated using mobx computed. In the use case above, we are exactly looking for computed.struct instead: https://mobx.js.org/computeds.html#computed-struct

Only use it if changes in the underlying observables can still lead to the same output.

This is because, while the keys of a cells can change, a row may have the same cells array structurally due to filter.

---

The following seems to work by using raw MobX: https://codesandbox.io/s/mobx-state-tree-table-forked-xz0zkc?file=/models/Row.js:151-1123

export const Row = types
  .model("Row", {
    id: types.optional(types.identifier, () => crypto.randomUUID())
  })
  .extend((self) => {
    let cells;
    return {
      views: {
        get cells() {
          return cells?.get() || [];
        }
      },
      actions: {
        afterCreate() {
          const root = getRoot(self);
          cells = computed(
            () => values(root.cells).filter((cell) => cell.row === self),
            {
              equals: comparer.structural
            }
          );
        }
      }
    };
  })

Would like a cleaner solution though as this seems pretty messy.

[coolsoftwaretyler]

Hey @BrianHung - thanks for the detailed walkthrough. Can you say more about what an ideal solution looks like?

Do you want to be able to say, pass some kind of flag to a view to change its comparison behavior? Add some new wrapper or type of view on MST models? Something else?

What would make MST feel more ergonomic for this use case?

[dbstraight]

I'm having trouble understanding from all the above what the current best way to achieve the goal here is. Below is a minimal example of the problem. What is the best way to make the second test also 0?

class Wrapper {
    constructor(public readonly z: number) {}
}

const Model = types.model({
    x: 0,
}).actions(self => ({
    setX(x: number): void {
        self.x = x;
    },
})).views(self => ({
    get y(): number {
        return Math.trunc(self.x);
    },
    get z(): Wrapper {
        return new Wrapper(Math.trunc(self.x));
    },
}));

test("count y reactions", () => {
    const m = Model.create();
    let reactions = 0;
    reaction(() => m.y, () => reactions += 1);
    m.setX(0.1);
    m.setX(0.2);
    m.setX(0.3);
    expect(reactions).toMatchInlineSnapshot(`0`);
});

test("count z reactions", () => {
    const m = Model.create();
    let reactions = 0;
    reaction(() => m.z, () => reactions += 1);
    m.setX(0.1);
    m.setX(0.2);
    m.setX(0.3);
    expect(reactions).toMatchInlineSnapshot(`3`);
});

[coolsoftwaretyler]

@dbstraight - since you're returning an instance of a class, I think volatile is an appropriate place for that. I think this is what Brian was basically doing where he said:

For what its worth, I was able to optimize performance by creating a volatile ObservableGroupMap on the root store, and then using that in my getter instead of directly subscribing to self.cells. The observable group map uses observable arrays so shallow comparison works.

I think this is the simplest answer, although there are a few other options in this response for your consideration.

const Model = types
  .model({
    x: 0,
  })
  .actions((self) => ({
    setX(x: number): void {
      self.x = x;
    },
  }))
  .views((self) => ({
    get y(): number {
      return Math.trunc(self.x);
    },
  }))
  .volatile((self) => ({
    z() {
      return new Wrapper(Math.trunc(self.x));
    },
  }))

If you really want that in a views block for any reason, you can write a view to return the result of the volatile property:

    zView() {
      return self.z;
    },

And just for my own understanding of what does and doesn't get cached in views, it looks like if you return a plain old JavaScript object, the view works as expected:

    zPojo() {
      return {
        z: Math.trunc(self.x),
      };
    },

I put this together in a CodeSandbox:

import { types } from "mobx-state-tree";
import { reaction } from "mobx";

class Wrapper {
  constructor(public readonly z: number) {}
}

const Model = types
  .model({
    x: 0,
  })
  .actions((self) => ({
    setX(x: number): void {
      self.x = x;
    },
  }))
  .views((self) => ({
    get y(): number {
      return Math.trunc(self.x);
    },
  }))
  .volatile((self) => ({
    z() {
      return new Wrapper(Math.trunc(self.x));
    },
  }))
  .views((self) => ({
    zView() {
      return self.z;
    },
    zPojo() {
      return {
        z: Math.trunc(self.x),
      };
    },
  }));

// Count y
const m = Model.create();
let reactions = 0;
reaction(
  () => m.y,
  () => (reactions += 1)
);
m.setX(0.1);
m.setX(0.2);
m.setX(0.3);
console.log("First result", reactions); // "First result 0"

// Count Z
const m2 = Model.create();
let reactions2 = 0;
reaction(
  () => m2.z,
  () => (reactions2 += 1)
);
m2.setX(0.1);
m2.setX(0.2);
m2.setX(0.3);
console.log("Second result", reactions2); // "Second result 0"

// Count Z as a view
const m3 = Model.create();
let reactions3 = 0;
reaction(
  () => m3.zView,
  () => (reactions3 += 1)
);
m3.setX(0.1);
m3.setX(0.2);
m3.setX(0.3);
console.log("Third result", reactions3); // "Third result 0"

// Count Z as a plain JS Object
const m4 = Model.create();
let reactions4 = 0;
reaction(
  () => m4.zPojo,
  () => (reactions4 += 1)
);
m4.setX(0.1);
m4.setX(0.2);
m4.setX(0.3);
console.log("Fourth result", reactions4); // "Fourth result 0"

[dbstraight]

I think because most of the properties are not getters, the sandbox is giving confusing results. m2.z and m3.zView and m4.zPojo are just the functions, not the calculated value, but...

Sandbox for a few different versions reacting to Wrapper value

Putting a computed in a volatile and referencing it in a view seems to work well... but I also can't seem to find any difference with my previous approach of putting the computed in a local variable inside of a view function.

I think I'm missing something in this sandbox to illustrate the performance problem, but I don't know what it is.

[coolsoftwaretyler]

Whoops, you are correct! Too much boilerplate. The POJO example doesn't work, but the zView does.

Your previous example looked a lot like this issue: #818

But I'm actually having trouble reproducing the original performance problem, so either it's been fixed, the original thread was incorrect, or I don't fully understand it myself.

I gotta step away for a bit but I'll keep thinking about this and see if I can get to a satisfactory answer.

[coolsoftwaretyler]

Hey @dbstraight - I brought this over to the maintainers' Discord channel and @thegedge gave me a second set of eyes. This appears to no longer be an issue

In this sandbox, both flavors of views (dereferenced, or chained views) get multiple recalculations because they are outside of an observer context: https://codesandbox.io/p/sandbox/gracious-ellis-gzcgh4?file=%2Fsrc%2Findex.ts%3A26%2C35.

In this sandbox, both flavors of views are appropriately cached inside the observer component: https://codesandbox.io/p/sandbox/compassionate-https-zrdxmj?file=%2Fsrc%2FApp.tsx%3A53%2C31

We've also looked at a couple of other scenarios (like in autorun or actions) and cannot reproduce the problem stated in #818 (which is also called out in our docs).

Unfortunately, I have no idea when this behavior changed. The idea was that in the dereferenced version, views isn't the Proxy that self is, so accessing it "escapes" all the dependency tracking that mobx has set up.

But that statement appears to be incorrect. Apologies for all the churn, my warning to you has been pre-empted by some work here that never got documented appropriately (or perhaps MobX has made a change).

I will make a note to fix the docs