-
Notifications
You must be signed in to change notification settings - Fork 0
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
Merge Component Lifecycle guide into React.Component reference #1
Merge Component Lifecycle guide into React.Component reference #1
Conversation
…/react into lacker-reference-react-component
…reate React without ES6 Guide
|
||
`React.Component` is an abstract base class, so it rarely makes sense to refer to `React.Component` directly. Instead, you will typically subclass it, and define at least a [`.render()`](#.render) method. | ||
|
||
Normally you would define a React component as a plain JavaScript class: |
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.
Let's make "JavaScript class" a link to MDN: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Classes
|
||
#### Updating | ||
|
||
State changes can lead to the component being re-rendered. The following methods will be called and are used to determine if the DOM should be updated. |
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.
Maybe "Changes to props or state can lead..."? Updating is not just for state.
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.
The following methods will be called and are used to determine if the DOM should be updated.
The only method that determines if DOM should be updated is shouldComponentUpdate
, the rest don't influence this. Can we omit "and are used to determine..."?
State changes can lead to the component being re-rendered. The following methods will be called and are used to determine if the DOM should be updated. | ||
|
||
- [`.componentWillReceiveProps()`](#.componentwillreceivepropsnextprops) is invoked before a mounted component receives new props and can be used to update state in response to prop changes. | ||
- Components will always re-render when the state is updated. Take a look at [`.shouldComponentUpdate(nextProps, nextState)`](#.shouldcomponentupdatenextprops-nextstate) if you need to change the default behavior. |
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.
Components will always re-render when the state is updated
People often misinterpret "re-render" as "expensive DOM change". Can we make it clear re-rendering is usually absolutely fine?
- [`.componentWillReceiveProps()`](#.componentwillreceivepropsnextprops) is invoked before a mounted component receives new props and can be used to update state in response to prop changes. | ||
- Components will always re-render when the state is updated. Take a look at [`.shouldComponentUpdate(nextProps, nextState)`](#.shouldcomponentupdatenextprops-nextstate) if you need to change the default behavior. | ||
- [`.componentWillUpdate(nextProps, nextState)`](#.componentwillupdatenextprops-nextstate) can be used for preparation before an update occurs. | ||
- [`.componentDidUpdate(prevProps, prevState)`](#.componentdidupdateprevprops-prevstate) is invoked after an update and can be used to operate on the DOM after the component has been updated. |
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.
Maybe add "... or to fetch new data in response to prop changes?"
|
||
[`.componentWillUnmount()`](#.componentwillunmount) can be used to perform any cleanup, as it is invoked immediately before a component is unmounted and destroyed. | ||
|
||
## Reference | ||
|
||
- [`constructor(props)`](#constructor) |
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 understand these are sorted by alphabet, but maybe we can sort them by their order instead? This seems more useful to me, and it's one of the most commonly asked questions about lifecycle.
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 I agree - let me do that in a subsequent PR though, since this one is merging Hector's content into the layout that I set up.
@@ -44,27 +99,47 @@ constructor(props) { | |||
|
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.
It's okay to initialize state based on props.
Only if you know what you're doing. Most beginners do this by mistake. This effectively "forks" the props and can lead to bugs.
Instead of syncing props to state, you often want to lift the state up: facebook#7920 (comment).
If you "fork" props by using them for state, you might also want to implement componentWillReceiveProps()
to keep the state up-to-date with them. But lifting state up is often easier and less bug-prone.
TODO | ||
`componentDidUpdate()` is invoked immediately after updating occurs. This method is not called for the initial render. | ||
|
||
Use this as an opportunity to operate on the DOM when the component has been updated. |
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 also a good place to do network requests as long as you compare the props to previous props.
TODO | ||
`componentWillMount()` is invoked immediately before mounting occurs. It is called before `render()`, therefore setting state in this method will not trigger a re-rendering. Avoid introducing any side-effects or subscriptions in this method. | ||
|
||
This is the only lifecycle hook called on server rendering. |
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.
Maybe add a note that generally we recommend using constructor
instead?
@@ -88,7 +163,13 @@ The `displayName` string is used in debugging messages. JSX sets this value auto | |||
|
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.
If
props.color
is not provided, it will be set by default to'blue'
.
Maybe mention that defaultProps are only used for undefined props, but not for null props.
@@ -155,11 +240,19 @@ The second parameter is an optional callback function that will be executed once | |||
|
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.
The second parameter is an optional callback function that will be executed once
setState
is completed and the component is re-rendered.
Maybe add "Generally we recommend using componentDidUpdate()
for such logic instead."
All feedback addressed. I also removed the dots in the method names. @lacker I think we can merge this into your fork now, and do any further reordering/organization in your main PR. |
The overview section at the top of a reference guide is a good place to cover best practices for use of the class whenever the scope is greater than any single method.
Opening this PR against @lacker's fork so we can then merge these commits into the existing react/docs PR #7919.