xaviergonz / mobx-keystone

A MobX powered state management solution based on data trees with first class support for Typescript, support for snapshots, patches and much more
https://mobx-keystone.js.org
MIT License
554 stars 25 forks source link

model does not detached from root store if it in array #522

Closed ZFail closed 1 year ago

ZFail commented 1 year ago

in this test detach on model A never called:

@model("A")
class A extends Model({
  n: tProp(types.number).withSetter()
}) {
  protected onAttachedToRootStore() {
    console.log("attach", this.n);
    return () => {
      console.log("detach", this.n);
    };
  }
}

@model("B")
class B extends Model({
  as: tProp(types.array(types.model(A))).withSetter()
}) {}

const b = new B({ as: [new A({ n: 1 })] });
registerRootStore(b);
b.setAs([]);
xaviergonz commented 1 year ago

Thanks, fixed in v1.6.2

ZFail commented 1 year ago

Detach now works. But a little different than I expected, if the array contains a model that is present in the new version of the array, then it is still detached and then immediately attached back. sample:

@model("A")
class A extends Model({
  n: idProp.withSetter()
}) {
  protected onAttachedToRootStore() {
    console.log("attach", this.n);
    return () => {
      console.log("detach", this.n);
    };
  }
}

@model("B")
class B extends Model({
  as: tProp(types.array(types.model(A))).withSetter(),
  ar: tProp(types.record(types.model(A))).withSetter()
}) {
}

const b = new B({
  as: [new A({ n: "1" })],
  ar: {
    "3": new A({ n: "3" })
  }
});
registerRootStore(b);
const sn1 = getSnapshot(b);
console.log("b.setAs");
b.setAs([...b.as, new A({ n: "2" })]);
console.log("b.setAr");
b.setAr({
  "3": b.ar["3"],
  "4": new A({ n: "4" })
});
console.log("applySnapshot");
applySnapshot(b, sn1);

output:

attach 1 
attach 3 
b.setAs 
detach 1 
attach 1 
attach 2 
b.setAr 
detach 3 
attach 3 
attach 4 
applySnapshot 
detach 2 
detach 1 
attach 1 
detach 4 

Its by design? If its not a bug, I need find the way, how to avoid unnecessary detach/attaches. With arrays i can use push/splice functions, not as comfotable as reassignment, but you can live with it. But with applySnapshot i don`t know hot to avoid unnecessary detach/attaches.

xaviergonz commented 1 year ago

The first ones are by design (it is just mimicking the operations you are actually doing), but the applySnapshot one was a bug. That one should be fixed now in v1.6.3

ZFail commented 1 year ago

works, thanks!