Skip to content

feat: throw error if the read-only property is tried to change #749

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

Merged
merged 3 commits into from
Jun 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/test-utils/src/vue-wrapper.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @flow

import Wrapper from './wrapper'
import { throwError } from 'shared/util'
import { setWatchersToSync } from './set-watchers-to-sync'
import { orderWatchers } from './order-watchers'

Expand All @@ -11,17 +12,17 @@ export default class VueWrapper extends Wrapper implements BaseWrapper {
// $FlowIgnore : issue with defineProperty
Object.defineProperty(this, 'vnode', {
get: () => vm._vnode,
set: () => {}
set: () => throwError(`VueWrapper.vnode is read-only`)
Copy link
Member

@eddyerburgh eddyerburgh Jun 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a Wrapper or a VueWrapper, I think just go with:

I don't think we need to distinguish between a wrapper and a VueWrapper, I think just go with wrapper:

wrapper.vnode is read-only

})
// $FlowIgnore
Object.defineProperty(this, 'element', {
get: () => vm.$el,
set: () => {}
set: () => throwError(`VueWrapper.element is read-only`)
})
// $FlowIgnore
Object.defineProperty(this, 'vm', {
get: () => vm,
set: () => {}
set: () => throwError(`VueWrapper.vm is read-only`)
})
if (options.sync) {
setWatchersToSync(vm)
Expand Down
4 changes: 2 additions & 2 deletions packages/test-utils/src/wrapper-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ export default class WrapperArray implements BaseWrapper {
// $FlowIgnore
Object.defineProperty(this, 'wrappers', {
get: () => wrappers,
set: () => {}
set: () => throwError(`WrapperArray.wrappers is read-only`)
})
// $FlowIgnore
Object.defineProperty(this, 'length', {
get: () => length,
set: () => {}
set: () => throwError(`WrapperArray.length is read-only`)
})
}

Expand Down
15 changes: 8 additions & 7 deletions packages/test-utils/src/wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { orderWatchers } from './order-watchers'

export default class Wrapper implements BaseWrapper {
+vnode: VNode | null;
_vnode: VNode | null;
+vm: Component | null;
_emitted: { [name: string]: Array<Array<any>> };
_emittedByOrder: Array<{ name: string, args: Array<any> }>;
Expand All @@ -39,27 +40,28 @@ export default class Wrapper implements BaseWrapper {
const element = node instanceof Element ? node : node.elm
// Prevent redefine by VueWrapper
if (this.constructor.name === 'Wrapper') {
this._vnode = vnode
// $FlowIgnore
Object.defineProperty(this, 'vnode', {
get: () => vnode,
set: () => {}
get: () => this._vnode,
set: () => throwError(`Wrapper.vnode is read-only`)
})
// $FlowIgnore
Object.defineProperty(this, 'element', {
get: () => element,
set: () => {}
set: () => throwError(`Wrapper.element is read-only`)
})
// $FlowIgnore
Object.defineProperty(this, 'vm', {
get: () => undefined,
set: () => {}
set: () => throwError(`Wrapper.vm is read-only`)
})
}
const frozenOptions = Object.freeze(options)
// $FlowIgnore
Object.defineProperty(this, 'options', {
get: () => frozenOptions,
set: () => {}
set: () => throwError(`${this.constructor.name}.options is read-only`)
})
if (
this.vnode &&
Expand Down Expand Up @@ -399,7 +401,6 @@ export default class Wrapper implements BaseWrapper {
}

return !!(
this.element &&
this.element.getAttribute &&
this.element.matches(selector)
)
Expand Down Expand Up @@ -667,7 +668,7 @@ export default class Wrapper implements BaseWrapper {
})

// $FlowIgnore : Problem with possibly null this.vm
this.vnode = this.vm._vnode
this._vnode = this.vm._vnode
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddyerburgh
There is a problem.
This line did not make error at #748.
Is this line necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can flowignore it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since vueWrapper.vnode is read-only, I removed this line.

orderWatchers(this.vm || this.vnode.context.$root)
Vue.config.silent = originalConfig
}
Expand Down
7 changes: 4 additions & 3 deletions test/specs/vuewrapper.js → test/specs/vue-wrapper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ describeWithShallowAndMount('VueWrapper', mountingMethod => {
it(`has the ${property} property which is read-only`, () => {
const wrapper = mountingMethod({ template: '<div><p></p></div>' })
expect(wrapper.constructor.name).to.equal('VueWrapper')
const originalProperty = wrapper[property]
wrapper[property] = 'foo'
expect(wrapper[property]).to.equal(originalProperty)
const message = `[vue-test-utils]: VueWrapper.${property} is read-only`
expect(() => { wrapper[property] = 'foo' })
.to.throw()
.with.property('message', message)
})
})
})
10 changes: 10 additions & 0 deletions test/specs/wrapper-array.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ describeWithShallowAndMount('WrapperArray', mountingMethod => {
return wrappers ? new wrapperArray.constructor(wrappers) : wrapperArray
}

['wrappers', 'length'].forEach(property => {
it(`has the ${property} property which is read-only`, () => {
const wrapperArray = getWrapperArray()
const message = `[vue-test-utils]: WrapperArray.${property} is read-only`
expect(() => { wrapperArray[property] = 'foo' })
.to.throw()
.with.property('message', message)
})
})

it('returns class with length equal to length of wrappers passed in constructor', () => {
const wrapperArray = getWrapperArray()
expect(wrapperArray.length).to.equal(3)
Expand Down
7 changes: 4 additions & 3 deletions test/specs/wrapper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ describeWithShallowAndMount('Wrapper', mountingMethod => {
const wrapper = mountingMethod({ template: '<div><p></p></div>' })
.find('p')
expect(wrapper.constructor.name).to.equal('Wrapper')
const originalProperty = wrapper[property]
wrapper[property] = 'foo'
expect(wrapper[property]).to.equal(originalProperty)
const message = `[vue-test-utils]: Wrapper.${property} is read-only`
expect(() => { wrapper[property] = 'foo' })
.to.throw()
.with.property('message', message)
})
})
})
7 changes: 0 additions & 7 deletions test/specs/wrapper/attributes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,4 @@ describeWithShallowAndMount('attributes', mountingMethod => {
const wrapper = mountingMethod(compiled)
expect(wrapper.attributes()).to.eql({})
})

it('returns empty object if wrapper element is null', () => {
const compiled = compileToFunctions('<div />')
const wrapper = mountingMethod(compiled)
wrapper.element = null
expect(wrapper.attributes()).to.eql({})
})
})
6 changes: 0 additions & 6 deletions test/specs/wrapper/is.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ describeWithShallowAndMount('is', mountingMethod => {
expect(wrapper.is('#div')).to.equal(true)
})

it('returns false if wrapper does not contain element', () => {
const wrapper = mountingMethod(ComponentWithChild)
wrapper.element = null
expect(wrapper.is('a')).to.equal(false)
})

it('returns true if root node matches Vue Component selector', () => {
const wrapper = mountingMethod(ComponentWithChild)
const component = wrapper.findAll(Component).at(0)
Expand Down