Skip to content

beforeDestroy not invoked #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fpoliquin opened this issue Apr 20, 2017 · 7 comments · Fixed by vuejs/vue#5514
Closed

beforeDestroy not invoked #87

fpoliquin opened this issue Apr 20, 2017 · 7 comments · Fixed by vuejs/vue#5514

Comments

@fpoliquin
Copy link

Hi,

I just created a sample Vue/Typescript app using Toilal/vue-webpack-template. Everything is working great (compilation, lint, hot reload, etc.).

I defined a small popup component to test different features and I've hit a problem with the beforeDestroy hook. It does not get invoked.

Here is my code:

import Vue from 'vue'
import Component from 'vue-class-component'
import WithRender from './Popup.html?style=./Popup.css'

@WithRender
@Component
export default class Popup extends Vue {

    ...

    mounted() {
        console.info('mounted')
        window.addEventListener('keyup', this.onKeyup)
    }

    beforeDestroy() {
        console.info('beforeDestroy')
        window.removeEventListener('keyup', this.onKeyup)
    }
}

The mounted hook is getting invoked. I've tried the same approach with the standard webpack template and it worked.

I'm using the latest versions :
"vue": "^2.2.6",
"vue-class-component": "^5.0.1",
"vue-router": "^2.4.0"

Thanks for your help.

@ktsn
Copy link
Member

ktsn commented Apr 21, 2017

Sorry, I could not reproduce. Can you provide a complete reproduction?
https://jsfiddle.net/ktsn/j1f5dgp3/2/

@fpoliquin
Copy link
Author

Hi @ktsn,

I tried to reproduce in fiddle but I wasn't able to setup the router.

I see in your example that you are calling vm.$destroy explicitly, do I have to do this if I am using the router ? I expect my components to get destroyed when I navigate, is my assumption right ?

@fpoliquin
Copy link
Author

I've updated the fiddle with the router component but I cannot reproduce. I'll try to see what are the differences between the fiddle and my code.

@fpoliquin
Copy link
Author

I've tracked it down a little further. When I replace the @WithRender with an inline template, the hook gets invoked. So it seems related with vue-template-loader.

Here are the steps to reproduce:

$ npm install -g vue-cli
$ vue init Toilal/vue-webpack-template my-project
$ cd my-project
$ npm install
$ npm run dev
Replace main.ts
Add Popup2.html

main.ts

import Vue from 'vue'
import Router from 'vue-router'
import Component from 'vue-class-component'

import WithRender from './Popup2.html'

@Component({
    template:
    `<div id="app">
        <router-link to="/">/home</router-link>
        <router-link to="/popup1">/popup1</router-link>
        <router-link to="/popup2">/popup2</router-link>
        <router-view></router-view>
    </div>`
})
class App extends Vue {
}

@Component({template: '<p>Home</p>'})
class Home extends Vue {
}

@Component({template: '<p>Popup</p>'})
class Popup1 extends Vue {
    mounted() {
        console.info('mounted popup1')
    }

    beforeDestroy() {
        console.info('beforeDestroy popup1')
    }
}

@WithRender
@Component
class Popup2 extends Vue {
    mounted() {
        console.info('mounted popup2')
    }

    beforeDestroy() {
        console.info('beforeDestroy popup2')
    }
}

Vue.use(Router)

let router = new Router({
  routes: [
    {
      path: '/',
      name: 'Home',
      component: Home
    },
    {
      path: '/popup1',
      component: Popup1
    },
    {
      path: '/popup2',
      component: Popup2
    }
  ]
})

Vue.config.productionTip = false

// tslint:disable-next-line:no-unused-new
new Vue({
  el: '#app',
  router,
  template: '<App/>',
  components: { App }
})

Popup2.html

<p>Popup 2</p>

@ktsn ktsn removed the need repro label Apr 22, 2017
@ktsn
Copy link
Member

ktsn commented Apr 22, 2017

Thank you for the reproduction. I'll looking into it.

@mj-hd
Copy link

mj-hd commented Apr 25, 2017

I fixed this issue! PR is here.

@ktsn
Copy link
Member

ktsn commented Apr 25, 2017

@mjhd-devlion Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants