Use fewer global variables in Hooks #17480
Merged
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 means that it's harder to find it since it's not in the dispatch function's stack but we can add a DEV only one for that if we really need it. Alternatively, we can check it in against the renderUpdates queue.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1ea22f8:
|
When accessed, this should always be set. This could enforced by storing this on the dispatcher for example.
sebmarkbage
force-pushed
the
hooks-less-global-state
branch
from
e73d0f2
to
25aa2e5
Compare
acdlite
approved these changes
Nov 28, 2019
acdlite
reviewed
Dec 2, 2019
| do { | ||
| didScheduleRenderPhaseUpdate = false; | ||
|
|
||
| invariant( | ||
| numberOfReRenders < RE_RENDER_LIMIT, |
There was a problem hiding this comment.
The stack on this error is less useful now
There was a problem hiding this comment.
In the commit I mentioned that we could add a DEV warning to help this. However, I think I could do a follow up of this that uses the dispatcher to error instead in this: #17484
trueadm
pushed a commit
to trueadm/react
that referenced
this pull request
Dec 4, 2019
* We don't need the global state for this * Move componentUpdateQueue and sideEffectTag out of global state * Move firstWorkInProgressHook off global state * Move remainingExpirationTime off global state * Reset fiber to its current state if it throws * Move rerender error check to avoid global state This means that it's harder to find it since it's not in the dispatch function's stack but we can add a DEV only one for that if we really need it. Alternatively, we can check it in against the renderUpdates queue. * Move next___Hook out of global state * Assert that currentlyRenderingFiber is always set When accessed, this should always be set. This could enforced by storing this on the dispatcher for example. * Add another test just to be safe
trueadm
pushed a commit
to trueadm/react
that referenced
this pull request
Dec 4, 2019
* We don't need the global state for this * Move componentUpdateQueue and sideEffectTag out of global state * Move firstWorkInProgressHook off global state * Move remainingExpirationTime off global state * Reset fiber to its current state if it throws * Move rerender error check to avoid global state This means that it's harder to find it since it's not in the dispatch function's stack but we can add a DEV only one for that if we really need it. Alternatively, we can check it in against the renderUpdates queue. * Move next___Hook out of global state * Assert that currentlyRenderingFiber is always set When accessed, this should always be set. This could enforced by storing this on the dispatcher for example. * Add another test just to be safe
Merged
This was referenced Mar 6, 2020
This was referenced Sep 25, 2024
This was referenced Sep 28, 2024
This was referenced Sep 29, 2024
This was referenced Oct 4, 2024
This was referenced Oct 5, 2024
This was referenced Oct 12, 2024
This was referenced Oct 25, 2024
This was referenced Oct 26, 2024
This was referenced Oct 27, 2024
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.
Module level variables is an exception that we should use sparingly. It will make using multiple threads in a threaded implementation infeasible, harder to port to stricter languages, it's harder to reason about the performance and it is kind of hard to reason about whether these are allowed to diverge from the main state.