feat: throw error if the read-only property is tried to change #749
Conversation
|
|
||
| constructor (wrappers: Array<Wrapper | VueWrapper>) { | ||
| this.wrappers = wrappers || [] |
There was a problem hiding this comment.
wrappers is always an Array object.
| // $FlowIgnore | ||
| Object.defineProperty(this, 'wrappers', { | ||
| get: () => wrappers, | ||
| set: () => {} |
There was a problem hiding this comment.
We should throw an error to tell the users it's read only.
I realize this isn't part of this PR, but I think we should do the same for vnode, and element.
There was a problem hiding this comment.
Thank you for pointing out.
I will change them.
There was a problem hiding this comment.
vnode is undocmumented.
Should vnode is added to document?
There was a problem hiding this comment.
Thanks. No I don't think it needs to be documented
There was a problem hiding this comment.
Thank you for reply.
I think so too.
38elements
changed the title
38elements
changed the title
| @@ -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`) | |||
There was a problem hiding this comment.
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
packages/test-utils/src/wrapper.js
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
@eddyerburgh
There is a problem.
This line did not make error at #748.
Is this line necessary?
There was a problem hiding this comment.
Since vueWrapper.vnode is read-only, I removed this line.
|
I changed them. |
| @@ -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>; | |||
There was a problem hiding this comment.
Cool I didn't know you could specify read only with flow 👍
This makes the read-only property to throw error if the read-only property is tried to change.
This is related to #747.