mobx-vue icon indicating copy to clipboard operation
mobx-vue copied to clipboard

Using mobx computed inside vue computed

Open xrado opened this issue 6 years ago • 17 comments

When using mobx computed inside vue computed, vue computed don't get updated.

https://codesandbox.io/s/m5k1yrn2xx click add

xrado avatar Jun 21 '18 12:06 xrado

That is because mobx-vue do not modify/wrap the original vue computed definition, so the mobx observable is not recognized by vue thus mobx changes will not trigger vue data reevaluation.

BTW, we suggest using mobx to manage all your state rather than mixed them in, since you had chosen mobx🙂.

kuitos avatar Jun 21 '18 15:06 kuitos

I agree, it is probably better to move all the state to mobx, but sometimes it comes handy if you can mix mobx shared state with vue local state. Is this vue computed modification/wrapping even possible, as you better know the internals?

xrado avatar Jun 21 '18 19:06 xrado

yes it is possible and not very complicated. But if we convert vue computed into mobx computed as the first step, it means we should take over the whole vue watcher system, means we should not only convert computed but also convert vue props to observable, vue watch to observe, vue method to action, and so on. And, the most important is, all of these conversion are implicit, the syntactic inconsistencies between vue and mobx will make us confused and may introduce more development bugs.

Local states are useful but we could also construct them with mobx. As a workaround you could do like this:

class ViewModel {}

const App = observer({
  data: { vm: new ViewModel() }
})

Thanks for your feedback!😀

kuitos avatar Jun 22 '18 01:06 kuitos

@kuitos How would that example let one use Vue and mobx computed properties together? We are running into this issue and lacking a good solution for making something like...

// A global singleton
import Store from '@/store';

@Observer
@Component
class SomeComponet extends Vue {
    @computed
    get things() {
        return Store.things; // A computed property
    }
}

... into a reactive property is basically preventing us from adopting mobx in our Vue apps.

kevlarr avatar May 18 '19 02:05 kevlarr

At the very least, it would be nice if this inability to use mobx computed inside of Vue computed is called out in the main README as a more obvious limitation or this issue is left in an "opened" state to make it easier to find. If there won't be a fix, this should at least be surfaced more.

kevlarr avatar May 18 '19 03:05 kevlarr

I’ll second this request, and would suggest to be reconsidered. There are plenty of use cases where a computed mobx and computed vue property together would be really useful.

gmoneh avatar May 22 '19 04:05 gmoneh

reopen this issue, try to find a solvable way

kuitos avatar May 22 '19 05:05 kuitos

Thanks for reconsidering this. I don't think this has to extend to all the other Vue mechanisms. There is a clear difference for example between MobX actions and Vue methods. The latter can invoke the former if necessary, no need to interlope them. Similar with watches... that's equivalent to a MobX reaction and no need to merge those... one works for local state and other for the observable. Although some decorator to be able to add "reaction" methods in a Vue component would be another great improvement. The other improvement I can see, which might or might not be related to the computed properties, is the firing of the Updated hook when the component is re-rendered because of changes in the observable state.

gmoneh avatar May 22 '19 14:05 gmoneh

Probably it could be achieved with writing @computed decorator in a way that it registers all accessed properties as dependency (Dep.target.depend()) in Vue dependencies/watchers system.

partyka1 avatar Sep 09 '19 11:09 partyka1

Simplest workaround is to disable cache on that getter:

//decorators.js
import { createDecorator } from 'vue-class-component'

export const NoCache = createDecorator((options, key) => {
  // component options should be passed to the callback
  // and update for the options object affect the component
  options.computed[key].cache = false
})

//MyComp.vue
@Component
class MyComp extends Vue {
  // the computed property will not be cached
  @NoCache
  get random () {
    return Math.random()
  }
}

read more: https://github.com/vuejs/vue-class-component#create-custom-decorators

partyka1 avatar Sep 09 '19 11:09 partyka1

Ive written universal @computed decorator, that can be used on vue classes, and as a mobx computed decorator:

import {createDecorator} from 'vue-class-component';
import Vue from 'vue';
import {computed as mobxComputed, IComputed} from 'mobx';

/**
 * Disables cache on vue-class-component getter
 */
export const NoCache = createDecorator((options: any, key: string) => {
    // component options should be passed to the callback
    // and update for the options object affect the component
    options.computed[key].cache = false;
});

/**
 * Enables to use mobx observables in vue-class-component getters
 * @param target
 * @param key
 * @param index
 */
export const computed: IComputed | any = (target: any, key: any, index: any) => {
    // Using Vue's computed on MobX's computed is not supported: https://github.com/mobxjs/mobx-vue/issues/11
    // adding mobx's @computed --> results in `call stack exceeded`
    // In order to use MobX observables in Vue computed, cache must be disabled on this computed:
    if (target instanceof Vue) {
        return NoCache(target, key, index);
    } else {
        return mobxComputed(target, key, index);
    }
};

partyka1 avatar Sep 09 '19 11:09 partyka1

Any updates on the issue?

jbbch avatar Oct 05 '19 08:10 jbbch

@jbbch for now you can just use the solution i posted. If it's only for aliasing MobX store variables purposes, you won't have any performance drawbacks. If you have some calculations inside that getter you should move it to MobX getter and end result will be the same as you would use cache provided by Vue computed properties

partyka1 avatar Oct 05 '19 11:10 partyka1

@partyka1 Thanks for the proposition! I specifically want to keep MobX store properties and Vue components' representation properties separated. By doing so, I need to have calculations inside my getters and can't use your solution.

jbbch avatar Oct 05 '19 20:10 jbbch

i think its not stressed enough, but assumption of this plugin is when you install it you ditch vue watch system in favour of mobx. you shouldnt mix vue computed and mobx store. thats the assumption and design of mobx-vue, so probably this issue wont be followed by fix from the dev team.

partyka1 avatar Oct 05 '19 20:10 partyka1

i think it should be one of the section in the readme about this problem just so people know what they are getting into :)

partyka1 avatar Oct 05 '19 20:10 partyka1

Also, I think showing warning with short description of the issue in development environment if someone uses @computed on vue-class-component getter. @kuitos what's your take on this?

partyka1 avatar Oct 05 '19 20:10 partyka1