-
Notifications
You must be signed in to change notification settings - Fork 667
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
Change the way of setting props #1300
Conversation
Thanks for the PR! This PR fixes For example: it.only('fails when run on child component', async () => {
const prop1 = 'prop 1'
const ChildComponent = {
template: '<div class="prop1">{{prop1}}</div>',
props: ['prop1']
}
const TestComponent = {
template: '<child-component />',
components: {
ChildComponent
}
}
const wrapper = mountingMethod(TestComponent)
wrapper.find(ChildComponent).setProps({ prop1 })
await Vue.nextTick()
expect(wrapper.find('.prop-1').element.textContent).to.equal(prop1)
}) The solution is to only allow |
@eddyerburgh Nice catch. Will do - it seems perfectly logical for me to allow setProps only on root component :) |
Hi @xanf, are you planning on adding the fix? |
@eddyerburgh Thank you for ping. Felt a bit off my radar, will update PR this week |
Hi @xanf! Did you find the opportunity to work on this? :) |
@afontcu thank you for ping, will have something on Monday I hope, was busy with upgrading GitLab codebase for beta.30 release :) |
} | ||
}) | ||
// $FlowIgnore : Problem with possibly null this.vm | ||
this.vm.$forceUpdate() | ||
Vue.config.silent = originalConfig | ||
// We need to explicitly trigger parent watcher to support sync scenarios |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this, sync mode is gone now.
Since we're already wrapping a component into a parent one we can get rid of explicitly setting props on component, instead modifying them being passed from parent component instead
4d65406
to
cbf1bd4
Compare
@@ -244,6 +245,17 @@ describeWithShallowAndMount('setProps', mountingMethod => { | |||
expect(wrapper.vm.propA).to.equal('value') | |||
}) | |||
|
|||
it('correctly sets props in async mode when component has immediate watchers', async () => { | |||
const wrapper = mountingMethod(ComponentWithWatchImmediate, { | |||
sync: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be missing something here, but I think sync
is gone as per #1141?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled it out of the src. That's a typo. Thanks for the catch
Closing this, since #1618 was merged |
Since we're already wrapping a component into a parent one we can get rid of explicitly setting props on component, instead modifying them being passed from parent component instead.
This fixes weird behaviors in async mode, when prop is immediately reset to it's original value
Fixes #1140