pulse-framework / pulse

✨ Pulse is a global state and logic framework for reactive Typescript & Javascript applications. Supporting frameworks like VueJS, React and React Native.
https://pulsejs.org
MIT License
252 stars 28 forks source link

Using Pulse with VueJS Computed is not reactive [FIX NEEDED] #4

Closed jamiepine closed 5 years ago

jamiepine commented 5 years ago

Pulse has its own reactivity system. It maps the dependencies for each Vue component and updates the necessary components on change. This means that you can just use $collection.thing directly on the template and it is completely reactive. While this is awesome, there's an issue with computed methods and the way Vue caches them.

Vue is similar to Pulse in that it reads the dependencies and creates a cached value, then when one of the dependencies changes, the computed function / getter is re-run. The same goes for filters in Pulse. However Vue's computed system doesn't recognise the Pulse collections as a reactive dependency, thus computed properties do not re-render. We need to find a way to force Vue to update a specific computed property, shouldn't be hard :)

flozero commented 5 years ago

I think you should use Vue.observable.

Vue.$set will be update in futur version normally so take care of that it can be breaking change

anthrogan commented 5 years ago

Is this not related to how vue see's the collections? As far as vue is concerned on init, the computed area is empty in terms of objects from $collection as they aren't vue native. Then $collection is init afterwards, vue can't detect additions or deletions to a object as the setter/getter is added to things during init. https://vuejs.org/v2/guide/reactivity.html#Change-Detection-Caveats

For example, I have this issue on an app at work, we collect machine data as an Object but only once the app is loaded and on the right page. As a result I have to add an empty Object for each machine we expect data from in the store, so Vue can map a getter/setter to it so it can show its state (which changes lots) once the data is actually collected over REST/MQTT.

Why not try adding the $collection to vuex state so it has its getter/setter init'd it should then see the properties inside of $collection regardless and be able to use them in its own computed methods. It might actually be difficult due to how deep the nested objects go, it might be worth mapping each module separately so as not to get a massive change when a value changes.

I've not had a chance to use pulse yet so this might be way off I'm just spitballing! But I know vue is a bit iffy with things it doesn't see right from the start and reactivity in general is a bit of an 'out there on your own type' topic, especially when you are dealing with changes from outside of vue and trying to make vue react to them

anthrogan commented 5 years ago

I think you should use Vue.observable.

Vue.$set will be update in futur version normally so take care of that it can be breaking change

Or yeah you could wrap collection in Vue.observable its only available in 2.6+

const collectionReactivity = Vue.observable({
  $collection
})
flozero commented 5 years ago

There is no breaking change so easy to update vue version

jamiepine commented 5 years ago

I wrote out a response last night but forgot to send, I lost everything I wrote damn it. Anyway, basically I don't use Vue.$set anymore, currently I have a proxy set up to record access to Pulse properties while the components are creating, so I have an index of which components are dependent on which properties. Then when a property updates, I use $forceUpdate to rerender the specific components. This works perfectly for the $collection properties that are added to the Vue instance and referenced during render, but like I said- not computed.

This is the Vue plugin:

install(Vue) {
    const self = this;
    Vue.mixin({
      beforeCreate() {

       // bind collections to Vue instance
        this.$pulse = self._collections.root;
        for (let collection of Object.keys(self._collections)) {
          if (this.hasOwnProperty('$' + collection))
            return assert(`Error binding collection "${collection}" to Vue instance, "$${collection}" already exists.` );
          this['$' + collection] = self._collections[collection]._public;
        }

        // if this is a Vue instance, 
        if (this.$vnode) {

         // mark this component for analysation
          self._global.analysingComponent = this.$vnode.tag;

         // save reference to this component instance
          self._global.subscribedComponents[this.$vnode.tag] = this;
        }
      },
      beforeUpdate() {
        // re-analyse...
      }
    });
  }

Then the Proxy records property access on the GET trap while the Vue components initialize:

if (depGraph.hasOwnProperty(component) && !depGraph[component].includes(encodedKey))
    depGraph[component].push(encodedKey);
else depGraph[component] = [encodedKey];

Now when properties change the updateSubscribers() method forces the appropriate component(s) to re-render.

for (let component of Object.keys(this._global.componentDependencyGraph)) {
   if (depGraph[component].includes(encodedKey)) {
        this._global.subscribedComponents[component].$forceUpdate();
   }
}

This is what the componentDependencyGraph looks like after render:

screenshot 2019-02-21 at 12 08 56
flozero commented 5 years ago

Hum as vuejs core use forceupdate to update child component async components etc, I think it's the best answear.

 Vue.prototype.$forceUpdate = function () {
    const vm: Component = this
    if (vm._watcher) {
      vm._watcher.update()
    }
  }

He is doing pretty the same get all watcher ands update them with it after all them children are rendered

ps: I see he using update function. Check if exist on components. But it can be just a class object add by some object in cores so maybe not exist

flozero commented 5 years ago

If you need more help please create a branch that i can check your work :)

jamiepine commented 5 years ago

https://github.com/vuejs/vue/issues/7395#issuecomment-355721554 As Evan You points out here, forceUpdate should only re-render the template, not the computed properties, they should have pure reactive references. I want to follow Vue's best practices as best I can, so I wonder how I could make vue see the $collection object as reactive. I still want to use dependency mapping + forceUpdate if possible.

jamiepine commented 5 years ago

I'm starting to understand Vue better now, in order for a computed property to find dependencies, the data in question must be part of the component's data, and just setting this[$componentName] does not mean it's part of the data, Vue can not see it. Still looking into options.

flozero commented 5 years ago

oh ok didnt see it was for computed properties sorry.

Yeap did you see about update method i said ? I saw into their codebase you can make data reactive. There is a function who do that in util from vue

jamiepine commented 5 years ago

I fixed this, see https://github.com/jamiepine/pulse#using-data