refactor: Update setData method (address #563) #565
Conversation
shortdiv
force-pushed
the
dev
branch
2 times, most recently
from
c3874ae to
5b24600
Compare
|
Ah I'm so sorry, I wasn't clear in my comment! I think we should keep the signature the same as it is currently. Instead, we could update the function to do a deep merge: const Cmp = {
data: () => ({
anObject: {
propA: 'a',
propB: 'b'
}
})
}
const wrapper = mount(Cmp)
wrapper.setData({
anObject: {
propB: 'c'
}
})
wrapper.vm.anObject.propA // 'a'
wrapper.vm.anObject.propB // 'c' |
|
Ah gotcha, will update accordingly! |
shortdiv
force-pushed
the
dev
branch
2 times, most recently
from
59a26a0 to
37924d7
Compare
packages/test-utils/src/wrapper.js
Outdated
| } | ||
|
|
||
| Object.keys(data).forEach((key) => { | ||
| // $FlowIgnore : Problem with possibly null this.vm |
There was a problem hiding this comment.
I think that this line should be removed.
packages/test-utils/src/wrapper.js
Outdated
| if (typeof data[key] === typeof {} && data[key] !== null) { | ||
| Object.keys(data[key]).forEach((key2) => { | ||
| var newObj = Object.assign({}, data[key], data[key][key2]) | ||
| this.vm.$set(this.vm, [key], newObj) |
There was a problem hiding this comment.
I think it is necessary to add the following line before this line.
// $FlowIgnore : Problem with possibly null this.vm
| this.vm.$set(this.vm, [key], newObj) | ||
| }) | ||
| } else { | ||
| this.vm.$set(this.vm, [key], data[key]) |
There was a problem hiding this comment.
I think it is necessary to add the following line before this line.
// $FlowIgnore : Problem with possibly null this.vm
packages/test-utils/src/wrapper.js
Outdated
| this.vm.$set(this.vm, [key], data[key]) | ||
| if (typeof data[key] === typeof {} && data[key] !== null) { | ||
| Object.keys(data[key]).forEach((key2) => { | ||
| var newObj = Object.assign({}, data[key], data[key][key2]) |
There was a problem hiding this comment.
I think spread syntax is frequently used in this repository.
I think it is better to change var to const.
const newObj = { ...data[key], ...data[key][key2]}
packages/test-utils/src/wrapper.js
Outdated
| Object.keys(data).forEach((key) => { | ||
| // $FlowIgnore : Problem with possibly null this.vm | ||
| this.vm.$set(this.vm, [key], data[key]) | ||
| if (typeof data[key] === typeof {} && data[key] !== null) { |
There was a problem hiding this comment.
Would you tell me why are you using typeof data[key] === typeof {}, not typeof data[key] === 'object' ?
I think typeof data[key] === 'object' is better.
There was a problem hiding this comment.
The null check was necessary because null is of type object. If you're updating an object with {message: null}, it will pass this conditional, which it technically shouldn't since it's not a nested object.
|
It is necessary to execute |
packages/test-utils/src/wrapper.js
Outdated
| if (typeof data[key] === 'object' && data[key] !== null) { | ||
| Object.keys(data[key]).forEach((key2) => { | ||
| var newObj = Object.assign({}, data[key], data[key][key2]) | ||
| const newObj = { ...data[key], ...data[key][key2]} |
There was a problem hiding this comment.
It is necessary to add space.
const newObj = { ...data[key], ...data[key][key2] }
|
It is necessary to execute |
test/specs/wrapper/setData.spec.js
Outdated
| wrapper.setData({ message: null }) | ||
| expect(wrapper.text()).to.equal('There is no message yet') | ||
| }) | ||
| it('should update a data object', () => { |
There was a problem hiding this comment.
I think it is better to add a blank line before this line.
| } | ||
| } | ||
| }) | ||
| expect(wrapper.vm.anObject.propA.prop1).to.equal('c') |
There was a problem hiding this comment.
Can you give propA another property, and add an assertion that the value is unchanged after calling setData.
test/specs/wrapper/setData.spec.js
Outdated
| expect(wrapper.text()).to.equal('There is no message yet') | ||
| }) | ||
| it('should update a data object', () => { | ||
| const Cmp = { |
There was a problem hiding this comment.
The convention we follow in this repo is to name these TestComponent
|
Updated! ✨ |
3 participants
This is a refactor of the setData method to allow for setting data values and objects. The current implementation of setData only allows for setting values and not objects.
Addresses #563