Pass env into preProcessSnapshot

[ryedin]

I believe env would always be available (if it exists) in applySnapshot(target, data) and Foo.create(data, env) situations. When applying a snapshot, call getEnv on target before calling preProcessSnapshot, and in create it's either there or it's not (user decides).

If I had env as a second argument to preProcessSnapshot I could, for example, use potentially different transformation rules based on env (or the transformation function itself could be injected, etc).

Another use case is one I'm currently faced with, which is that our server API routes could potentially return fully populated child objects that actually point to another resource's MST store (all top level stores glued together with a single composition root). So in those situations we want to peel those objects off and put them in their respective stores, replacing that path in the current object with a reference (a normalization step, to allow API routes to be as fat or thin as they want to be). So right now I'm doing this in an action before calling applySnapshot or create, accessing the various applicable stores via env. This is logic I would love to generalize in the preProcessSnapshot (i.e. a processor that has non-local side effects)

[ryedin]

tbc, I intend to try to carve out some time to implement this and submit a PR. Interested in thoughts about the feature request though.

[mweststrate]

I think that change would be in principle fine, although there is a small risk; if preProcessnapshot becomes more powerful one might loose track of the fact that the transformation should be idempotent, pure and cheap :-).

The reason for that is that preProcesssnapshot might be called without actually creating an instance, for example in a union(Car, Plane), and passing a plaine shape into the resulting union type; MST will still run possible preProcesssnapshots of Car before determinging the final type.

[rjimenezda]

I second this (for postProcessSnapshot), although I understand the concern that transformations should be "idempotent, pure and cheap", so I wonder if it is a good idea after all.

We are working on a feature that relates some models to a store that describes asynchronous tasks. We're not using types.reference (perhaps that's a better solution), but a simple id instead, and the task store is not part of the rootStore, it is injected through env.

So our idea was to access this store on postProcessSnapshot, and depending on the state of the asynchronous task, the serialization would be slightly different.

I might take a shot at this, just to see how it would behave.

[coolsoftwaretyler]

Hey folks,

This MobX-State-Tree issue is pretty old and inactive, so I'm gonna close it out.

But the good news is we implemented something related in #2116, where we now provide the parent node to the post processor. Not exactly the same intention but it should solve similar problems.

If anyone wants to discuss, please feel free to open a discussion thread, new issue, or better yet, a PR.

Thanks for your time, hope all is well!