-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Reorganize component props to introduce prop types earlier, fixes #1644 #1646
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
Conversation
@BlitZz96 I can't tag you for review, but I'd love to hear your thoughts. |
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.
This is a huge improvement from the previous state. I love the part where you also shine a light that it's considered a good practice as you are also documenting your component. Great job, this is what i had in mind. Nice touch with the referral to prop validations.
Glad to hear it! Thanks again for pointing out the problem. 🙂 Now one more review from a core team member and I'll merge this. |
props: ['title', 'likes', 'isPublished', 'commentIds', 'author'] | ||
``` | ||
|
||
Usually though, you'll want every prop to be a specific type of value. In these cases, you can list props as an object, where the properties' names and values contain the prop names and types, respectively: |
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.
Isn't it "to be of a specific type of value?"
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'm not sure on grammatical correctness, but "to be a" is more common than "to be of a". The sentence reads fine to me.
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.
Yeah, not a native speaker myself, just drawing my inspiration from the infamous "Argument of type 'string | number' is not assignable" error. Then again, it's very minor and can be safely ignored if need be.
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.
@phanan I think if we didn't have "of value" at the end I agree that "of type" would sound better, but as it is sounds good to me. 🙂 Thanks for the review!
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 have but a super minor feedback, otherwise all LGTM. Thanks!
* Update unit-testing-vue-components.md (vuejs#1623) * Fixed a typo (vuejs#1625) Removed an extra 's' to fix a grammatical mistake * fix a typo (vuejs#1627) * Update v-for list example (vuejs#1628) * Add presentation role to style-only li item * fix: code snippet language (close vuejs#1617) (vuejs#1626) * Update todo-list-example for a11y (vuejs#1630) * Update todo-list-example for a11y * Implement review suggestions * Use explicit label instead of aria-label * Move v-model directive to top for improved readability * change NODE_ENV to follow recommendations from webpack (vuejs#1622) * change NODE_ENV to follow recommendations from weback * Update deployment.md * note that modes are not available in transition groups, fixes vuejs#1619 * add examples for in API doc * add comparison note about react-like libraries * Update components.md (vuejs#1632) * Update components.md * Update components.md * improve flow of single root component section * cookbook entry for storage (vuejs#1550) * cookbook entry for storage * minor tweak * updates to article * updates to article * maybe the last tweak * the honest to god really last mod * Update index.md (vuejs#1634) * docs(guide/comparison): correct size stat of Angular CLI project (vuejs#1638) * Update client-side-storage.md (vuejs#1640) * Update creating-custom-scroll-directives.md (vuejs#1639) * chore: update ad code * remove unnecessary word 'know' (vuejs#1641) * Updated broken links for the Vue Test Utils documentation. (vuejs#1643) * Reorganize component props to introduce prop types earlier, fixes vuejs#1644 (vuejs#1646) @BlitZz96 brought up a good point in [this issue](vuejs#1644) about users lacking the necessary background knowledge to fully understand Passing Static and Dynamic Props, at least as it relates to booleans and potentially other types in the future. To fix this, I added a new Prop Types section right before it. * First stab at SFC to npm documentation (vuejs#1558) * First stab at SFC to npm documentation * Clean up sample code, clarify .vue usage with SSR builds * Run build tasks in parallel, fix dependency order * Backtick all instances of .vue, minor edits for clarity * Fix typo on link * Update url to vue-sfc-rollup utility in acknowledgements * Fix order, other edits suggested by @sdras * Additional explanation about sfc-to-npm recipe package.json * Rename packaing-sfc-for-npm.md to packaging-sfc-for-npm.md (vuejs#1652) * Update link in comparison guide (vuejs#1653) The phrase "These state management patterns and even Redux itself can be easily integrated into Vue applications" has a link to a deprecated project. Instead I link to a yarnpkg.com search I saw used in https://vuejs.org/v2/guide/state-management.html, which links all "redux vue" projects (so it's always up to date) * chore: update vue version * Fix link to docs on custom inputs (vuejs#1660) The old link redirected me to https://vuejs.org/v2/guide/components-custom-events.html#Binding-Native-Events-to-Components which was unexpected. I think the correct link is https://vuejs.org/v2/guide/components.html#Using-v-model-on-Components as per this change. * correct wrong link (vuejs#1661) * Update events.md correct wrong link * Update events.md correct wrong link * Feature/vuemeetups (vuejs#1665) * feature: Add link to VueMeetups * feature: Add section on becoming a community leader * minor tweaks to meetup section * Added details about Lifecycle Hooks (vuejs#1664) * Added details about Lifecycle Hooks Mostly a clarification for beginners without prior knowledge about what Lifecycle hooks are. Makes it easier to understand when perhaps only knowing that the developer has a `mounted()` hook. I left out the function syntax because of generality. Add if it makes more sense to add. I am not fully knowledgable in Vue yet. * add (de)activated to lifecycle hooks list in style guide * improve comparisons react scaling down section * add text versions of image code examples * remove extra comma in components * TypeScript project generation in @vue/cli 3.0 (vuejs#1668) * TypeScript project generation in @vue/cli 3.0 update information about generating TypeScript project using new @vue/cli 3.0 * tweaks to Vue CLI 3 TypeScript support docs * chore: update sponsors * Sylvain and Forresst feedbacks Signed-off-by: MachinisteWeb <[email protected]> * @ForrestT and @raisonblue feedback Signed-off-by: MachinisteWeb <[email protected]> * Feedback of @rspt Signed-off-by: MachinisteWeb <[email protected]>
* Update unit-testing-vue-components.md (vuejs#1623) * Fixed a typo (vuejs#1625) Removed an extra 's' to fix a grammatical mistake * fix a typo (vuejs#1627) * Update v-for list example (vuejs#1628) * Add presentation role to style-only li item * fix: code snippet language (close vuejs#1617) (vuejs#1626) * Update todo-list-example for a11y (vuejs#1630) * Update todo-list-example for a11y * Implement review suggestions * Use explicit label instead of aria-label * Move v-model directive to top for improved readability * change NODE_ENV to follow recommendations from webpack (vuejs#1622) * change NODE_ENV to follow recommendations from weback * Update deployment.md * note that modes are not available in transition groups, fixes vuejs#1619 * add examples for in API doc * add comparison note about react-like libraries * Update components.md (vuejs#1632) * Update components.md * Update components.md * improve flow of single root component section * cookbook entry for storage (vuejs#1550) * cookbook entry for storage * minor tweak * updates to article * updates to article * maybe the last tweak * the honest to god really last mod * Update index.md (vuejs#1634) * docs(guide/comparison): correct size stat of Angular CLI project (vuejs#1638) * Update client-side-storage.md (vuejs#1640) * Update creating-custom-scroll-directives.md (vuejs#1639) * chore: update ad code * remove unnecessary word 'know' (vuejs#1641) * Updated broken links for the Vue Test Utils documentation. (vuejs#1643) * Reorganize component props to introduce prop types earlier, fixes vuejs#1644 (vuejs#1646) @BlitZz96 brought up a good point in [this issue](vuejs#1644) about users lacking the necessary background knowledge to fully understand Passing Static and Dynamic Props, at least as it relates to booleans and potentially other types in the future. To fix this, I added a new Prop Types section right before it. * First stab at SFC to npm documentation (vuejs#1558) * First stab at SFC to npm documentation * Clean up sample code, clarify .vue usage with SSR builds * Run build tasks in parallel, fix dependency order * Backtick all instances of .vue, minor edits for clarity * Fix typo on link * Update url to vue-sfc-rollup utility in acknowledgements * Fix order, other edits suggested by @sdras * Additional explanation about sfc-to-npm recipe package.json * Rename packaing-sfc-for-npm.md to packaging-sfc-for-npm.md (vuejs#1652) * Update link in comparison guide (vuejs#1653) The phrase "These state management patterns and even Redux itself can be easily integrated into Vue applications" has a link to a deprecated project. Instead I link to a yarnpkg.com search I saw used in https://vuejs.org/v2/guide/state-management.html, which links all "redux vue" projects (so it's always up to date) * chore: update vue version * Fix link to docs on custom inputs (vuejs#1660) The old link redirected me to https://vuejs.org/v2/guide/components-custom-events.html#Binding-Native-Events-to-Components which was unexpected. I think the correct link is https://vuejs.org/v2/guide/components.html#Using-v-model-on-Components as per this change. * correct wrong link (vuejs#1661) * Update events.md correct wrong link * Update events.md correct wrong link * Feature/vuemeetups (vuejs#1665) * feature: Add link to VueMeetups * feature: Add section on becoming a community leader * minor tweaks to meetup section * Added details about Lifecycle Hooks (vuejs#1664) * Added details about Lifecycle Hooks Mostly a clarification for beginners without prior knowledge about what Lifecycle hooks are. Makes it easier to understand when perhaps only knowing that the developer has a `mounted()` hook. I left out the function syntax because of generality. Add if it makes more sense to add. I am not fully knowledgable in Vue yet. * add (de)activated to lifecycle hooks list in style guide * improve comparisons react scaling down section * add text versions of image code examples * remove extra comma in components * TypeScript project generation in @vue/cli 3.0 (vuejs#1668) * TypeScript project generation in @vue/cli 3.0 update information about generating TypeScript project using new @vue/cli 3.0 * tweaks to Vue CLI 3 TypeScript support docs * chore: update sponsors * Sylvain and Forresst feedbacks Signed-off-by: MachinisteWeb <[email protected]> * chore: update sponsors * update CLI docs link * add Scrimba links for preview * @ForrestT and @raisonblue feedback Signed-off-by: MachinisteWeb <[email protected]> * Feedback of @rspt Signed-off-by: MachinisteWeb <[email protected]> * Fix example indentation in the Unit Testing Vue Components recipe (vuejs#1683) * Update debugging-in-vscode.md (vuejs#1662) * Added link to vuemeetups.org to the ecosystem drop-down menu on website. (vuejs#1685) * Correct mistake of Migration from Vue 1.x (vuejs#1686) * Fix code style in Form Validation recipe (vuejs#1682) * Fix code style in Form Validation recipe * More style code slight improvement in Form Validation recipe * Fix code style in Client-Side Storage recipe (vuejs#1690) * [Doc EN]: `typescript.md` (vuejs#1679) * New in with + symbol Signed-off-by: Bruno Lesieur <[email protected]> * Review of 2.5.0 doc Signed-off-by: Bruno Lesieur <[email protected]> * Review Signed-off-by: Bruno Lesieur <[email protected]> * Fix syntax typo Signed-off-by: Bruno Lesieur <[email protected]> * Add space between new line of documentation Signed-off-by: MachinisteWeb <[email protected]> * Add @posva review Signed-off-by: MachinisteWeb <[email protected]> * Add french str Signed-off-by: MachinisteWeb <[email protected]> * Change directeur to director Signed-off-by: MachinisteWeb <[email protected]> * Fix EN doc Signed-off-by: MachinisteWeb <[email protected]> * Subtitle for typescript.md Signed-off-by: MachinisteWeb <[email protected]> * Revert instance.md Signed-off-by: MachinisteWeb <[email protected]> * [cookbook] Dockerize Vue.js App (vuejs#1483) * First draft with 'Simple Example' section * Fix typo on Docker 'build' command * Fix copy of multiple files * Remove unnecessary whitespace * Fix typo in webpack template link * First draft of 'Real-World Example' * Update 'Real-World Example' with multi-stage build * Add 'Why Dockerize a Vue.js App?' section * Rename 'Why Dockerize a Vue.js App?' as 'Additinal Context' * Add 'Alternative Patterns' section * Minor fix on 'Alternative Patterns' * Fixed typo * Update order to avoid collision with other cookbooks * Clarify why NGINX in real-world example * Update debugging-in-vscode.md (vuejs#1663) * remove outdated information about polymer from comparison * update React comparison status * Remove extra part Signed-off-by: Haeresis <[email protected]> * Remove extra content from api/index.md Signed-off-by: Haeresis <[email protected]> * Remove extra merge Signed-off-by: Haeresis <[email protected]>
* Update unit-testing-vue-components.md (vuejs#1623) * Fixed a typo (vuejs#1625) Removed an extra 's' to fix a grammatical mistake * fix a typo (vuejs#1627) * Update v-for list example (vuejs#1628) * Add presentation role to style-only li item * fix: code snippet language (close vuejs#1617) (vuejs#1626) * Update todo-list-example for a11y (vuejs#1630) * Update todo-list-example for a11y * Implement review suggestions * Use explicit label instead of aria-label * Move v-model directive to top for improved readability * change NODE_ENV to follow recommendations from webpack (vuejs#1622) * change NODE_ENV to follow recommendations from weback * Update deployment.md * note that modes are not available in transition groups, fixes vuejs#1619 * add examples for in API doc * add comparison note about react-like libraries * Update components.md (vuejs#1632) * Update components.md * Update components.md * improve flow of single root component section * cookbook entry for storage (vuejs#1550) * cookbook entry for storage * minor tweak * updates to article * updates to article * maybe the last tweak * the honest to god really last mod * Update index.md (vuejs#1634) * docs(guide/comparison): correct size stat of Angular CLI project (vuejs#1638) * Update client-side-storage.md (vuejs#1640) * Update creating-custom-scroll-directives.md (vuejs#1639) * chore: update ad code * remove unnecessary word 'know' (vuejs#1641) * Updated broken links for the Vue Test Utils documentation. (vuejs#1643) * Reorganize component props to introduce prop types earlier, fixes vuejs#1644 (vuejs#1646) @BlitZz96 brought up a good point in [this issue](vuejs#1644) about users lacking the necessary background knowledge to fully understand Passing Static and Dynamic Props, at least as it relates to booleans and potentially other types in the future. To fix this, I added a new Prop Types section right before it. * First stab at SFC to npm documentation (vuejs#1558) * First stab at SFC to npm documentation * Clean up sample code, clarify .vue usage with SSR builds * Run build tasks in parallel, fix dependency order * Backtick all instances of .vue, minor edits for clarity * Fix typo on link * Update url to vue-sfc-rollup utility in acknowledgements * Fix order, other edits suggested by @sdras * Additional explanation about sfc-to-npm recipe package.json * Rename packaing-sfc-for-npm.md to packaging-sfc-for-npm.md (vuejs#1652) * Update link in comparison guide (vuejs#1653) The phrase "These state management patterns and even Redux itself can be easily integrated into Vue applications" has a link to a deprecated project. Instead I link to a yarnpkg.com search I saw used in https://vuejs.org/v2/guide/state-management.html, which links all "redux vue" projects (so it's always up to date) * chore: update vue version * Fix link to docs on custom inputs (vuejs#1660) The old link redirected me to https://vuejs.org/v2/guide/components-custom-events.html#Binding-Native-Events-to-Components which was unexpected. I think the correct link is https://vuejs.org/v2/guide/components.html#Using-v-model-on-Components as per this change. * correct wrong link (vuejs#1661) * Update events.md correct wrong link * Update events.md correct wrong link * Feature/vuemeetups (vuejs#1665) * feature: Add link to VueMeetups * feature: Add section on becoming a community leader * minor tweaks to meetup section * Added details about Lifecycle Hooks (vuejs#1664) * Added details about Lifecycle Hooks Mostly a clarification for beginners without prior knowledge about what Lifecycle hooks are. Makes it easier to understand when perhaps only knowing that the developer has a `mounted()` hook. I left out the function syntax because of generality. Add if it makes more sense to add. I am not fully knowledgable in Vue yet. * add (de)activated to lifecycle hooks list in style guide * improve comparisons react scaling down section * add text versions of image code examples * remove extra comma in components * TypeScript project generation in @vue/cli 3.0 (vuejs#1668) * TypeScript project generation in @vue/cli 3.0 update information about generating TypeScript project using new @vue/cli 3.0 * tweaks to Vue CLI 3 TypeScript support docs * chore: update sponsors * chore: update sponsors * update CLI docs link * add Scrimba links for preview * Fix example indentation in the Unit Testing Vue Components recipe (vuejs#1683) * Update debugging-in-vscode.md (vuejs#1662) * Added link to vuemeetups.org to the ecosystem drop-down menu on website. (vuejs#1685) * Correct mistake of Migration from Vue 1.x (vuejs#1686) * Fix code style in Form Validation recipe (vuejs#1682) * Fix code style in Form Validation recipe * More style code slight improvement in Form Validation recipe * Fix code style in Client-Side Storage recipe (vuejs#1690) * [Doc EN]: `typescript.md` (vuejs#1679) * New in with + symbol Signed-off-by: Bruno Lesieur <[email protected]> * Review of 2.5.0 doc Signed-off-by: Bruno Lesieur <[email protected]> * Review Signed-off-by: Bruno Lesieur <[email protected]> * Fix syntax typo Signed-off-by: Bruno Lesieur <[email protected]> * Add space between new line of documentation Signed-off-by: MachinisteWeb <[email protected]> * Add @posva review Signed-off-by: MachinisteWeb <[email protected]> * Add french str Signed-off-by: MachinisteWeb <[email protected]> * Change directeur to director Signed-off-by: MachinisteWeb <[email protected]> * Fix EN doc Signed-off-by: MachinisteWeb <[email protected]> * Subtitle for typescript.md Signed-off-by: MachinisteWeb <[email protected]> * Revert instance.md Signed-off-by: MachinisteWeb <[email protected]> * [cookbook] Dockerize Vue.js App (vuejs#1483) * First draft with 'Simple Example' section * Fix typo on Docker 'build' command * Fix copy of multiple files * Remove unnecessary whitespace * Fix typo in webpack template link * First draft of 'Real-World Example' * Update 'Real-World Example' with multi-stage build * Add 'Why Dockerize a Vue.js App?' section * Rename 'Why Dockerize a Vue.js App?' as 'Additinal Context' * Add 'Alternative Patterns' section * Minor fix on 'Alternative Patterns' * Fixed typo * Update order to avoid collision with other cookbooks * Clarify why NGINX in real-world example * Update debugging-in-vscode.md (vuejs#1663) * remove outdated information about polymer from comparison * update React comparison status * chore: adjust sponsor placement * chore: sponsor meta * adjust sponsor naming on pages * Add alt text to images to help with screen reader accessibility (vuejs#1676) * chore: update sponsors * chore: add tidelift
@BlitZz96 brought up a good point in this issue about users lacking the necessary background knowledge to fully understand Passing Static and Dynamic Props, at least as it relates to booleans and potentially other types in the future. To fix this, I added a new Prop Types section right before it.