MDSLKTR / pouch-vue

Pouchdb bindings for Vue.js
MIT License
90 stars 17 forks source link

Reactive Pouch Database Property is null in data() but available on root-level this #12

Closed assemblethis closed 5 years ago

assemblethis commented 5 years ago

The following lines of code in the $watch function in index.js can cause some confusion if you go looking in data() for your reactive pouch database on your Vue component:

        if (typeof vm.$data[key] === 'undefined') {
                    vm.$data[key] = null;
                }

If you haven't defined your pouch database property in data() on your Vue component already, like so:

data: {
    todos = [];
}

then the todos reactive property will only really be available on the top-level and you'll only see a null object for todos in data().

So this.todos will work, but not this.$data.todos. What you will find in this.$data.todos is just a null object which was assigned in the $watch function as shown above. Also, in Vue DevTools you'll only see a null object for the todos property and you won't see the top-level todos property, so you may incorrectly assume your pouch database is not working properly.

The workaround is to declare the pouch database property beforehand in data().

I think the above code might be more useful as a string with something like

                  if (typeof vm.$data[key] === 'undefined') {
                      vm.$data[key] = 'The reactive pouch database property you are looking for is not available here at this.$data.' + key + ' but can instead be found at this.' + key + ' due to ' + key + ' not being present in data() at the time of Vue initialization';
                  }
assemblethis commented 5 years ago

I've come up with some code that fixes this, but it's only been tested in a Vue TypeScript web app. The issue was that you can't make a property reactive in the Vue instance's data once the Vue instance has already been initialized. So, I used the beforeCreate lifecycle hook to make the necessary changes:

  // lifecycle hooks for mixin
        beforeCreate: function beforeCreate(){
            var pouchOptions = this.$options.pouch;

            if (!pouchOptions) {
                return;
            }

            let oldFunc = this.$options.data;      

            this.$options.data= function(vm) {
                // get the Vue instance's data object from the constructor
                var plainObject = oldFunc.call(vm, vm);

                // map the pouch databases to an object in the Vue instance's data
                Object.keys(pouchOptions).map(function (key) {

                    var pouchFn = pouchOptions[key];
                    if (typeof pouchFn !== 'function') {
                        pouchFn = function () {
                            return pouchOptions[key];
                        };
                    }

                  if (typeof plainObject[key] === 'undefined') {
                      plainObject[key] = null;
                  }

                });

                // return the Vue instance's data with the additional pouch objects
                // the Vue instance's data will be made reactive before the 'created' lifecycle hook runs
                return plainObject;
            }

          },

If you look at the Vue.js code for initialization (https://github.com/vuejs/vue/blob/dev/src/core/instance/init.js) you'll see a function called initState() in between the beforeCreate and created lifecycle hooks:

    initLifecycle(vm)
    initEvents(vm)
    initRender(vm)
    callHook(vm, 'beforeCreate')
    initInjections(vm) // resolve injections before data/props
    initState(vm)
    initProvide(vm) // resolve provide after data/props
    callHook(vm, 'created')

The data and props are made reactive in the initState() function which is called after the beforeCreate lifecycle hook and before the created lifecycle hook. In initState() the data and props are observed and all their objects are walked and made reactive. So by the time the created lifecycle hook of pouch-vue kicks in, the data property has already been made reactive. According to Vue's documentation, you can't add to data at the root-level after that (https://vuejs.org/v2/guide/reactivity.html):

'Since Vue performs the getter/setter conversion process during instance initialization, a property must be present in the data object in order for Vue to convert it and make it reactive. ... Vue does not allow dynamically adding new root-level reactive properties to an already created instance. '

So in the beforeCreate lifecycle hook, the vm.$options.data function or object needs to be modified.

With TypeScript there is a modified function in vm.$options.data that is inserted in the TypeScript Component class called collectDataFromConstructor() (https://github.com/vuejs/vue-class-component/blob/master/src/component.ts)

// add data hook to collect class properties as Vue instance's data ;(options.mixins || (options.mixins = [])).push({ data (this: Vue) { return collectDataFromConstructor(this, Component) } })

I've just made sure that it works in a TypeScript Vue web app. Can someone try it with a regular JavaScript Vue web app when the Vue instance's data is an object and also when the Vue instance's data is a function?

This is much nicer than the workaround of declaring the pouch database in the Vue instance's data. You can declare the pouch database once in the pouch options and then you're done.

assemblethis commented 5 years ago

I've written some unit tests to test the beforeCreate hook. I had to modify the beforeCreate hook to account for an empty data object.

pouch-vue tests
    √ Test Plugin with Empty Data Function (26ms)
    √ Test Plugin with Empty Data Object (3ms)
    √ Test Plugin with No Data Function Or Object (2ms)
    √ Test Plugin with Existing Data Function (2ms)

I'll do a pull request for this new hook.

sigi commented 5 years ago

I think there is a better solution for this than the beforeCreate() hook, namely to define a data() function in the PouchVue-mixin. The mixed-in data() would have to generate a key for each reactive PouchDB database (from the pouch configuration property).

I have tested this approach and it would work.

This will be less code, easier to understand, and it will use the documented mixin behaviour from Vue.js. The beforeCreate() hook would be completely unnecessary.

I'm planning to make this rewrite myself.

sigi commented 5 years ago

This should work if it's mixed in to the target instance (along with the rest of the API):

data: function() {
  return Object.keys(this.$options.pouch).reduce((a,e) => { a[e] = null; return a }, {})
}

Have not tested it yet, however.

assemblethis commented 5 years ago

Sounds interesting. Run the unit tests in the tests/ folder against your rewrite and see if it works.