introduce lazy loading of types [WIP]

[mattiamanzati]

This PR introduces lazy loading.
This is done by introducing in MST a new type composer named "lazy".
This type composer has the following parameters:

1. name? The name to give at the resulting type
2. shouldLoadType This is a function that given the parent value owning this node should return true when the type loading should be triggered. RFC: Should we instead pass 2 parameters, the current value and the parent? Not always the parent value could be resolved from the current value, for e.g. frozen does not store the parent, so getRoot() is impossible on that value.
3. loadType This is a function that returns a promise that resolves the value. Its handy to perform tricks like import("./module")
4. replacementType? = frozen This is the type to use on the state tree until the loading is triggered and the promise resolves. By default it is frozen, but the user can provide, for e.g., a model with some frozen fields that will be resolved later.

This types can be used only as child of ComplexTypes (model, array, map) as it will trigger behind the scene a replace of the value of the corresponding property. For this reason, performing actions like onPatch(this.lazy, fn) on afterAttach may break because the node will be destroyed and replaced later.

cc @arackaf

[coveralls]

Coverage decreased (-0.2%) to 95.988% when pulling 385598b on feature/lazy-loading into 4656e14 on master.

[coveralls]

Coverage decreased (-0.1%) to 96.041% when pulling 63080c8 on feature/lazy-loading into 4656e14 on master.

[mweststrate]

Hey @mattiamanzati

I like the idea, not entirely sure about the entire api yet:

First of all, I would skip the replacementType

Secondly, which I am not sure about, is that it might be more convenient to have a method that performs the loading, rather then a reaction. Although it makes it arguably a bit more imperative.

What about something along these lines:

types.lazy(loader:() => Promise<TargetType>)

and then make the type of lazy this: union(UnloadedLazyType, TargetType)

where UnloadedType is a model type defined by MST with the following signature:

const UnloadedType = types.model({
   isPending: literal(true), // always true, but for convenience, see example below
   state: enum("unloaded", "loading", "error") // no loaded state, because this node gets replaced!
}, {
   load() {
      // kicks of the promise if not already done so
      // once promise is resolved, instance is replaced like in the PR
   }
}

Example from test

        const Model = types.model(
            "Model",
            {
                lazy: types.lazy(
                    () => Promise.resolve(SomethingUseful)
                )
            },
            {
                setLoaded() {
                    if (this.lazy.isPending)
                       this.lazy.load()
                    // else, already loading
                }
            }
        )

I think this api is smaller, and feels more composed, as it is just a union (although same limitiations apply; only as direct descendant of a structure). I doubt about the isPending utility, maybe that should be just expressed as if (UnloadedLazyType.is(this.lazy)) this.lazy.load() but that feels cumbersome to remember...

[mattiamanzati]

Yeah, loading the type through a method could be a smarter idea, even if loading should be triggered after some method being called. The user could use the reaction-like syntax by setting up a reaction in the afterAttach of the parent.
Uhm, would'nt you store the state somewhere until the type is loaded? :)
The reason about my idea of using a replacement type was to not modify the snapshot provided if the type isnt loaded yet, using an union-like syntax as you provided would end up omitting the pending state and return {state: "pending"} instead :)
About the isPending maybe we should make lazy a wrapping type acting like fromPromise from mobx-utils?
So that the user will do things like item.case({ loaded: () => etc... }) ?

[mweststrate]

Uhm, would'nt you store the state somewhere until the type is loaded? :)
The reason about my idea of using a replacement type was to not modify the snapshot provided if the type isnt loaded yet, using an union-like syntax as you provided would end up omitting the pending state and return {state: "pending"} instead :)

Good points. I need to think harder :-P.

[mweststrate]

Ok, I think the replacement type should be frozen indeed, and not overrideable :) Didn't verify it against the actual implementation, but I think api signature should be like:

interface IUnloadedType {
  isPending: true,
  state: "pending" | "loading" | "error"
  load() 
}

types.lazy<S, T>(name?, (parent, pendingSnapshot) => Promise<IType<S, T>): IType<S, T | IUnloadedType>

This means that getSnapshot / applySnapshot produces identical results for both an loaded and unloaded types indeed, and that the state of the type loading is not reflected in the snapshot. Until type is loaded, it will behave in other aspect as a frozen type. (Why am I suddenly reminded to time-to-interactive 🤔 )

If you put isPending, state or load in your snapshot, you might get unexpected results, so let's warn about that at least

I am not sure if IType<S, T | IUnloadedType causes compile errors with the signature export interface IComplexType<S, T> extends IType<S, T & ISnapshottable<S> & IStateTreeNode> {}, but I think it should be ok.

Typescript might become quite annoying as strict null checks will probably trigger on accessing any attribute in a lazy type, as it doesn't know whether it is loaded already or not.

[mattiamanzati]

