Skip to content

fix(props): consider prop types order for default bool value #7583

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

Closed
wants to merge 2 commits into from

Conversation

miro-ku
Copy link

@miro-ku miro-ku commented Feb 1, 2018

Fix #7485

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@miro-ku
Copy link
Author

miro-ku commented Feb 7, 2018

Can i know, why there is no any action? Maybe i've made something wrong?

@posva
Copy link
Member

posva commented Feb 7, 2018

Hey sorry for the delay, it went under my radar 😄
Could you add a test, please?

@miro-ku
Copy link
Author

miro-ku commented Feb 8, 2018

@posva done ;)

@miro-ku
Copy link
Author

miro-ku commented Feb 17, 2018

Anyone alive there?

@mariolamacchia
Copy link

Hey there! Does this PR require more work to do? I'd love to have these changes deployed 🙂

@yyx990803
Copy link
Member

Thanks for the PR, I will be doing final reviews/merging of outstanding PRs next week, please be patient.

@yyx990803
Copy link
Member

Thanks again for the PR. There is an edge case not taken into account in this fix - that the Vue runtime might run in a different document from the app's code (see https://github.com/vuejs/vue/blob/dev/src/core/util/props.js#L178-L186)

I implemented a similar logic in 81e1e47 which fixes the original issue.

@yyx990803 yyx990803 closed this Mar 9, 2018
@miro-ku miro-ku deleted the consider-order-of-prop-types branch March 10, 2018 16:46
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.

Multi-type props defined without value gets empty string, should be true [Boolean, String]
4 participants