Provide a way to un-sync

[nkoterba]

Long story short: We're migrating our app from Angular 1 to Vue.js. To do that, we're building new "views" in Vue.js and hooking them into our Angular 1 app.

When our user navigates away from the portion of our app where a Vue.js root component was mounted, we destroy the Vue component:

// ($onDestroy is angular component destroy hook)
  $onDestroy() {
    console.log('DESTROYING VUE')
    this.vueApp.$destroy();
  }

Our vueApp is built in an Angular 1 component constructor and successfully mounted to the DOM.

import Vue from 'vue'
import { sync } from 'vuex-router-sync'

import store from '../store'
import router from '../router'

import Main from '../Main.vue'

class BootstrapReportsController {
  constructor () {
    sync(store, router)

    this.vueApp = new Vue({
      router,
      store,
      ...Main
    }).$mount('#reports')
  }

  $onDestroy() {
    console.log('DESTROYING VUE')
    this.vueApp.$destroy();
  }
}

Everything works as expected except when the user navigates away from this part of our app and then returns, a nasty exception gets thrown in vue-router-sync because the original store.watch (https://github.com/vuejs/vuex-router-sync/blob/master/index.js#L18) is never cleaned up when the vueApp.$destroy method is called (I'm assuming b/c vue-router-sync still retains a reference to the store).

Here is the error message:

lib_bundle.js:246653 [Vue warn]: Error in callback for watcher "function () { return getter(this$1.state, this$1.getters); }": "TypeError: Cannot assign to read only property 'path' of object '#<Object>'"

(found in <Root>)
warn @ lib_bundle.js:246653
handleError @ lib_bundle.js:246736
run @ lib_bundle.js:249148
update @ lib_bundle.js:249120
notify @ lib_bundle.js:246952
reactiveSetter @ lib_bundle.js:247174
ROUTE_CHANGED @ defense_all.js:123
wrappedMutationHandler @ lib_bundle.js:254206
commitIterator @ lib_bundle.js:253930

lib_bundle.js:246740 TypeError: Cannot assign to read only property 'path' of object '#<Object>'
    at Object.match (lib_bundle.js:244995)
    at VueRouter.match (lib_bundle.js:246014)
    at AbstractHistory.transitionTo (lib_bundle.js:245466)
    at AbstractHistory.push (lib_bundle.js:245925)
    at VueRouter.push (lib_bundle.js:246082)
    at Vue$3.store.watch.sync (defense_all.js:145)
    at Watcher.run (lib_bundle.js:249146)
    at Watcher.update (lib_bundle.js:249120)
    at Dep.notify (lib_bundle.js:246952)
    at Object.reactiveSetter [as route] (lib_bundle.js:247174)

Since the original store (the first time our component/Vue) is loaded is not destroyed when the user navigates away from our component, the watch triggers again when the user returns to the component and if route.fullPath is different than the previous currentPath variable, then it thinks TimeTraveling is occurring, that the Vuex state has changed, and tries to replace the current route.

So my "issue/bug" is that I assume vuex-router-sync was written without the idea that someone would try to destroy the root Vue component. Is it possible to register a watch handler when scope.watch is called and then export a desync method or some way to allow the developer to "unwatch" the Vuex state.

I found this on Vuex Github: vuejs/vuex#599

It sounds like Vuex won't naturally clean up watch elements and they should be manually unwatched but this plugin doesn't provide a method to do that.

Follow-up:

It looks like this project also hooks into router.afterEach. Again, if there's no way to "unhook" this event registration, won't this plugin leak each time sync(store, router) is called with a new Vuex and VueRouter object?

I recognize we could not destroy our root Vue component when the user navigates away from our Angular 1 component or we could create a global Vuex state for the entire application and never destroy it, but while we "upgrade-in-place" it makes it more efficient, performant, and modular to destroy root Vue components when a user navigates back to a legacy part of our Angular 1 application.

[nkoterba]

Any update/comments/thoughts from the maintainers?

[LinusBorg]

Hey,

sorry for not responding, it seems this issue slipped most people's radar - this repo is generally nor very active since it's a simple lib.

Your issue raises a valid concern. I guess the possibility of an app that uses vuex, vue-router and vuex-router-sync being destryoed and rebuilt wasn't taken into account.

My idea to solve this would be the following:

Both store.watch and router.afterEach() return callback functions that remove th hooks when called. We could rerurn those from the main function so you can use those to remove both the watch and the navigation guard:

const unsync = sync(store, router)

// ... later:
unsync()

// when re-starting the app, call `sync()` again.

[nkoterba]

@LinusBorg Thanks for the reply! Yes, one of the things I like most about the Vue ecosystem is that most plugins/mixins are often 200 lines of code or less.

I contemplated forking this project but figure it would be better to try to get this "fix" added into the official project.

Depending on your workload, I am ok submitting a PR if that would help get this capability into production sooner. Please let me know!

Thanks!

[LinusBorg]

Hey, we are always open to PRs, so go ahead if you can! :)

I guess all it would take is assigning each of the functions returned from this and this call to a variable and then return a function at the end of the main function that would invoke those two when called.

Agreed?

[nkoterba]

@LinusBorg Yep agreed! Perhaps add a unit test or test case as well?

I can do the PR later Today and submit the request.

[LinusBorg]

A Test case would of course be required, as would be a mention in the README of this repo.

[nkoterba]

@LinusBorg Sorry for the delay. Added a pull request. Regarding testing, I wasn't sure exactly how best to test the registration/de-registration of the store, router, and moduleName. I accessed some internal/private Vuex store properties (but these could obviously change as Vuex gets updated and break the tests). If you have a better or more clever way, I'm open to suggestions (or just remove those assertions).