Simplified Life Cycles #4171
Merged
Simplified Life Cycles #4171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This should only be used for tracking string refs. For that purpose, we need a single central stateful module that is coupled to createElement. Which is why it needs to live in isomorphic. Eventually this will go away completely.
It was impossible to get here because if you enqueue something Also ensure that they're only used in DEV because we will be reading state that is DEV only here.
This was used for any invariant that was subsequently removed. It turns out that this is completely unnecessary now. Any setState calls will enqueue and update and the component added to the update queue. However, since the pending fields are reset after componentWillUnmount, any update will still be ignored.
This simply ignores any enqueued actions. This means that we don't have to have special logic for componentWillMount. It is just that those updates are never enqueued.
This allows updates to be enqueued during render. setState in componentWillMount will still be collected as part of the first pass so if nothing else get added as pending, they won't trigger a second rerender. This allow us to get rid of one more stateful special case.
Since I fixed the server-side rendering it is now possible to trigger these callbacks on the client alone. They will still be queued up on the server but they are never executed.
This hack allow us to get rid of the stateful module ReactLifeCycle since we can infer the value of isMounted even without it. This gets rid of the try/catch which is deopting all mountComponent calls. As a next step we could deprecate isMounted completely and avoid stateful APIs. Since it can be easily simulated if you truly need it.
|
Can you add a test for what happens when you call setState on an unmounted composite, and can you add a test to make sure that a composite that returns null still gives Otherwise good. |
|
We already have a test for setState on an unmounted here: and unmounting here: |
|
Can you make sure it doesn't re-call render? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a sequence of changes that simplifies the way we do life-cycles. At the end we are able to completely get rid of the stateful ReactLifeCycle module.
The breaking change is that setState callbacks in componentWillMount now works again but won't fire on the server.