From e0115fd8b84f3fbe362d159583b140035441d8e1 Mon Sep 17 00:00:00 2001 From: eddyerburgh Date: Sun, 20 Jan 2019 08:06:37 +0000 Subject: [PATCH 1/4] fix: throw error after destroy --- packages/test-utils/src/create-local-vue.js | 3 +- packages/test-utils/src/error-handler.js | 15 ------ packages/test-utils/src/error.js | 51 ++++++++++++++++++++ packages/test-utils/src/mount.js | 33 ++++++------- packages/test-utils/src/wrapper.js | 2 + test/specs/create-local-vue.spec.js | 6 --- test/specs/mount.spec.js | 52 ++++++++------------- test/specs/wrapper/destroy.spec.js | 14 ++++++ 8 files changed, 102 insertions(+), 74 deletions(-) delete mode 100644 packages/test-utils/src/error-handler.js create mode 100644 packages/test-utils/src/error.js diff --git a/packages/test-utils/src/create-local-vue.js b/packages/test-utils/src/create-local-vue.js index 1732997dd..d1611cf39 100644 --- a/packages/test-utils/src/create-local-vue.js +++ b/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 --git a/packages/test-utils/src/error-handler.js b/packages/test-utils/src/error-handler.js deleted file mode 100644 index 0706fb2ef..000000000 --- a/packages/test-utils/src/error-handler.js +++ /dev/null @@ -1,15 +0,0 @@ -// @flow - -export default function errorHandler ( - errorOrString: any, - vm: Component -): void { - const error = - typeof errorOrString === 'object' - ? errorOrString - : new Error(errorOrString) - - vm._error = error - - throw error -} diff --git a/packages/test-utils/src/error.js b/packages/test-utils/src/error.js new file mode 100644 index 000000000..c14fea206 --- /dev/null +++ b/packages/test-utils/src/error.js @@ -0,0 +1,51 @@ +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 during render, 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) { + if (!hasWarned) { + warn( + `Global error handler detected (Vue.config.errorHandler). \n` + + `Vue Test Utils sets a custom error handler to throw errors ` + + `thrown by the instance during render. If you want this behavior in ` + + `your tests, you must remove the global error handler.` + ) + hasWarned = true + } + } else { + _Vue.config.errorHandler = errorHandler + } +} diff --git a/packages/test-utils/src/mount.js b/packages/test-utils/src/mount.js index a9c752e86..aecd638d9 100644 --- a/packages/test-utils/src/mount.js +++ b/packages/test-utils/src/mount.js @@ -6,13 +6,16 @@ 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 @@ -20,42 +23,36 @@ 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) + // delete component._Ctor = {} 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 --git a/packages/test-utils/src/wrapper.js b/packages/test-utils/src/wrapper.js index 36fd4128a..e12fc1c14 100644 --- a/packages/test-utils/src/wrapper.js +++ b/packages/test-utils/src/wrapper.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 --git a/test/specs/create-local-vue.spec.js b/test/specs/create-local-vue.spec.js index d1d88da04..f03bafad6 100644 --- a/test/specs/create-local-vue.spec.js +++ b/test/specs/create-local-vue.spec.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 --git a/test/specs/mount.spec.js b/test/specs/mount.spec.js index cdbdfcef4..77a816d7f 100644 --- a/test/specs/mount.spec.js +++ b/test/specs/mount.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: '
', - 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: '
' - } - 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 a render', () => { + const TestComponent = { + template: '
', + 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 --git a/test/specs/wrapper/destroy.spec.js b/test/specs/wrapper/destroy.spec.js index fd616e965..8daf6b111 100644 --- a/test/specs/wrapper/destroy.spec.js +++ b/test/specs/wrapper/destroy.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: '
', + beforeDestroy () { + throw new Error('error') + }, + data: () => ({ + a: 1 + }) + } + const wrapper = mountingMethod(TestComponent) + expect(() => wrapper.destroy()).to.throw() + }) }) From ecd23af2e3a44715a83885ac335bd14115313bfb Mon Sep 17 00:00:00 2001 From: eddyerburgh Date: Sun, 20 Jan 2019 09:20:57 +0000 Subject: [PATCH 2/4] fix: handle extended components --- packages/create-instance/patch-create-element.js | 12 ++++++------ packages/server-test-utils/src/renderToString.js | 2 +- test/specs/mounting-options/localVue.spec.js | 1 + 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/create-instance/patch-create-element.js b/packages/create-instance/patch-create-element.js index 272d47772..43e0a8dd6 100644 --- a/packages/create-instance/patch-create-element.js +++ b/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 --git a/packages/server-test-utils/src/renderToString.js b/packages/server-test-utils/src/renderToString.js index 15b42267e..cec6b7707 100644 --- a/packages/server-test-utils/src/renderToString.js +++ b/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 --git a/test/specs/mounting-options/localVue.spec.js b/test/specs/mounting-options/localVue.spec.js index 6706bdf72..08e2fe97e 100644 --- a/test/specs/mounting-options/localVue.spec.js +++ b/test/specs/mounting-options/localVue.spec.js @@ -92,6 +92,7 @@ describeWithMountingMethods('options.localVue', mountingMethod => { localVue.prototype.$route = {} const Extends = { + template: '
', created () { console.log(this.$route.params) } From 4523b850937162d25b4356c385c53aa3905704d3 Mon Sep 17 00:00:00 2001 From: eddyerburgh Date: Sun, 20 Jan 2019 09:30:35 +0000 Subject: [PATCH 3/4] refactor: combine conditions --- packages/test-utils/src/error.js | 18 ++++++++---------- packages/test-utils/src/mount.js | 1 - 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/test-utils/src/error.js b/packages/test-utils/src/error.js index c14fea206..b8e9cbee9 100644 --- a/packages/test-utils/src/error.js +++ b/packages/test-utils/src/error.js @@ -35,16 +35,14 @@ export function addGlobalErrorHandler (_Vue) { return } - if (_Vue.config.errorHandler) { - if (!hasWarned) { - warn( - `Global error handler detected (Vue.config.errorHandler). \n` + - `Vue Test Utils sets a custom error handler to throw errors ` + - `thrown by the instance during render. If you want this behavior in ` + - `your tests, you must remove the global error handler.` - ) - hasWarned = true - } + 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 the instance during render. If you want this behavior in ` + + `your tests, you must remove the global error handler.` + ) + hasWarned = true } else { _Vue.config.errorHandler = errorHandler } diff --git a/packages/test-utils/src/mount.js b/packages/test-utils/src/mount.js index aecd638d9..e89b9cb0d 100644 --- a/packages/test-utils/src/mount.js +++ b/packages/test-utils/src/mount.js @@ -31,7 +31,6 @@ export default function mount ( const mergedOptions = mergeOptions(options, config) - // delete component._Ctor = {} const parentVm = createInstance( component, mergedOptions, From a2ff83550e3bd4e66644882828fb90e8288a0f86 Mon Sep 17 00:00:00 2001 From: eddyerburgh Date: Sun, 20 Jan 2019 09:50:42 +0000 Subject: [PATCH 4/4] refactor: improve comment --- packages/test-utils/src/error.js | 4 ++-- test/specs/mount.spec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/test-utils/src/error.js b/packages/test-utils/src/error.js index b8e9cbee9..3873256cb 100644 --- a/packages/test-utils/src/error.js +++ b/packages/test-utils/src/error.js @@ -23,7 +23,7 @@ export function throwIfInstancesThrew (vm) { let hasWarned = false -// Vue swallows errors thrown during render, even if the global error handler +// 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 @@ -39,7 +39,7 @@ export function addGlobalErrorHandler (_Vue) { warn( `Global error handler detected (Vue.config.errorHandler). \n` + `Vue Test Utils sets a custom error handler to throw errors ` + - `thrown by the instance during render. If you want this behavior in ` + + `thrown by instances. If you want this behavior in ` + `your tests, you must remove the global error handler.` ) hasWarned = true diff --git a/test/specs/mount.spec.js b/test/specs/mount.spec.js index 77a816d7f..c1dd1a462 100644 --- a/test/specs/mount.spec.js +++ b/test/specs/mount.spec.js @@ -384,7 +384,7 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => { expect(wrapper.findAll(ChildComponent).length).to.equal(1) }) - it('throws if component throws during a render', () => { + it('throws if component throws during update', () => { const TestComponent = { template: '
', updated () {