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

onAttachedToRootStore triggers after user-land reactions #175

Closed Amareis closed 4 years ago

Amareis commented 4 years ago

Currently this hook fired after parent action has ended. In my app there is some corner case, where other reactions fired after action bounds, but before hook, and this is not really good, because there is chance to access partial-initialised model, which depends on root store.

xaviergonz commented 4 years ago

Could you elaborate (with an example if possible)?

xaviergonz commented 4 years ago

By the way, onAttachedToRootStore was changed to run after the current action is finished because of this: https://github.com/xaviergonz/mobx-keystone/issues/103

Amareis commented 4 years ago

Could you elaborate (with an example if possible)?

Sure, https://codesandbox.io/s/mobx-keystone-lazy-attached-problem-fxfd7 Parent already accessible, but hook is not fired yet. In my project it cause problem, because root store contains some environment data, which is needed by some computed values, which is triggered by external watchers, just as in example. Currently I workaround this with a getParentOfType, but I certainly think that current behavior is confusing.

xaviergonz commented 4 years ago

What if you use const attached = !!getRootStore(this) instead of this.attached? would that solve your use case?

xaviergonz commented 4 years ago

Actually the issue in the example seems to be that it is missing

registerRootStore(r); // you forgot this line

after that change it shows true when logging this.attached.

https://codesandbox.io/s/mobx-keystone-lazy-attached-problem-forked-tnr4h?file=/src/index.tsx:879-949

xaviergonz commented 4 years ago

Can you try "0.45.3-alpha.0"? It is a version that still runs onAttachedToRootStore after all model actions are finished, but before all "user" reactions are run

Amareis commented 4 years ago

Uh-oh, my bad, totally forgot about registerRootStore. But still, there is simple reproduction: https://codesandbox.io/s/mobx-keystone-lazy-attached-problem-forked-o1n7o?file=/src/index.tsx

Amareis commented 4 years ago

And yep, with latest keystone problem solved!

xaviergonz commented 4 years ago

Nice, the fix should be out now in 0.45.3 :)

Amareis commented 4 years ago

Thank you!