Skip to content

Commit

Permalink
feat: warn when operating on destroyed Vue component
Browse files Browse the repository at this point in the history
  • Loading branch information
@xanf
xanf committed Oct 2, 2020
1 parent a821908 commit 45cf4df
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 0 deletions.
56 changes: 56 additions & 0 deletions packages/test-utils/src/wrapper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getCheckedEvent,
isPhantomJS,
nextTick,
warn,
warnDeprecated
} from 'shared/util'
import { isElementVisible } from 'shared/is-visible'
Expand Down Expand Up @@ -82,14 +83,27 @@ export default class Wrapper implements BaseWrapper {
}
}

/**
* Prints warning if component is destroyed
*/
warnIfDestroyed() {
if (!this.exists()) {
warn('Operations on destroyed component are discouraged')
}
}

at(): void {
this.warnIfDestroyed()

throwError('at() must be called on a WrapperArray')
}

/**
* Returns an Object containing all the attribute/value pairs on the element.
*/
attributes(key?: string): { [name: string]: string } | string {
this.warnIfDestroyed()

const attributes = this.element.attributes
const attributeMap = {}
for (let i = 0; i < attributes.length; i++) {
Expand All @@ -104,6 +118,8 @@ export default class Wrapper implements BaseWrapper {
* Returns an Array containing all the classes on the element
*/
classes(className?: string): Array<string> | boolean {
this.warnIfDestroyed()

const classAttribute = this.element.getAttribute('class')
let classes = classAttribute ? classAttribute.split(' ') : []
// Handle converting cssmodules identifiers back to the original class name
Expand Down Expand Up @@ -134,6 +150,9 @@ export default class Wrapper implements BaseWrapper {
'contains',
'Use `wrapper.find`, `wrapper.findComponent` or `wrapper.get` instead'
)

this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'contains')
const nodes = find(this.rootNode, this.vm, selector)
return nodes.length > 0
Expand Down Expand Up @@ -209,6 +228,8 @@ export default class Wrapper implements BaseWrapper {
* matches the provided selector.
*/
get(rawSelector: Selector): Wrapper {
this.warnIfDestroyed()

const found = this.find(rawSelector)
if (found instanceof ErrorWrapper) {
throw new Error(`Unable to find ${rawSelector} within: ${this.html()}`)
Expand All @@ -221,6 +242,8 @@ export default class Wrapper implements BaseWrapper {
* matches the provided selector.
*/
find(rawSelector: Selector): Wrapper | ErrorWrapper {
this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'find')
if (selector.type !== DOM_SELECTOR) {
warnDeprecated(
Expand All @@ -237,6 +260,8 @@ export default class Wrapper implements BaseWrapper {
* matches the provided selector.
*/
findComponent(rawSelector: Selector): Wrapper | ErrorWrapper {
this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'findComponent')
if (!this.vm && !this.isFunctionalComponent) {
throwError(
Expand Down Expand Up @@ -270,6 +295,8 @@ export default class Wrapper implements BaseWrapper {
* the provided selector.
*/
findAll(rawSelector: Selector): WrapperArray {
this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'findAll')
if (selector.type !== DOM_SELECTOR) {
warnDeprecated(
Expand All @@ -285,6 +312,8 @@ export default class Wrapper implements BaseWrapper {
* the provided selector.
*/
findAllComponents(rawSelector: Selector): WrapperArray {
this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'findAll')
if (!this.vm) {
throwError(
Expand Down Expand Up @@ -318,13 +347,17 @@ export default class Wrapper implements BaseWrapper {
* Returns HTML of element as a string
*/
html(): string {
this.warnIfDestroyed()

return pretty(this.element.outerHTML)
}

/**
* Checks if node matches selector or component definition
*/
is(rawSelector: Selector): boolean {
this.warnIfDestroyed()

const selector = getSelector(rawSelector, 'is')

if (selector.type === DOM_SELECTOR) {
Expand All @@ -351,6 +384,8 @@ export default class Wrapper implements BaseWrapper {
'Consider a custom matcher such as those provided in jest-dom: https://github.com/testing-library/jest-dom#tobeempty. ' +
'When using with findComponent, access the DOM element with findComponent(Comp).element'
)
this.warnIfDestroyed()

if (!this.vnode) {
return this.element.innerHTML === ''
}
Expand All @@ -375,6 +410,8 @@ export default class Wrapper implements BaseWrapper {
* Checks if node is visible
*/
isVisible(): boolean {
this.warnIfDestroyed()

return isElementVisible(this.element)
}

Expand All @@ -384,6 +421,8 @@ export default class Wrapper implements BaseWrapper {
*/
isVueInstance(): boolean {
warnDeprecated(`isVueInstance`)
this.warnIfDestroyed()

return !!this.vm
}

Expand All @@ -393,6 +432,7 @@ export default class Wrapper implements BaseWrapper {
*/
name(): string {
warnDeprecated(`name`)
this.warnIfDestroyed()

if (this.vm) {
return (
Expand All @@ -416,6 +456,7 @@ export default class Wrapper implements BaseWrapper {
*/
overview(): void {
warnDeprecated(`overview`)
this.warnIfDestroyed()

if (!this.vm) {
throwError(`wrapper.overview() can only be called on a Vue instance`)
Expand Down Expand Up @@ -495,6 +536,7 @@ export default class Wrapper implements BaseWrapper {
if (!this.vm) {
throwError('wrapper.props() must be called on a Vue instance')
}
this.warnIfDestroyed()

const props = {}
const keys = this.vm && this.vm.$options._propKeys
Expand All @@ -519,6 +561,8 @@ export default class Wrapper implements BaseWrapper {
* @deprecated
*/
setChecked(checked: boolean = true): Promise<*> {
this.warnIfDestroyed()

if (typeof checked !== 'boolean') {
throwError('wrapper.setChecked() must be passed a boolean')
}
Expand Down Expand Up @@ -568,6 +612,8 @@ export default class Wrapper implements BaseWrapper {
* @deprecated
*/
setSelected(): Promise<void> {
this.warnIfDestroyed()

const tagName = this.element.tagName

if (tagName === 'SELECT') {
Expand Down Expand Up @@ -613,6 +659,8 @@ export default class Wrapper implements BaseWrapper {
throwError(`wrapper.setData() can only be called on a Vue instance`)
}

this.warnIfDestroyed()

recursivelySetData(this.vm, this.vm, data)
return nextTick()
}
Expand All @@ -630,6 +678,8 @@ export default class Wrapper implements BaseWrapper {
if (!this.vm) {
throwError(`wrapper.setMethods() can only be called on a Vue instance`)
}
this.warnIfDestroyed()

Object.keys(methods).forEach(key => {
// $FlowIgnore : Problem with possibly null this.vm
this.vm[key] = methods[key]
Expand Down Expand Up @@ -657,6 +707,7 @@ export default class Wrapper implements BaseWrapper {
if (!this.vm) {
throwError(`wrapper.setProps() can only be called on a Vue instance`)
}
this.warnIfDestroyed()

// Save the original "silent" config so that we can directly mutate props
const originalConfig = Vue.config.silent
Expand Down Expand Up @@ -730,6 +781,7 @@ export default class Wrapper implements BaseWrapper {
const tagName = this.element.tagName
// $FlowIgnore
const type = this.attributes().type
this.warnIfDestroyed()

if (tagName === 'OPTION') {
throwError(
Expand Down Expand Up @@ -782,13 +834,17 @@ export default class Wrapper implements BaseWrapper {
* Return text of wrapper element
*/
text(): string {
this.warnIfDestroyed()

return this.element.textContent.trim()
}

/**
* Dispatches a DOM event on wrapper
*/
trigger(type: string, options: Object = {}): Promise<void> {
this.warnIfDestroyed()

if (typeof type !== 'string') {
throwError('wrapper.trigger() must be passed a string')
}
Expand Down
58 changes: 58 additions & 0 deletions test/specs/wrapper/destroy.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
import { describeWithShallowAndMount } from '~resources/utils'
import { config } from 'packages/test-utils/src'

describeWithShallowAndMount('destroy', mountingMethod => {
let originalConsoleError

beforeEach(() => {
config.showDeprecationWarnings = true
originalConsoleError = console.error
console.error = jest.fn()
})

afterEach(() => {
console.error = originalConsoleError
})

it('triggers beforeDestroy ', () => {
const stub = jest.fn()
mountingMethod({
Expand Down Expand Up @@ -61,4 +74,49 @@ describeWithShallowAndMount('destroy', mountingMethod => {
const wrapper = mountingMethod(TestComponent)
expect(() => wrapper.destroy()).toThrow()
})

const StubComponent = { template: '<div><p></p></div>' }

;[
['attributes'],
['classes'],
['isEmpty'],
['isVisible'],
['isVueInstance'],
['name'],
['overview'],
['props'],
['text'],
['html'],
['contains', ['p']],
['get', ['p']],
['find', ['p']],
['findComponent', [StubComponent]],
['findAll', [StubComponent]],
['findAllComponents', [StubComponent]],
['is', [StubComponent]],
['setProps', [{ a: 1 }]],
['setData', [{}]],
['setMethods', [{}]],
['trigger', ['test-event']]
].forEach(([method, args = []]) => {
it(`displays warning when ${method} is called on destroyed wrapper`, () => {
config.showDeprecationWarnings = false
const wrapper = mountingMethod(StubComponent)
wrapper.destroy()
wrapper[method](...args)

expect(console.error).toHaveBeenCalled()
})
})
;['emitted', 'emittedByOrder', 'exists'].forEach(method => {
it(`does not display warning when ${method} is called on destroyed wrapper`, () => {
config.showDeprecationWarnings = false
const wrapper = mountingMethod(StubComponent)
wrapper.destroy()
wrapper[method]()

expect(console.error).not.toHaveBeenCalled()
})
})
})

0 comments on commit 45cf4df

Please sign in to comment.