mobxjs / mobx-vue

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

⚑️ map component props to state automatically #19

Closed snaptopixel closed 6 years ago

codecov-io commented 6 years ago

Codecov Report

Merging #19 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #19   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines          40     55   +15     
  Branches        7     11    +4     
=====================================
+ Hits           40     55   +15
Impacted Files Coverage Ξ”
src/collectData.ts 100% <100%> (ΓΈ) :arrow_up:
src/observer.ts 100% <100%> (ΓΈ) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update 22ed64b...a038414. Read the comment docs.

kuitos commented 6 years ago

Great effort! But what if we had used multi-stores in one component, and those who had the same field?

class Store1 { name: string }
class Store2 { name: string }

export default class Component extends Vue {
  state1 = new Store1()
  state2 = new Store2()
}

IMO, if we wanna keep state synced with props, we should map it via a manual watching, rather than an implicit converting by observer.

import { Watch } from 'vue-property-decorator'
export default class Component extends Vue {
  state = new Store()
  @Watch('prop', { immediate: true })
  onPropChange(newVal) {
    this.state.prop = newVal
  }
}
snaptopixel commented 6 years ago

Thanks @kuitos, you're right that it would update both, but I think that's simply a caveat to be aware of, rather than a deal-breaker. The watch syntax is pretty verbose and undesirable being that you'll have to write it over and over inside your codebase.

Since the recommended usage is to have only one viewmodel per component, I think it's fine...

One objection for adopting mobx-vue in my org is "Why use MobX at all when it adds this overhead" so I'm trying to reduce the barriers to entry and show how it can be elegant 😊

kuitos commented 6 years ago

Since the recommended usage is to have only one viewmodel per component, I think it's fine...

yes we recommended that but not means restrict what you do, the basic principle of mobx & mobx-vue are unopinioned.

One objection for adopting mobx-vue in my org is "Why use MobX at all when it adds this overhead" so I'm trying to reduce the barriers to entry and show how it can be elegant 😊

It's great that mobx-vue is helpful for your org!πŸŽ‰ I understand what u considered about, from my side, might you could create mixin/decorator to sync props with store data, but not provided by mobx-vue by default, and as far as I know that none of mobx bindings had provided such a mechanism yet. And even if you just use vue data to manage your state, there is no way to sync props with data provided by vue as well, except manual operation.

snaptopixel commented 6 years ago

Thanks @kuitos, I'm going to close this and spin off a new solution for MobX + Vue usage. I'll let you know when it's up. I've done some early work and it's quite nice πŸ˜„

kuitos commented 6 years ago

@snaptopixel Very appreciative for your great efforts! And expecting for your experience sharing!πŸ˜„

snaptopixel commented 6 years ago

Sure thing @kuitos, here's what I've got working so far...

Usage: (notice the state property of the component)

class ViewModel {
    @observable firstName = "";
    @observable lastName = "";
    @computed
    get fullName() {
      return `${this.firstName} ${this.lastName}`;
    }
  }

  export default Vue.extend({
    name: 'MyComponent',
    state: new ViewModel(),
    props: {
      firstName: String
    },
    methods: {
      clicked1() {
        this.state.firstName = 'John';
        this.state.lastName = 'Doe';
      },
      clicked2() {
        this.state.firstName = 'Ricky';
        this.state.lastName = 'Bobby';
      }
    }
  });

Mixin:

Vue.mixin({
  destroyed(this: any) {
    this.__reactionDisposer__ && this.__reactionDisposer__();
  },
  created(this: any) {
    const {state, render, props, name} = this.$options;
    if (state) {
      const reaction = new Reaction(`${state.constructor.name}.render()`, () => {
        this.$forceUpdate();
      });
      this.__reactionDisposer__ = reaction.getDisposer();
      this.$options.render = function(...args: any[]) {
        let result;
        reaction.track(() => {
          result = render.apply(this, args);
        });
        return result;
      };
      this.state = state;
      if (props) {
        Object.keys(props).map(prop => {
          if (prop in state) {
            this.$watch(prop, (value: any) => {
              runInAction(`@prop ${prop}`, () => {
                state[prop] = value;
              });
            }, {immediate: true})
          }
        })
      }
    }
  }
});