MST to mobx-keystone: bonus points, pain points, barriers to groking

[justin-hackin]

Hi folks, I'm in the process of converting an MST project to mobx-keystone. I wanted to spark up some discussion around the process in hopes of crowd-sourcing knowledge on how to make the leap. Perhaps it could help raise awareness about the risks and benefits of adoption.

One of the things I like about this library is that I don't have to use MST Instance for type comparison. It seems instead I can just do myInstance instanceof MyClass. It was a minor leap to go from getParentOfType(this, MyModel) to findParent<MyModel>(this, (parentNode) => parentNode instanceof MyModel).

One of the conceptual barriers I'm experiencing is the relationship between the types export and props. For example:

• in MST, we have types.union. Translating this was not immediately obvious but I think I'm just supposed to do prop<AppleModel | OrangeModel>() 🤞
• types has has properties for primitives e.g. types.number but the docs demonstrates prop<number> so I'm not sure what that's for
• relationship between types.frozen and property definition is unclear, e.g. how do I make translate: prop<RawPoint>(() => ({ x: 0, y: 0 })), into an immutable instance. The data property intermediary accessor makes it dissimilar to MST so I wonder if a rename to types.immutable would help to disambiguate.

I'd appreciate hearing more about others' experiences.

[sisp]

• in MST, we have types.union. Translating this was not immediately obvious but I think I'm just supposed to do prop<AppleModel | OrangeModel>() 🤞

mobx-keystone has two APIs for defining model props: prop (TS types only) and tProp (runtime types similar to MST). See also https://mobx-keystone.js.org/classModels#your-first-class-model. If you want a statically typed union, your approach is correct. If you want to use runtime types, it would be tProp(types.or(types.model(AppleModel), types.model(OrangeModel))).

• types has has properties for primitives e.g. types.number but the docs demonstrates prop<number> so I'm not sure what that's for

prop<number>() uses the props API which only checks TS types. If you want runtime type-checking, you can use tProp(types.number) instead.

[justin-hackin]

@sisp ahhhh, now I understand, I noticed types.or but couldn't figure out how to use it, I glossed over runtime type checking so now it all makes so much more sense now 🙌, thanks, my bad for not RTFM well enough.

[justin-hackin]

A common pattern I used in MST was to create mobx reactions on afterCreate, storing the disposer functions in a volatile array and then executing each disposer on beforeDestroy. I don't see a lifecycle method for teardown in this library though which came as quite a surprise. Am I missing something?

[sisp]

You can use onAttachedToRootStore, although the semantics is slightly different than MST's afterCreate/beforeDestroy – it's similar to afterAttach/beforeDetach and the root model must be a root store. Would this be sufficient for you?

There's also onInit (see life cycle hooks), but it has no counterpart for destruction.

[justin-hackin]

Major +1 that I could just replace mst-persist with mobx-keystone-persist

[justin-hackin]

🙌 yay to runtime type checking only where you need it. Most of my models don't need it and the typescript type generics work nicely.

[evelant]

FYI the runtime type checking is optional even if you use the tProp API and is enabled only in development by default. For this reason I stick to the tProp API since it offers the flexibility to check types when I need it but is otherwise functionally identical to the prop API.

[justin-hackin]

+1 for Path as array of strings, the syntax of JSONPath had me mixed up for a while and with this feature, no more string wrangling e.g. joinJSONPath

[justin-hackin]

One little gotcha: you can't destructure model methods like you could in MST or else this will be undefined within the method. I think this is more just a question of basic JS behaviour though 🤷

[evelant]

mobx-keystone models require $modelType on every model and submodel in a snapshot in order to use fromSnapshot. If $modelType is missing fromSnapshot will fail silently and return a plain observable object instead of a model instance! This leads to unexpected behavior (crashes) at runtime that TS cannot catch.

This makes it difficult to work with MST data. fromSnapshot requires $modelType on each model and sub-model and new MyModel(...) requires manually creating any submodel instances. This means that unless you carefully pre-process all data to make sure that $modelType is added everywhere in the tree there is a model then your model will not rehydrate correctly. That's potentially a lot of boilerplate and a really easy case to trip over.

