mobxjs / mobx-vue

๐Ÿ‰ Vue bindings for MobX
MIT License
475 stars 22 forks source link

Thoughts about mobx-vue #3

Open nighca opened 6 years ago

nighca commented 6 years ago

Hi there! I've read the docs and came with some ideas:

  1. Mostly what we want is to connect Store instance with Component instance/element, instead of to connect Store instance with Component class/definition. The later may introduce issues when the same Component class/definition used (instantiated) multiple times - different Component instance/element will share the same state. Such behavior suits global-state-situation, while not local-viewModal-state-situation.

  2. Maybe connect is not a good idea. connect defines extra properties on Vue vm in an implicit way. IMO, it will be better not to provide the ability of connect. Offering ability of observer allows developer to organize their state in more explicit way, maybe something like:

    @observer // assuming that we provide `observer` like mobx-react
    @Component()
    export default class App extends Vue {
      viewModel = new ViewModel() // which means `viewModel.age` in template
    }

What do you think?

kuitos commented 6 years ago

It's so great that met someone thought the same!

  1. Usually we use store in a singleton way, which means they are global and that is what redux and vuex does actually. So I think maybe you need a workaround to solve the shared-state problem(also needs in redux and vuex scenario):

    @Connect(store)
    @Component()
    export default class App extends Vue {
      mounted() {
          this.initStore()
      }
    
      destroyed() {
          this.destroyStore()
      }
    }

    That is not my favour too so maybe I should support initializing the state automatically.

    class ViewModel() {}
    @Connect(ViewModel)
    @Component()
    export default class App extends Vue {}

    But the risk of this workaround is that Store and ViewModel are different conceptually, the former lives singleton and the later lives around the component lifecycle, but they are defined the same way(class recipe). For a bindings/connector we shouldn't introduce too much extra concepts imo.

    Or you can try the special solution with DI which mmlpx provided, you can take a glance althought it not documented well yet๐Ÿ˜„.

  2. Yes observer is more semantical than connect in a reactive system. But what I concern is, since we have take over the data and methods definition with a mobx model, why we need to define a extra field to be the member of component instance? which is against the experience of vue users.

    Mobx model should be equivalent to the component instance/vm conceptually, that means we can access the model directly instead of through a vm member.

    Definitely, observer is more semantical for a reactive system, I need to think it over.

xrado commented 6 years ago

What about connecting multiple stores?

kuitos commented 6 years ago

@xrado good question! Actually that is what made me in a dilemma. I think one component should bind with one ViewModel, and the ViewModel could composed by multiple store and itself local states IMO, which mmlpx dose and that matching the standard MVVM philosophy. But now I realized that I should not export any values of my own to users, be unopinioned is the basic principle of mobx and mobx-vue.

@nighca @xrado I will realize a version with observer and custom initialization these days. like:

@Observer
@Component() 
export default class App extends Vue {
  store = store
  vm = new ViewModel()
}

and using in template:

<div>{{store.user}} {{vm.loading}}</div>

Thanks for your feedback!

kuitos commented 6 years ago

Released v2.0.0! @nighca @xrado

nighca commented 6 years ago

@kuitos ๐Ÿ‘ Very impressive productivity.

To me, it is reasonable to provide another API like @Connect(ViewModel) and do class instantiation automatically. As it is more relevant to DI thing, maybe such an API and DI solution for Vue-Mobx can be provided together.

kuitos commented 6 years ago

@nighca Automatic instantiation will introduce several new issues which should not simply construct the ViewModel with new, also applies to di system. Look such a scenario:

class ViewModel {
    id: number;
    @observable users = [];

    constructor(id: number) {
        this.id = id;
    }

    async onInit() {
        this.users = await http.get(`/users/${this.id}`)
    }
}

We need provide the ability to pass parameters to constructor, maybe it looks like this:

Connect(ViewModel, 10)

And things are moving forward. The parameter cames from vue instance, such as a router query string. We need support another usage:

Connect(ViewModel, vm => [vm.$route.query.userId])

In fact that are what mmlpx provided already!๐Ÿ˜‰.

With mmlpx you can initialize states like this:

import { inject } from 'mmlpx';
class App extends Vue {

    @inject()
    vm1: ViewModel;

    @inject(ViewModel)
    vm2;

    @inject(ViewModel, 10)
    vm3;

    @inject(ViewModel, vm => [vm.$routes.userId])
    vm4;
}

DI should be an independent solution for who don't wanna initilize dependencies manually, and applies to any other framework imo. But Connect also required to me, maybe I should provide a more convenient instantiation way for mobx-vue by mmlpx-vue, and documented as best practice๐Ÿ˜„.

nighca commented 6 years ago

@kuitos Yes, what I mean is just what mmlpx does.

But I'm not sure if mmlpx already suits Vue context well. Maybe there is some more work needed, and if so, it is fine to provide these things within mobx-vue (instead of some other package like mmlpx-vue), just like that mobx-react provides Provider/inject. Connect is just another high level function based on Observer.

gregpalaci commented 4 years ago

I'm kinda wondering what direction this project might take with a "new vue" on the horizon. Since vue seems to move away from classes and therefore decorators is there a happy path to using mobx with vue in 2020?