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
555 stars 25 forks source link

Invalid snapshot when model prop changed in onAttachedToRootStore #74

Closed sisp closed 4 years ago

sisp commented 4 years ago

I think a test case that reproduces this problem explains it best:

import {
  getSnapshot,
  Model,
  model,
  modelAction,
  prop,
  registerRootStore,
} from 'mobx-keystone';

test('snapshot', () => {
  @model('#74/A')
  class A extends Model({
    value: prop<number>(),
  }) {
    public onAttachedToRootStore() {
      this.value = 1; // but, e.g, this works: setTimeout(() => { this.value = 1; })
    }
  }

  @model('#74/Store')
  class Store extends Model({
    all: prop<A[]>(() => []),
  }) {
    @modelAction
    public add(a: A): void {
      this.all.push(a);
    }
  }

  const store = registerRootStore(new Store({}));
  store.add(new A({ value: 0 }));

  expect(getSnapshot(store).all).toHaveLength(store.all.length);
});

The state is correct, but the snapshot is wrong. Unfortunately, I haven't been able to determine the cause of this error.

xaviergonz commented 4 years ago

Thanks! should be fixed in 0.28.3, please reopen if not. The fix adds a slight change in behaviour though. onAttachedToRootStore and its disposer will now run after the current action / runUnprotected block is finsihed (it will still run synchronously though).

sisp commented 4 years ago

Works perfectly, thanks a lot!