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

Change the way of setting props #1300

Closed
wants to merge 2 commits into from
Closed

Conversation

xanf
Copy link
Contributor

@xanf xanf commented Sep 2, 2019

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

@eddyerburgh
Copy link
Member

eddyerburgh commented Sep 8, 2019

Thanks for the PR!

This PR fixes setProps for root components, but not child components.

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 setProps on root-level components. Can you update setProps to trow an error if it's called on a non-root component?

@xanf
Copy link
Contributor Author

xanf commented Sep 8, 2019

@eddyerburgh Nice catch. Will do - it seems perfectly logical for me to allow setProps only on root component :)

@eddyerburgh
Copy link
Member

Hi @xanf, are you planning on adding the fix?

@xanf
Copy link
Contributor Author

xanf commented Dec 18, 2019

@eddyerburgh Thank you for ping. Felt a bit off my radar, will update PR this week

@afontcu
Copy link
Member

afontcu commented Jan 3, 2020

Hi @xanf! Did you find the opportunity to work on this? :)

@xanf
Copy link
Contributor Author

xanf commented Jan 3, 2020

@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
Copy link
Member

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
@@ -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
Copy link
Member

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct.

Copy link
Collaborator

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

@xanf
Copy link
Contributor Author

xanf commented Aug 22, 2020

Closing this, since #1618 was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prop not updated when immediate watcher accessing it (async mode)
5 participants