Checked for undefined/null on component during destroy

[carsongee]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch for v2.x (or to a previous version branch), not the master branch
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#development-setup
• New/updated tests are included

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
When we run our test suite with relatively new ( >beta13 ) on vue-test-utils we end up with this issue failing a portion of our test suite as it appears somehow that componentInstance is undefined by the time this hook gets called.

I spent about an hour trying to find where to put a test where the destroy hook gets called, but couldn't figure out how to test it properly. I even attempted to find a good spot by having that function throw, but even that wasn't very helpful, so I'd love some help with that.

I'm also happy to help with code there, or whatever, but I could use some guidance since there is a lot of complexity going on around this area of the code, the stack trace we got was:

    TypeError: Cannot read property '_isDestroyed' of undefined

      at destroy (node_modules/vue/dist/vue.runtime.common.js:4174:27)
      at invokeDestroyHook (node_modules/vue/dist/vue.runtime.common.js:5741:59)
      at removeVnodes (node_modules/vue/dist/vue.runtime.common.js:5757:11)
      at VueComponent.patch [as __patch__] (node_modules/vue/dist/vue.runtime.common.js:6170:11)
      at VueComponent.Vue._update (node_modules/vue/dist/vue.runtime.common.js:2668:19)
      at VueComponent.updateComponent (node_modules/vue/dist/vue.runtime.common.js:2786:10)
      at Watcher.get (node_modules/vue/dist/vue.runtime.common.js:3140:25)
      at Watcher.run (node_modules/vue/dist/vue.runtime.common.js:3217:22)
      at Watcher.update (node_modules/vue/dist/vue.runtime.common.js:3205:10)
      at VueComponent.Vue.$forceUpdate (node_modules/vue/dist/vue.runtime.common.js:2689:19)
      at updateChildComponent (node_modules/vue/dist/vue.runtime.common.js:2863:8)
      at prepatch (node_modules/vue/dist/vue.runtime.common.js:4142:5)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5923:7)
      at updateChildren (node_modules/vue/dist/vue.runtime.common.js:5820:9)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5934:29)
      at updateChildren (node_modules/vue/dist/vue.runtime.common.js:5820:9)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5934:29)
      at updateChildren (node_modules/vue/dist/vue.runtime.common.js:5820:9)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5934:29)
      at updateChildren (node_modules/vue/dist/vue.runtime.common.js:5820:9)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5934:29)
      at updateChildren (node_modules/vue/dist/vue.runtime.common.js:5820:9)
      at patchVnode (node_modules/vue/dist/vue.runtime.common.js:5934:29)
      at VueComponent.patch [as __patch__] (node_modules/vue/dist/vue.runtime.common.js:6094:9)
      at VueComponent.Vue._update (node_modules/vue/dist/vue.runtime.common.js:2668:19)
      at VueComponent.updateComponent (node_modules/vue/dist/vue.runtime.common.js:2786:10)
      at Watcher.get (node_modules/vue/dist/vue.runtime.common.js:3140:25)
      at Watcher.run (node_modules/vue/dist/vue.runtime.common.js:3217:22)
      at Watcher.update (node_modules/vue/dist/vue.runtime.common.js:3205:10)
      at Dep.notify (node_modules/vue/dist/vue.runtime.common.js:695:13)
      at Object.reactiveSetter [as isEditing] (node_modules/vue/dist/vue.runtime.common.js:1012:11)
      at VueComponent.proxySetter [as isEditing] (node_modules/vue/dist/vue.runtime.common.js:3298:26)
      at Object.<anonymous> (projects/static/projects/js/views/task-item.spec.js:107:18)
          at Promise (<anonymous>)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

[posva]

Did you make sure it also fails without vue test utils?

[carsongee]

Good point, no it doesn't if I just do the $mount myself. So it sounds like something is going on over there with the VNode lifecycle too, but the fix here seems relevant as well, since it seems harmless to null check before operating on the object. Of course unless it was left out explicitly so that this sort of thing would happen to help track down other bugs lol

[posva]

I think you should share the test that caused the error (a boiled down version is better 😄 ). pinging @eddyerburgh just in case but I will also look at it

[carsongee]

I thought you might want that, here is the most boiled down version I have that still fails:

<template>
  <div class="tasks__task-container col-sm-6">
    <transition name="fade" mode="out-in">
      <div v-if="!isEditing" key="editing">
      </div>
      <task-form
        v-else
        key="editing"
        ></task-form>
    </transition>
  </div>
</template>
<script>
import TaskForm from './task-form.vue'

export default {
  components: { TaskForm, },
  props: {
    task: {
      type: Object,
      default: () => {},
    },
    isCreator: Boolean,
    isParticipant: Boolean,
  },
  data() {
    return {
      isCreating: false,
      isEditing: false,
    }
  },
}
</script>

and the spec (using jest if that helps):

import { shallow } from '@vue/test-utils'
import Vue from 'vue'

import TaskItem from './task-item.vue'

describe('Test suite for Task Item', () => {
  let wrapper
  let vm

  beforeEach(() => {
    wrapper = shallow(TaskItem, {
      propsData: {
        task: {
          id: 55,
        },
        isCreator: false,
        isParticipant: false,
      },
    });
    ({ vm } = wrapper)
  })
  it('shows the tasks form when isEditing, otherwise shows the task', () => {
    vm.isEditing = true
  })
})