Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hot-reload poly types #8

Open
ksjogo opened this issue Jul 6, 2018 · 5 comments
Open

hot-reload poly types #8

ksjogo opened this issue Jul 6, 2018 · 5 comments

Comments

@ksjogo
Copy link

ksjogo commented Jul 6, 2018

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?

@jjrv
Copy link
Member

jjrv commented Aug 17, 2018

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.

@ksjogo
Copy link
Author

ksjogo commented Aug 17, 2018

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.

@jjrv
Copy link
Member

jjrv commented Aug 17, 2018

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.

@ksjogo
Copy link
Author

ksjogo commented Aug 17, 2018

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.

@jjrv
Copy link
Member

jjrv commented Oct 9, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants