-
Notifications
You must be signed in to change notification settings - Fork 3
Open
@TonyGravagno
Description
ref:
https://github.com/reactpatterns/reactpatterns-website/blob/main/docs/stateless-function.md
First Suggestion
I recommend a revision of this example
import PropTypes from 'prop-types' const UserPassword = props => <p>The user password is: {this.props.userpassword}</p> UserPassword.propTypes = { userpassword: PropTypes.string.isRequired, } Username.defaultProps = { username: 'Jonh', }
to:
import PropTypes from 'prop-types' const UserName = props => <p>The user name is: {this.props.username}</p> UserName.propTypes = { username: PropTypes.string.isRequired, } UserName.defaultProps = { username: 'Jonh', }
Reasons
- In the original, we see a default for username but we don't see where it's used. In the revision it's clear how the username is used.
- In the original, UserPassword is PascalCased, but Username is a single word. In the revision there is only one function, and it's consistently PascalCased.
- It's not practical to display a password. It is practical to display a name.
Second Suggestion
Regarding this example:
import PropTypes from 'prop-types' const UserPassword = (props, context) => <p>User password is {context.password}</p> WowComponent.contextTypes = { password: PropTypes.string.isRequired, }
Concerns:
- contextTypes is from the legacy API.
- WowComponent isn't used in these examples - this example should include a reference to UserName.
- The RefOrContext parameter (second 'context' param on function) should probably be replaced with useContext.
- I believe PropTypes.shape should be used to describe context.
Since there are multiple ways of approaching changes to this example, I'm not providing a suggestion.
Thanks!
Metadata
Metadata
Assignees
Labels
No labels