Skip to content
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

fix: handle errors in destroy #1106

Merged
merged 4 commits into from
Jan 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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()
})
})