IMO it would be great if we didn't need $modelType. Is it only there to enable the prop API since that lacks runtime type info? If so, given that tProp is a superset of prop and is functionally equivalent with runtime checking turned off I think this might be another good reason to consider removing the prop API. Having only a single API and avoiding the confusion of $modelType would be quite nice IMO.

[justin-hackin]

I got tripped up about the data property of the model constructors. In MST, the create property can be a snapshot but here we must use fromSnapshot I now see.

I observed that fromSnapshot was not able to resolve the type for a union type property. At first I thought it was because I wasn't using prop<ModelOne | ModelTwo> so I changed to tProp(types.or(types.model(ModelOne), types.model(ModelTwo))), but even when I changed it, I could not do the following (where pattern is the union type)

  this.texture = fromSnapshot<TextureModel>({
    pattern: {
      pathD, sourceFileName, isPositive: DEFAULT_IS_POSITIVE,
    },
    });

and instead had to do

    this.texture = new TextureModel({
      pattern: new PathFaceDecorationPatternModel({
        pathD, sourceFileName, isPositive: DEFAULT_IS_POSITIVE,
      }),
    });

This forced me to add some extra conditional logic to my code. In this case it is fine, but in other places I need to restore a snapshot of a deeply nested tree containing the texture and thus I am under the impression that after many days of working to convert my project from MST, I may have to abandon this translation or use some kind of hacky workaround where I create 2 "maybe" props 😞 I hope I'm wrong, is this a bug or just confusion?

[evelant]

fromSnapshot depends on having $modelType on every nested model in your snapshot. In the case you listed I think the snapshot would have to look like so:

{
  $modelType: "TextureModel",
  pattern: 
    { $modelType: "PathFaceDecorationPatternModel", pathD, sourceFileName, isPositive: DEFAULT_IS_POSITIVE } 
}

If you don't provide $modelType on everything in the snapshot that should be a model then fromSnapshot fails silently and returns a plain observable object instead of a model instance. IMO this is very confusing and error prone behavior. I think it would be really great if we could do away with $modelType but I think it's a hard requirement as long as the prop syntax exists because there's no runtime type information to choose which model to build when only using TS types.

[justin-hackin]

@

fromSnapshot depends on having $modelType on every nested model in your snapshot. In the case you listed I think the snapshot would have to look like so:

{
  $modelType: "TextureModel",
  pattern: 
    { $modelType: "PathFaceDecorationPatternModel", pathD, sourceFileName, isPositive: DEFAULT_IS_POSITIVE } 
}

If you don't provide $modelType on everything in the snapshot that should be a model then fromSnapshot fails silently and returns a plain observable object instead of a model instance. IMO this is very confusing and error prone behavior. I think it would be really great if we could do away with $modelType but I think it's a hard requirement as long as the prop syntax exists because there's no runtime type information to choose which model to build when only using TS types.

@AndrewMorsillo 💡, I had tried to add the pattern model type but didn't think about the texture, thanks! It's too bad the $modelType is not a static property of the class, as hard-coding the model name opens up the possibility of typos. In the instance of the larger tree, that snapshot will have the $modelName so you've allayed some serious concerns 😌

[justin-hackin]

In MST, I was able to return instantiated models from views' getters but this doesn't seem to work unless I first store the instance as a property of the class and at that point it's useless to define a getter and I might as well use a reaction to set a class attribute instead of having a getter and an attribute. This creates a lack of consistency around syntax for computed props in my app and adds an extra step of dealing with the disposer.

[evelant]

I'm not sure I understand what you mean here. What value is there in returning models from views/getters unless they're already part of the tree?

[justin-hackin]

@AndrewMorsillo I might want a model instance to not be a part of the tree and instead just use it for computed properties. Yes, that can be done through pure functions but in some instances I have a model that I want both in the tree in some places and out of the tree in other places (transient and based on other properties). Ya, one can export the $modelType passed to the decorator so not a huge deal.

[justin-hackin]

@AndrewMorsillo the solution with

