Propogate thrown errors by configuring error handler and attaching in…

[Austio]

… mount function resolves #147

High level overview

Creates an errorHandler function which we assign to Vue's config.errorHandler. This errorHandler will be called with the error, the vm and additional info from vue. The error handler handles

• thrown Error objects throw(new Error('error'))
• thrown string object throw('error')
• appending info to the error message if it is provided

Background/Justification

Currently vue-test-utils silently swallows errors that are thrown in components. This can lead to false green test suits. Proof of issue is in this repo pull request https://github.com/Austio/vue-test-utils-mocha-webpack-example/pull/1

[eddyerburgh]

Can you remove the nested describe blocks? Everywhere else the describe blocks aren't nested

[Austio]

👍

[eddyerburgh]

Can you export the functions rather than an object that contains the functions

[Austio]

👍

[eddyerburgh]

I think we should export errorHandler and call Vue.config.errorHandler = errorHandler, instead of calling a function. Especially since we set productionTip false just above the function

[Austio]

👍

[eddyerburgh]

Looks good, I'd just like to see a few style changes in the code. errorHandler should be a default export, and we should set Vue.config.errorHandler = errorHandler in mount

[Austio]

@eddyerburgh should be good

I think something to consider is having a TestUtilsVue and import Vue from there when we are implementing things like mount, shallow, etc. It would allow us to centralize things that need to be set on each Vue instance globally (like the error handler)

[eddyerburgh]

When you import/ require in node it caches the instance, so every import to vue is the same vue constructor as every other require.

// some-file.js 
import Vue from 'vue'
Vue.config.errorHandler = true

// some-other-file.js 
import Vue from 'vue'
console.log(Vue.config.errorHandler) // true

So if I understood correctly, there's no need to centralize a Vue singleton, because node requires are always singletons (unless you explicitly delete the cache)

[eddyerburgh]

Ah I just read your code, I see what you mean. We could add it in the createInstance—https://github.com/vuejs/vue-test-utils/blob/dev/src/lib/create-instance.js#L18. This is the same variable for localVue and for a regular Vue

[eddyerburgh]

I'm happy to merge as it is

[Austio]

Cool let's go ahead and merge and if we need the central in future can add then.

[Austio]

@eddyerburgh hey hey, wanted to ping you on this one, good to merge when you are ☮️

[eddyerburgh]

The tests are failing on my machine for some reason. The custom error is thrown, but it doesn't propagate 🤔

I'm going to skip them for the moment and have a look later, but do you have any idea why that might be?

[Austio]

Hey nothing off top of head. Any specific reproduction steps? Should be able to gander here in a few hours.

[Austio]

@eddyerburgh patched #170