Skip to content

Commit

Permalink
fix: handle errors in destroy (#1106)
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyerburgh authored Jan 20, 2019
1 parent d2f26e8 commit efab983
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 81 deletions.
12 changes: 6 additions & 6 deletions packages/create-instance/patch-create-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/server-test-utils/src/renderToString.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default function renderToString (
// $FlowIgnore
renderer.renderToString(vm, (err, res) => {
if (err) {
console.log(err)
throw err
}
renderedString = res
})
Expand Down
3 changes: 1 addition & 2 deletions packages/test-utils/src/create-local-vue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
15 changes: 0 additions & 15 deletions packages/test-utils/src/error-handler.js

This file was deleted.

49 changes: 49 additions & 0 deletions packages/test-utils/src/error.js
Original file line number Diff line number Diff line change
@@ -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
}
}
32 changes: 14 additions & 18 deletions packages/test-utils/src/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions packages/test-utils/src/wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -152,6 +153,7 @@ export default class Wrapper implements BaseWrapper {
}
// $FlowIgnore
this.vm.$destroy()
throwIfInstancesThrew(this.vm)
}

/**
Expand Down
6 changes: 0 additions & 6 deletions test/specs/create-local-vue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
52 changes: 19 additions & 33 deletions test/specs/mount.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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()
})
})
1 change: 1 addition & 0 deletions test/specs/mounting-options/localVue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ describeWithMountingMethods('options.localVue', mountingMethod => {
localVue.prototype.$route = {}

const Extends = {
template: '<div />',
created () {
console.log(this.$route.params)
}
Expand Down
14 changes: 14 additions & 0 deletions test/specs/wrapper/destroy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})

0 comments on commit efab983

Please sign in to comment.