{
  $modelType: "TextureModel",
  pattern: 
    { $modelType: "PathFaceDecorationPatternModel", pathD, sourceFileName, isPositive: DEFAULT_IS_POSITIVE } 
}

does not work because the model also needs a $modelId for both the TextureModel and the PathFaceDecorationPatternModel 😖 getModelRefId requires an instance so I can't use it in this context to generate one either. Using the constructors works fine though and the deep nested problem I described is not an issue.

[evelant]

Ah when I was doing this my model had an idProp field that was already populated by my old MST models so I only had to add $modelType to the snapshot to use fromSnapshot. Sorry I forgot that part in describing the issue!

When you create a model mobx-keystone will automatically generate a $modelId for you but you can specify your own ids in snapshots or on creation and you can change which field is used with idProp. Still there are a lot of cases in MST where I have nested anonymous "models" that I'd rather not need to give ids or extra type props.

I opened an issue here for discussion about potentially deprecating the prop API so that $modelType and $modelId wouldn't be necessary anymore. #308 IMO prop is redundant since tProp can do the same thing and if I understand it correctly $modelType and $modelId are only necessary because the prop API has no runtime info about the model's type (since it only uses TS types).

[justin-hackin]

EDIT: this is irrelevant. I mistakenly thought prop types coudn't have undefined but this is wrong, it's only arrays that can't store undefined

Minor gotcha: when converting from MST, I had a maybe type that I converted to an | null type, this breaks de-structuring of what was possibly undefined value but now is possibly null.

[justin-hackin]

Bonus: with MST, I was storing reaction disposers in volatile state array to be executed on beforeDestroy. With keystone, disposers can remain scoped to onAttachedToRootStore and the returned teardown function follows a familiar pattern like in React's useEffect

[justin-hackin]

A major pain point for me is the requirement that a model be attached to a root store in order to have lifecycle methods via onAttachedToRootStore. This forces me to move transient models into its parent model properties instead of making them computed properties or @observable attributes (where I can leverage this for their computation or assignment).

[justin-hackin]

Gotcha: default references. Use case: I have some "static" models representing SVG dasharray variants, and a model that references these different styles of dotted lines. I create an array of (unattached) dasharray models and exported them, and in this way I could create a default reference as the first model in the array of dasharray models like prop<Ref<DashArrayModel>>(()=>dashArrayRef(dashArrayModels[0])). Then, in an ancestor node I store the dasharray options for a dropdown like dashArrayOptions: prop<DashArrayModel[]>(()=>dashArrayModels). This caused some peculiar behaviour when attempting to re-initialize an ancestor node this.selectedStore = new this.SelectedModel({}). Even if I perform a detach(this.selectedStore) before doing this, the library thinks I'm attaching those dasharray models to a second parent. This might have to do with the fact that the $modelId of the dasharray models are always the same (generated from constructor data). My solution was to make the reference optionally undefined and then set it in onAttachedToRootStore by climbing up the tree to get the dasharray options stored there. It was confusing why I couldn't "recycle" the models via the return of defaultValue function even after I detached the ancestor node and had to dynamically generate the models with a map function on the constructor data parameters.

[justin-hackin]

The use of context is a real game-changer for me 🙌 Coming from MST, I was looking for a way to get volatile data initialized at creation time. Supplying a context at model creation time is THE reason why I decided to switch over from MST. It has made my models much cleaner in the case of having a DOM node dependency for my model when orchestrating a Three.js scene in a model.

[boldwade]

Here's a quick reminder, that might save someone an hour of wasted time.
tldr: Use arrow methods within your keystone class that are referenced from the view directly.

Coming from MST, you could have a view that invoked an action directly on an MST store.

<Button onClick={myStore.doAction} />

Which has been the pattern in RN for a while now (i.e. not using arrow methods within views for various reasons).

However, I didn't think about this when creating our first keystone model.
All the examples online simply show @modelActions as being defined as named functions. So we happily went along.
We created the view, referencing our keystone @modelAction, but the view was never changing! Why not?!

Because the this that was referenced within the keystone action method was lost and as such was never upddating the property.

So keep that in mind when referencing action methods in your keystone models classes, from directly within your view.