Mobx-state-tree: Feature - types.snapshotProcessor(subModel, { preProcessor, postProcessor})

[mweststrate]

These features leads often to confusion:

• preProcessors are run multiple times, and expensive for all the object allocations
• these hooks don't fit nicely in the rest of the MST model, as they are unlike all other model things defined
  statically
• it kills optimizations like getSnapshot just returning the original snapshot when the node was never touched (see #926 for an example)

What would be the impact? What are typical usage patterns and what are the alternatives? For example: introduce views that use getSnapshot as replacement for postProcessing

[mattruby]

I agree that preProcessors are strange and cause issues. I do most of my data manipulation outside of MST.

My current common use case is renaming ref id fields over to something more ref like:
snapshot.mediaList = snapshot.mediaKeys
I know others are doing the same.
I've tried to make a model that could handle some of that work but we can't compose preProcessors.

[mweststrate]

Alternative, introduce a type that handles this: types.snapshotProcessor(subModel, { preProcessor, postProcessor}), that could probably mix this better in the lifecycle and of building trees and avoid needing static analysis of the subtypes used? cc @k-g-a

[mattiamanzati]

@mweststrate definetely agree for the latter one, would be neat to do the same for any lifecycle event; as those can be applied to any type; not just models

[k-g-a]

I love the dedicated type approach and could even implement it in mid August! For now there is a mess with nested pre/post processors. That’s the reason why I personally prefer modifying API data (used as snapshot) in a tiny custom “layer” outside the MST.

[bourquep]

I've been able to get rid of our snapshot pre/post processing requirements, and it indeed makes things a bit clearer/more predictable.

[ryedin]

We rely very heavily on these functions to support API endpoints that can be fat. We use these hooks for normalizing those related data into their top level stores and replacing the fat child props with references. We also use them for the obvious use cases of handling date conversions (as well as parsing floating point values to/from strings).

Any change here would completely break our architecture, so please make sure the alternative has full parity (please!)

[k-g-a]

We also use them for the obvious use cases of handling date conversions (as well as parsing floating point values to/from strings).

It looks like those cases are better covered by custom types.

[ryedin]

@k-g-a for the conversions I'm with you - although there is no need for us to change our code wrt to that either, unless this proposal lands and we have to. The real issue for us is going to be around the normalization of fat endpoints. We have a really slick system that lets us call backend endpoints and specify (potentially on a per-call basis) how fat/thin the endpoint should behave in terms of pre-populating references (or not). So this is a really nice lever we have that allows us to get pretty granular in terms of optimization and balancing the number of http requests we make with the amount of data we send across the wire. The preProcessSnapshot hooks for each of our generated MST models are then baked with the logic required to detect when the incoming snapshot has pre-populated references and moves those objects to the appropriate stores, replacing that point in the snapshot with a proper types.reference value. This logic is recursive as well, since those pre-populated references could themselves have deeper nested pre-populated data (to n levels).

It took us a bit to work all this out, and we are very happy with it right now. I'm sure we can figure out how to refactor it if we had to (if these hooks are removed), but it's really super nice when you don't have to re-do work you already did because a lib made a breaking change.

That's all... Michael asked about current usages and where things would break if he removes the hooks. This is how it would break for us.

[mweststrate]

@ryedin would you be able to PR a separate test file, or provide a code sandbox that is a PoC implementation of the setup you guys have developed? That would be extremely useful to determine whether it would still be possible to express it elegantly in a potential new system

[xaviergonz]

the use case of versioning types would nicely fit types.snapshotProcessor(subModel, { preProcessor, postProcessor}) since it would only require a pre processor for each type that needs versioning and update it to the latest version as needed upon "loading"

basically this is for loading old models/submodels from old program versions (imagine that you have a document model with a version 1 image node with a property typo that was fixed in v2 of such image node, the pre processor would fix the typo before feeding it to MST. additionally this image node can be exported stand alone,so the upgrade logic cannot reside in the parent document itself, but rather on the image node)

I'm not sure if I'm explaining myself properly...

[ryedin]

@mweststrate The more I look at this, the more I can see it will actually be relatively trivial for us to do this within a different method (a base-class like type that we can compose into our models).

I was overthinking the impact, based mostly on this area being something that we view as "big and hairy" because it took a while to work out. It really won't be that big of deal though.

[ryedin]

@xaviergonz yep I get it. You're talking about doing on-demand schema migrations. Something I've been considering as well (although we've identified a few gotchas in that model). I've been thinking about going one step further though, and storing schema itself as meta MST models, thereby being able to track schematic changes via the patches streams, and using that information to provide a means to hydrate the true MST models and then auto-patch the app-level instances that were created from an earlier version of the schema-tree for that type. Very much unrelated to this thread though. I don't believe pre/post snapshot processor pipelines are necessarily relevant to either vision of migrations though. They may help, but consider how the snapshot processor itself must mutate after each version change.

• Do you continue to escalate it such that it can still handle migrating v1 to v2 even though you are now on v123?
• If you intend to support an on-demand approach to migrations, how does that affect runtime queries that may not find the v1 data because it hasn't been migrated yet? Basically, we've come to the conclusion that in order for your program to be correct the migrations must be applied in total first. You could still say that doing it via preProcessSnapshot would be convenient, but you could name your migration function anything at all... it needs to be run in advance either way to assure that correctness of your program.

[xaviergonz]

@ryedin

Do you continue to escalate it such that it can still handle migrating v1 to v2 even though you are now on v123?

Actually more like it would support from v1, v2 to the latest version (but I guess that's up to the programmer to decide)

Basically what I (currently) do is to use the preprocessors to update v1-v122 snapshots to v123 (latest before it is fed up to MST. Saving is always done with the latest version. Migrations have to be total migrations, that's right.

While it could be done with separate functions I kind of like processors of some kind since it allows models to get their own isolated "migration" code without needing to keep the "structure" of the models in two separate places.

For example, given:

Document (v1)
- many of union of Text (v3) or Image (v5)

Say that we have two use cases:
1) loading a document that has a v2 Image (migrate doc to the latest version)
2) loading an image that is a v2 Image (migrate image to the latest version)

With pre-processors it is as easy as to add a pre-snapshot processor to the Image type and do it's v2->v5 update magic

With functions only I would need at least:
1) a function that updates Image snapshots to the latest version (you need this separate function to update image nodes standalone)
2) a function that takes a Document and traverses the structure exactly as already defined in the document type, updating each Image node using such function (for the document upgrade)

I think what bothers me of resorting to functions is the duplication of the tree structure definition needed for them to work. In the example given it is pretty trivial, but imagine the same on a semi-complex tree structure.