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

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

Merged
merged 3 commits into from
Jun 24, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/api/wrapper-array/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ A `WrapperArray` is an object that contains an array of [`Wrappers`](../wrapper/

## Properties

### `wrappers`
### `wrappers`

`array`: the `Wrappers` contained in the `WrapperArray`
`array` (read-only): the `Wrappers` contained in the `WrapperArray`

### `length`
### `length`

`number`: the number of `Wrappers` contained in the `WrapperArray`
`number` (read-only): the number of `Wrappers` contained in the `WrapperArray`

## Methods

Expand Down
4 changes: 2 additions & 2 deletions docs/api/wrapper/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ A `Wrapper` is an object that contains a mounted component or vnode and methods

`HTMLElement` (read-only): the root DOM node of the wrapper

### `options`
### `options`

#### `options.attachedToDocument`

`Boolean` (read-only): True if `attachedToDocument` in mounting options was `true`

#### `options.sync`
#### `options.sync`

`Boolean` (read-only): True if `sync` in mounting options was not `false`

Expand Down
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('wrapper.vnode is read-only')
})
// $FlowIgnore
Object.defineProperty(this, 'element', {
get: () => vm.$el,
set: () => {}
set: () => throwError('wrapper.element is read-only')
})
// $FlowIgnore
Object.defineProperty(this, 'vm', {
get: () => vm,
set: () => {}
set: () => throwError('wrapper.vm is read-only')
})
if (options.sync) {
setWatchersToSync(vm)
Expand Down
17 changes: 13 additions & 4 deletions packages/test-utils/src/wrapper-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@ import type VueWrapper from './vue-wrapper'
import { throwError, warn } from 'shared/util'

export default class WrapperArray implements BaseWrapper {
wrappers: Array<Wrapper | VueWrapper>;
length: number;
+wrappers: Array<Wrapper | VueWrapper>;
Copy link
Member

Choose a reason for hiding this comment

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

Cool I didn't know you could specify read only with flow 👍

+length: number;

constructor (wrappers: Array<Wrapper | VueWrapper>) {
this.wrappers = wrappers || []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrappers is always an Array object.

this.length = this.wrappers.length
const length = wrappers.length
// $FlowIgnore
Object.defineProperty(this, 'wrappers', {
get: () => wrappers,
set: () => throwError('wrapperArray.wrappers is read-only')
})
// $FlowIgnore
Object.defineProperty(this, 'length', {
get: () => length,
set: () => throwError('wrapperArray.length is read-only')
})
}

at (index: number): Wrapper | VueWrapper {
Expand Down
12 changes: 5 additions & 7 deletions packages/test-utils/src/wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,24 @@ export default class Wrapper implements BaseWrapper {
// $FlowIgnore
Object.defineProperty(this, 'vnode', {
get: () => vnode,
set: () => {}
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('wrapper.options is read-only')
})
if (
this.vnode &&
Expand Down Expand Up @@ -399,7 +399,6 @@ export default class Wrapper implements BaseWrapper {
}

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

// $FlowIgnore : Problem with possibly null this.vm
this.vnode = this.vm._vnode
orderWatchers(this.vm || this.vnode.context.$root)
Vue.config.silent = originalConfig
}
Expand Down Expand Up @@ -814,7 +812,7 @@ export default class Wrapper implements BaseWrapper {
*/
destroy () {
if (!this.isVueInstance()) {
throwError(`wrapper.destroy() can only be called on a Vue ` + `instance`)
throwError(`wrapper.destroy() can only be called on a Vue instance`)
}

if (this.element.parentNode) {
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]: wrapper.${property} is read-only`
expect(() => { wrapper[property] = 'foo' })
.to.throw()
.with.property('message', message)
})
})
})
22 changes: 13 additions & 9 deletions test/specs/wrapper-array.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@ describeWithShallowAndMount('WrapperArray', mountingMethod => {
const wrapper = mountingMethod(compiled)
const wrapperArray = wrapper.findAll('p')
expect(wrapperArray.constructor.name).to.equal('WrapperArray')
if (wrappers) {
wrapperArray.wrappers = wrappers
wrapperArray.length = wrappers.length
}
return wrapperArray
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 Expand Up @@ -67,8 +73,7 @@ describeWithShallowAndMount('WrapperArray', mountingMethod => {
if (method === 'at') {
return
}
const wrapperArray = getWrapperArray()
wrapperArray.wrappers = []
const wrapperArray = getWrapperArray([])
const message = `[vue-test-utils]: ${method} cannot be called on 0 items`
expect(() => wrapperArray[method]())
.to.throw()
Expand Down Expand Up @@ -99,8 +104,7 @@ describeWithShallowAndMount('WrapperArray', mountingMethod => {
) {
return
}
const wrapperArray = getWrapperArray()
wrapperArray.wrappers = [1, 2, 3]
const wrapperArray = getWrapperArray([1, 2, 3])
const message = `[vue-test-utils]: ${method} must be called on a single wrapper, use at(i) to access a wrapper`
expect(() => wrapperArray[method]())
.to.throw()
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