fix: handle errors in destroy (#1106)

Author: eddyerburgh

Diff for: packages/create-instance/patch-create-element.js

@@ -18,7 +18,8 @@ function shouldExtend (component, _Vue) {
 }
 
 function extend (component, _Vue) {
-  const stub = _Vue.extend(component.options)
+  const componentOptions = component.options ? component.options : component
+  const stub = _Vue.extend(componentOptions)
   stub.options.$_vueTestUtils_original = component
   return stub
 }
@@ -92,17 +93,16 @@ export function patchCreateElement (_Vue, stubs, stubAllComponents) {
           return originalCreateElement(el, ...args)
         }
 
+        if (isDynamicComponent(original)) {
+          return originalCreateElement(el, ...args)
+        }
+
         if (
           original.options &&
           original.options.$_vueTestUtils_original
         ) {
           original = original.options.$_vueTestUtils_original
         }
-
-        if (isDynamicComponent(original)) {
-          return originalCreateElement(el, ...args)
-        }
-
         const stub = createStubIfNeeded(stubAllComponents, original, _Vue, el)
 
         if (stub) {

Diff for: packages/server-test-utils/src/renderToString.js

@@ -37,7 +37,7 @@ export default function renderToString (
   // $FlowIgnore
   renderer.renderToString(vm, (err, res) => {
     if (err) {
-      console.log(err)
+      throw err
     }
     renderedString = res
   })

Diff for: packages/test-utils/src/create-local-vue.js

@@ -2,7 +2,6 @@
 
 import Vue from 'vue'
 import cloneDeep from 'lodash/cloneDeep'
-import errorHandler from './error-handler'
 
 function createLocalVue (_Vue: Component = Vue): Component {
   const instance = _Vue.extend()
@@ -27,7 +26,7 @@ function createLocalVue (_Vue: Component = Vue): Component {
   // config is not enumerable
   instance.config = cloneDeep(Vue.config)
 
-  instance.config.errorHandler = errorHandler
+  instance.config.errorHandler = Vue.config.errorHandler
 
   // option merge strategies need to be exposed by reference
   // so that merge strats registered by plugins can work properly

Diff for: packages/test-utils/src/error-handler.js

@@ -0,0 +1,49 @@
+import { warn } from 'shared/util'
+import { findAllInstances } from './find'
+
+function errorHandler (errorOrString, vm) {
+  const error =
+    typeof errorOrString === 'object'
+      ? errorOrString
+      : new Error(errorOrString)
+
+  vm._error = error
+  throw error
+}
+
+export function throwIfInstancesThrew (vm) {
+  const instancesWithError = findAllInstances(vm).filter(
+    _vm => _vm._error
+  )
+
+  if (instancesWithError.length > 0) {
+    throw instancesWithError[0]._error
+  }
+}
+
+let hasWarned = false
+
+// Vue swallows errors thrown by instances, even if the global error handler
+// throws. In order to throw in the test, we add an _error property to an
+// instance when it throws. Then we loop through the instances with
+// throwIfInstancesThrew and throw an error in the test context if any
+// instances threw.
+export function addGlobalErrorHandler (_Vue) {
+  const existingErrorHandler = _Vue.config.errorHandler
+
+  if (existingErrorHandler === errorHandler) {
+    return
+  }
+
+  if (_Vue.config.errorHandler && !hasWarned) {
+    warn(
+      `Global error handler detected (Vue.config.errorHandler). \n` +
+      `Vue Test Utils sets a custom error handler to throw errors ` +
+      `thrown by instances. If you want this behavior in ` +
+      `your tests, you must remove the global error handler.`
+    )
+    hasWarned = true
+  } else {
+    _Vue.config.errorHandler = errorHandler
+  }
+}

Diff for: packages/test-utils/src/error.js

@@ -6,56 +6,52 @@ import Vue from 'vue'
 import VueWrapper from './vue-wrapper'
 import createInstance from 'create-instance'
 import createElement from './create-element'
-import errorHandler from './error-handler'
-import { findAllInstances } from './find'
+import {
+  throwIfInstancesThrew,
+  addGlobalErrorHandler
+} from './error'
 import { mergeOptions } from 'shared/merge-options'
 import config from './config'
 import warnIfNoWindow from './warn-if-no-window'
 import createWrapper from './create-wrapper'
 import createLocalVue from './create-local-vue'
+
 Vue.config.productionTip = false
 Vue.config.devtools = false
 
 export default function mount (
   component: Component,
   options: Options = {}
 ): VueWrapper | Wrapper {
-  const existingErrorHandler = Vue.config.errorHandler
-  Vue.config.errorHandler = errorHandler
-
   warnIfNoWindow()
 
-  const elm = options.attachToDocument ? createElement() : undefined
+  addGlobalErrorHandler(Vue)
+
+  const _Vue = createLocalVue(options.localVue)
 
   const mergedOptions = mergeOptions(options, config)
 
   const parentVm = createInstance(
     component,
     mergedOptions,
-    createLocalVue(options.localVue)
+    _Vue
   )
 
-  const vm = parentVm.$mount(elm).$refs.vm
-
-  const componentsWithError = findAllInstances(vm).filter(
-    c => c._error
-  )
+  const el = options.attachToDocument ? createElement() : undefined
+  const vm = parentVm.$mount(el).$refs.vm
 
-  if (componentsWithError.length > 0) {
-    throw componentsWithError[0]._error
-  }
+  component._Ctor = {}
 
-  Vue.config.errorHandler = existingErrorHandler
+  throwIfInstancesThrew(vm)
 
   const wrapperOptions = {
     attachedToDocument: !!mergedOptions.attachToDocument,
     sync: mergedOptions.sync
   }
+
   const root = vm.$options._isFunctionalContainer
     ? vm._vnode
     : vm
 
-  component._Ctor = []
-
   return createWrapper(root, wrapperOptions)
 }

Diff for: packages/test-utils/src/mount.js

@@ -22,6 +22,7 @@ import { orderWatchers } from './order-watchers'
 import { recursivelySetData } from './recursively-set-data'
 import { matches } from './matches'
 import createDOMEvent from './create-dom-event'
+import { throwIfInstancesThrew } from './error'
 
 export default class Wrapper implements BaseWrapper {
   +vnode: VNode | null;
@@ -152,6 +153,7 @@ export default class Wrapper implements BaseWrapper {
     }
     // $FlowIgnore
     this.vm.$destroy()
+    throwIfInstancesThrew(this.vm)
   }
 
   /**

Diff for: packages/test-utils/src/wrapper.js

@@ -131,10 +131,4 @@ describeWithShallowAndMount('createLocalVue', mountingMethod => {
     }
     expect(installCount).to.equal(2)
   })
-
-  it('has an errorHandler', () => {
-    const localVue = createLocalVue()
-
-    expect(localVue.config.errorHandler).to.be.an('function')
-  })
 })

Diff for: test/specs/create-local-vue.spec.js

@@ -13,7 +13,7 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => {
   const windowSave = window
 
   beforeEach(() => {
-    sinon.stub(console, 'error')
+    sinon.stub(console, 'error').callThrough()
   })
 
   afterEach(() => {
@@ -322,38 +322,6 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => {
     expect(fn).to.throw('Error in mounted')
   })
 
-  itDoNotRunIf(vueVersion < 2.2, 'logs errors once after mount', done => {
-    Vue.config.errorHandler = null
-    const TestComponent = {
-      template: '<div/>',
-      updated: function () {
-        throw new Error('Error in updated')
-      }
-    }
-
-    const wrapper = mount(TestComponent, {
-      sync: false
-    })
-    wrapper.vm.$forceUpdate()
-    setTimeout(() => {
-      vueVersion > 2.1
-        ? expect(console.error).calledTwice
-        : expect(console.error).calledOnce
-      done()
-    })
-  })
-
-  it('restores user error handler after mount', () => {
-    const existingErrorHandler = () => {}
-    Vue.config.errorHandler = existingErrorHandler
-    const TestComponent = {
-      template: '<div/>'
-    }
-    mount(TestComponent)
-    expect(Vue.config.errorHandler).to.equal(existingErrorHandler)
-    Vue.config.errorHandler = null
-  })
-
   it('adds unused propsData as attributes', () => {
     const wrapper = mount(
       ComponentWithProps, {
@@ -415,4 +383,22 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => {
     })
     expect(wrapper.findAll(ChildComponent).length).to.equal(1)
   })
+
+  it('throws if component throws during update', () => {
+    const TestComponent = {
+      template: '<div :p="a" />',
+      updated () {
+        throw new Error('err')
+      },
+      data: () => ({
+        a: 1
+      })
+    }
+    const wrapper = mount(TestComponent)
+    const fn = () => {
+      wrapper.vm.a = 2
+    }
+    expect(fn).to.throw()
+    wrapper.destroy()
+  })
 })

Diff for: test/specs/mount.spec.js

@@ -92,6 +92,7 @@ describeWithMountingMethods('options.localVue', mountingMethod => {
     localVue.prototype.$route = {}
 
     const Extends = {
+      template: '<div />',
       created () {
         console.log(this.$route.params)
       }

Diff for: test/specs/mounting-options/localVue.spec.js

@@ -30,4 +30,18 @@ describeWithShallowAndMount('destroy', mountingMethod => {
     wrapper.destroy()
     expect(wrapper.vm.$el.parentNode).to.be.null
   })
+
+  it('throws if component throws during destroy', () => {
+    const TestComponent = {
+      template: '<div :p="a" />',
+      beforeDestroy () {
+        throw new Error('error')
+      },
+      data: () => ({
+        a: 1
+      })
+    }
+    const wrapper = mountingMethod(TestComponent)
+    expect(() => wrapper.destroy()).to.throw()
+  })
 })