classy-mst icon indicating copy to clipboard operation
classy-mst copied to clipboard

hot-reload poly types

Open ksjogo opened this issue 7 years ago • 5 comments

Hi, is it somehow possible to hot-reload types?

@action
  loadPlugin (name: string) {
    let plugin = pluginLoader(name) as Plugin //will get the new 'mst'ed type from a webpack context
    let previous = this.plugins.get(name)
    let snapshot = previous ? getSnapshot(previous) : null
    let instance = previous ? plugin.create(snapshot) : plugin.create()
    //running fine both times
    console.log(instance)
    try {
      if (previous)
        debugger
      this.plugins.set(name, instance) //will fail for the second time
    } catch (e) {
      debugger
    }
  }
"[mobx-state-tree] Error while converting <TestPlugin@<root>> to `late(function () { return mobx_state_tree_1.types.union.apply(mobx_state_tree_1.types, Union.$typeList); })`:

    value of type TestPlugin: <TestPlugin@<root>> is not assignable to type: `late(function () { return mobx_state_tree_1.types.union.apply(mobx_state_tree_1.types, Union.$typeList); })`, expected an instance of `late(function () { return mobx_state_tree_1.types.union.apply(mobx_state_tree_1.types, Union.$typeList); })` or a snapshot like `(TestPlugin | TestPlugin)` instead. (Note that a snapshot of the provided value is compatible with the targeted type)"

The doc is saying that all types need to be loaded before which I did (thus the first load works). Is there some way to overwrite the relevant part of the existing union with the new definition?

ksjogo avatar Jul 06 '18 15:07 ksjogo

Modifying the union definition on the fly would require poking mobx-state-tree internals past their public API. The issue is simply that for polymorphism, we want to be able to substitute a base class with its subclasses, and the only mechanism supported by MST is to make the base class a union with all subclasses. Once defined, the type cannot be changed.

Do you need polymorphic types? Maybe this could be fixed by supporting a flag to disable them:

const Todo = mst(TodoCode, TodoData, 'Todo', { sealed: true });

I guess in your code it could also be set based on whether the environment is development or production.

Want to test if the solution helps? Change this line in dist/classy-mst.js:

return (polymorphic(Code, Model, name));

to this:

return(Model);

That's what the new flag would do, if implemented.

jjrv avatar Aug 17 '18 12:08 jjrv

I ended up writing that: https://github.com/ksjogo/oxrti/blob/master/src/State/AppState.tsx#L73 Which isn't necessarily nice, but worked for the circumstances. I will try the other one.

ksjogo avatar Aug 17 '18 18:08 ksjogo

Hmm, if you hot reload just a subclass, it will no longer be a valid substitute for the base class type. Reloading all the classes fixes that though.

I'm not sure if / how that AppState code works. You can replace items in the internal $typeList but when an instance of the type is created, this runs:

types.late(() => types.union.apply(types, Union.$typeList));

AFAICT it will only run once per IModelType, and $typeList is not used anywhere else. It's from that function that the list contents continue deep inside mobx-state-tree internals. So once an instance exists, changing the list isn't supposed to do anything because the type won't be re-initialized. On the other hand if hot reloading sets up an entirely new type, its $typeList isn't supposed to contain any left over garbage.

Does your code fix a run-time issue that otherwise shows up? If so, I'll have to re-investigate how the unions work here.

jjrv avatar Aug 17 '18 21:08 jjrv

Afair it was related to the dispatcher function. The reloaded model will append itself on the parent's list, but the dispatcher will stop at the previous version, as it is earlier in the list. At some point it fixed something, not exactly sure what it was anymore though.

ksjogo avatar Aug 17 '18 21:08 ksjogo

The polymorphic type removal flag is now implemented in version 3.5.0.

Still thinking about what else needs to be done for hot reload.

jjrv avatar Oct 09 '18 09:10 jjrv