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

fix(props): handle misspelled keys on prop validation object #7198

Merged
merged 5 commits into from
Dec 14, 2017

Conversation

Alex-Sokolov
Copy link
Contributor

@Alex-Sokolov Alex-Sokolov commented Dec 7, 2017

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

  • Bugfix

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:

Other information:

When using object for validating props, there are no checks about object content. This may take a long time before noticing that you forget to pass required prop, but there are no warnings with a misprint.

Fail cases:

  • Typo in key name deafult: false or require: true
  • Using non-english letter in key name (for example, russian)

P.S.: Will be great to add instance sections names validation, but I have no idea how to solve this (once I spent a lot of time before see that writed component: {} section instead components: {}).

@yyx990803
Copy link
Member

Thanks for the PR. Placing the check in assertProp incurs more runtime cost than necessary (even only in dev mode) as assertProp is also called on every re-render of the component. This check only needs to be done once in initProps.

@Alex-Sokolov
Copy link
Contributor Author

@yyx990803 Thanks for info — moved to initProps!

@yyx990803
Copy link
Member

Thanks for the update - as I was reviewing again I realized the runtime cost can be further reduced by placing the check here when the options are normalized... (performed once per constructor)

Also, even though this is dev-only code, I'd prefer to refactor to use a for...in loop to get rid of the iteration functions.

@Alex-Sokolov
Copy link
Contributor Author

@yyx990803 done

@mattixittam
Copy link

This warns about anything which relies on adding custom attributes to the prop object. For instance this: vue-leaflet/Vue2Leaflet#109.

In another use-case, we add a note attribute to props for generating documentation.

I would suggest making this warning configurable somehow.

@JounQin
Copy link
Contributor

JounQin commented Dec 19, 2017

A custom key meta like vue-router would be enough.

lovelope pushed a commit to lovelope/vue that referenced this pull request Feb 1, 2018
f2009 pushed a commit to f2009/vue that referenced this pull request Jan 25, 2019
aJean pushed a commit to aJean/vue that referenced this pull request Aug 19, 2020
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.

None yet

4 participants