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

Organization of models that may need to access parents or other branches of tree #270

Open evelant opened 3 years ago

evelant commented 3 years ago

If I know that a model will always be attached to a root store of a particular type as soon as it's created would it be safe to assume a call to getRootStore will never be undefined?

Without safe guaranteed access to the root store it creates a situation where actions and computed getters need to be pushed up the tree to their highest common parent (a lot of the times the root) which quickly becomes an organization problem.

sisp commented 3 years ago

Could you use contexts to dependency-inject common actions (or perhaps also model instances) from parents to children? I think children should not be concerned with where exactly those actions are coming from but just expect them to be available. This also makes testing easier as you're decoupling models from each other.

evelant commented 3 years ago

I'm really only talking about code organization here -- functionally there are no problems.

These actions aren't common to (not called from) multiple models but they may need to access data from multiple models. They only make sense in the context of a particular model so separating them from that model tends to make the code a lot more disorganized.

Essentially data in different parts of the tree depend on each other but the actions only make sense in the context of particular models so it's more natural to locate the code on them.

For example if I'm modeling a game where I have a PlayerModel and an EquipmentItemModel. These models are in different parts of the tree because while a player may equip equipment there are also a lot of ways equipment data gets used that isn't related to the particular player model.

So in this case there might be an action equipItem on the PlayerModel. That action only makes sense in the context of the PlayerModel but needs to read data from some EquipmentItemModels elsewhere in the tree. This equipItem action could be moved to a model that's the parent of both PlayerModel and EquipmentItemModel but that wouldn't be a natural organization for it. It's an action strictly related to the PlayerModel that just happens to need to read some data from elsewhere in the tree.

That's what I meant by moving them to their "highest common parent" (or root). If they're on the root then they can access all the data without needing getRoot but the code becomes disorganized because these actions really only make sense in the context of (and are only called from) some sub-model.

Functionally this issue doesn't particularly matter, it also applies equally to mobx-state-tree. It works to put actions/views in the highest common ancestor of data they access but it isn't ergonomic or very nice for developer sanity.

xaviergonz commented 3 years ago

If I know that a model will always be attached to a root store of a particular type as soon as it's created would it be safe to assume a call to getRootStore will never be undefined?

If inside an action then yes. If inside a hook then it depends.

That being said it is usually better if you don't make your models depend on their parents/siblings, but I see your point about an organizational problem.

One option would be to make the player action take the EquipmentItemModel as parameter, rather than looking for it in its parents/siblings.

Another option is to use the standalone actions ( https://mobx-keystone.js.org/standardAndStandaloneActions ) and actually pass to it both pieces of data needed to make it work (e.g. the PlayerModel and the EquipmentItemModel), or even the common root of both, yet have it in a "player actions" file.