Uhm, that's also a fair point.
Which are the reason preventing us implementing a wrapping type instead of a replacement type? I mean, which are the advantages on that approach instead of something along this lines:

interface IUnloadedType {
   status: "pending" | "loading" | "error"
   error?: <frozen>
   state: <frozen>
}

interface ILoadedType<T> {
   status: "loaded"
   state: T
}

types.lazy<S, T>(name?, (parent, pendingSnapshot) => Promise<IType<S, T>): IType<{status: "pending" | "loading" | "error" | "loaded", state: S | <frozen>}>, IUnloadedType | ILoadedType<T>>

This allows the TypeChecker to easily dispatch between the loaded or unloaded case, by simply asserting the status string. This also enforces the developer to check for that, so warnings by TypeScript will be LGTM. It will also respect in time travel errors on render due to the state of type loading (e.g. accessing a computed property if the type has not loaded yet). On the other hand, the user can't simply drop it in as replacement type.

[mweststrate]

Yeah that might be simpler to work with and have less potential issues. On the other hand api / usage wise it is indeed quite ugly as it is untransparent to the consumer side of things.

Also it might be quite how to operate with lazy types in general. If you have e.g. types.array(SomeLazyType), should the user call load() on every instance first etc? (the answer is obviously no, but that might be unclear.

I also don't have any clear idea about lazy loading value objects yet.

I think it might be best to put this PR on hold, until we figured a really nice pattern or an urgent real life need? I think this needs some more time to be thought trough :)

[mattiamanzati]

1. Yeah, I'd prefer that too, would avoid lots of issues :)

2. And that's why I originally wrote a reaction predicate as load trigger, it's on the declared type, so the user understand that refers to the type, and avoid putting messy additional things over the instance :)

3. Uhm, what do you mean by value objects?

4. Sure, was initially opened for that, only to explore it API wise and implementation wise :)

[mweststrate]

For 3: having maybe a HigherOrderType that loads data somehow automatically when needed, like fromResource and lazyObsevable in MobxUtils Op do 20 jul. 2017 om 21:55 schreef Mattia Manzati <notifications@github.com

…

: 1. Yeah, I'd prefer that too, would avoid lots of issues :) 2. And that's why I originally wrote a reaction predicate as load trigger, it's on the declared type, so the user understand that refers to the type, and avoid putting messy additional things over the instance :) 3. Uhm, what do you mean by value objects? 4. Sure, was initially opened for that, only to explore it API wise and implementation wise :) — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#247 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhAk6vbHcLw8-zrOMbvA1xkBZLzDXks5sP7DMgaJpZM4OVnjy> .

[leesiongchan]

This seem like a very cool feature, do we have any new updates on this feature?

[mweststrate]

I think we can now use custom references to achieve the same right? Or would a separate type be nicer? Kinda out of the discussion :). Sry!

[pkrogel-kws]

Any updates on this? I'm interested in switching over from Redux, but we need to be able to match functionality of lazy loading our reducers.

[mweststrate]

I think for now we can close this issue? I don't think the demand is that high atm, and alternatively one can keep multiple tree definitions separated and store them in volatiles of a root component.

[arackaf]

I hope you'll reconsider. Lack of code splitting akin to Redux's replaceReducer is the primary reason I haven't looked at MST. I of course don't expect you to tailor development to what I happen to want, but I think this is an essential feature. I don't think many apps in practice will be shipping their entire state definition up front.

[lishine]

Reading the above solutions with references and volatile - I didn't quite understood it.
Is my problem bellow related to this issue ?

export const Store = types.model('Store', {
	app: types.optional(AppStore, {}),
	nav: types.optional(NavStore, {}),
	form: types.optional(FormStore, {}),
})

Here I want FormStore instantiated only after login.
With Mobx I did new and on logout I did delete.

class Store {
	constructor() {
		console.log('mounted store');
		this.app = new AppStore(this);
		this.nav = new NavStore(this);
		
		console.log('this.app.auth', this.app.auth);
		reaction(() => this.app.auth, async (auth) => {
			if (auth) {
				this.form = new FormStore(this);
			} else {
				delete this.form;
			}
		});
	}
}

What should I do ?
Thanks

[lishine]

I think in my case I should do:

export const Store = types
	.model({
		nav: types.maybe(NavStore),
	})
	.named('Store')
	.actions(self => ({
		afterCreate() {
			self.nav = NavStore.create({})
		},
	}))

[ThaJay]

Is there any way to accomplish dynamic types with observable properties at the moment?
#326 (comment)

[fwouts]

@mweststrate By using multiple trees stored as volatiles in a root component, wouldn't that prevent them from having references from one tree to another? (I tried it, and can confirm this doesn't seem to work)

I'd be keen to contribute to help make this feature happen as I think dynamic loading is pretty important in a healthy codebase nowadays. What would convince you that it's worth adding to MST?