Fixes a bug where lower priority updates on a components wrapped with
`memo` are sometimes left dangling in the queue without ever being
processed, if they are preceded by a higher priority bailout.
Cause
-----
The pending update priority field is cleared at the beginning of
`beginWork`. If there is remaining work at a lower priority level, it's
expected that it will be accumulated on the work-in-progress fiber
during the begin phase.
There's an exception where this assumption doesn't hold:
SimpleMemoComponent contains a bailout that occurs *before* the
component is evaluated and the update queues are processed, which means
we don't accumulate the next priority level. When we complete the fiber,
the work loop is left to believe that there's no remaining work.
Mitigation
----------
Since this only happens in a single case, a late bailout in
SimpleMemoComponent, I've mitigated the bug in that code path by
restoring the original update priority from the current fiber.
This same case does not apply to MemoComponent, because MemoComponent
fibers do not contain hooks or update queues; rather, they wrap around
an inner fiber that may contain those. However, I've added a test case
for MemoComponent to protect against a possible future regression.
Possible next steps
-------------------
We should consider moving the update priority assignment in `beginWork`
out of the common path and into each branch, to avoid similar bugs in
the future.
This warning already exists for class components, but not for functions.
It does not apply to render phase updates to the same component, which
have special semantics that we do support.
* Re-throw errors thrown by the renderer at the root
React treats errors thrown at the root as a fatal because there's no
parent component that can capture it. (This is distinct from an
"uncaught error" that isn't wrapped in an error boundary, because in
that case we can fall back to deleting the whole tree -- not great, but
at least the error is contained to a single root, and React is left in a
consistent state.)
It turns out we didn't have a test case for this path. The only way it
can happen is if the renderer's host config throws. We had similar test
cases for host components, but none for the host root.
This adds a new test case and fixes a bug where React would keep
retrying the root because the `workInProgress` pointer was not advanced
to the next fiber. (Which in this case is `null`, since it's the root.)
We could consider in the future trying to gracefully exit from certain
types of root errors without leaving React in an inconsistent state. For
example, we should be able to gracefully exit from errors thrown in the
begin phase. For now, I'm treating it like an internal invariant and
immediately exiting.
* Add comment
* Split recent passive effects changes into 2 flags
Separate flags can now be used to opt passive effects into:
1) Deferring destroy functions on unmount to subsequent passive effects flush
2) Running all destroy functions (for all fibers) before create functions
This allows us to test the less risky feature (2) separately from the more risky one.
* deferPassiveEffectCleanupDuringUnmount is ignored unless runAllPassiveEffectDestroysBeforeCreates is true
* Build both stable and experimental WWW builds
* Flip already experimental WWW flags to true
* Remove FB-specific internals from modern FB builds
We think we're not going to need these.
* Disable classic features in modern WWW builds
* Disable legacy ReactDOM API for modern WWW build
* Don’t include user timing in prod
* Fix bad copy paste and add missing flags to test renderer
* Add testing WWW feature flag file
We need it because WWW has a different meaning of experimental now.
* Flush all passive destroy fns before calling create fns
Previously we only flushed destroy functions for a single fiber.
The reason this is important is that interleaving destroy/create effects between sibling components might cause components to interfere with each other (e.g. a destroy function in one component may unintentionally override a ref value set by a create function in another component).
This PR builds on top of the recently added deferPassiveEffectCleanupDuringUnmount kill switch to separate passive effects flushing into two separate phases (similar to layout effects).
* Change passive effect flushing to use arrays instead of lists
This change offers a small advantage over the way we did things previous: it continues invoking destroy functions even after a previous one errored.
The mock Scheduler that we use in our tests has its own fake timer
implementation. The `unstable_advanceTime` method advances the timeline.
Currently, a call to `unstable_advanceTime` will also flush any pending
expired work. But that's not how it works in the browser: when a timer
fires, the corresponding task is added to the Scheduler queue. However,
we will still wait until the next message event before flushing it.
This commit changes `unstable_advanceTime` to more closely resemble the
browser behavior, by removing the automatic flushing of expired work.
```js
// Before this commit
Scheduler.unstable_advanceTime(ms);
// Equivalent behavior after this commit
Scheduler.unstable_advanceTime(ms);
Scheduler.unstable_flushExpired();
```
The general principle is to prefer separate APIs for scheduling tasks
and flushing them.
This change does not affect any public APIs. `unstable_advanceTime` is
only used by our own test suite. It is not used by `act`.
However, we may need to update tests in www, like Relay's.
This PR introduces adds `react/testing` and `react-dom/testing`.
- changes infra to generate these builds
- exports act on ReactDOM in these testing builds
- uses the new test builds in fixtures/dom
In the next PR -
- I'll use the new builds for all our own tests
- I'll replace usages of TestUtils.act with ReactDOM.act.
* Simplify Continuous Hydration Targets
Let's use a constant priority for this. This helps us avoid restarting
a render when switching targets and simplifies the model.
The downside is that now we're not down-prioritizing the previous hover
target. However, we think that's ok because it'll only do one level too
much and then stop.
* Add test meant to show why it's tricky to merge both hydration levels
Having both levels co-exist works. However, if we deprioritize hydration
using a single level, we might deprioritize the wrong thing.
This adds a test that catches it if we ever try a naive deprioritization
in the future.
It also tests that we don't down-prioritize if we're changing the hover
in the middle of doing continuous priority work.
* Flush useEffect clean up functions in the passive effects phase
This is a change in behavior that may cause broken product code, so it has been added behind a killswitch (deferPassiveEffectCleanupDuringUnmount)
* Avoid scheduling unnecessary callbacks for cleanup effects
Updated enqueuePendingPassiveEffectDestroyFn() to check rootDoesHavePassiveEffects before scheduling a new callback. This way we'll only schedule (at most) one.
* Updated newly added test for added clarity.
* Cleaned up hooks effect tags
We previously used separate Mount* and Unmount* tags to track hooks work for each phase (snapshot, mutation, layout, and passive). This was somewhat complicated to trace through and there were man tag types we never even used (e.g. UnmountLayout, MountMutation, UnmountSnapshot). In addition to this, it left passive and layout hooks looking the same after renders without changed dependencies, which meant we were unable to reliably defer passive effect destroy functions until after the commit phase.
This commit reduces the effect tag types to only include Layout and Passive and differentiates between work and no-work with an HasEffect flag.
* Disabled deferred passive effects flushing in OSS builds for now
* Split up unmount and mount effects list traversal
This reverts commit 01974a867c.
* Failing test: Expiring a partially completed tree
We should not throw out a partially completed tree if it expires in the
middle of rendering. We should finish the rest of the tree without
yielding, then finish any remaining expired levels in a single batch.
* Check if there's a partial tree before restarting
If a partial render expires, we should stay in the concurrent path
(performConcurrentWorkOnRoot); we'll stop yielding, but the rest of the
behavior remains the same.
We will only revert to the sync path (performSyncWorkOnRoot) when
starting on a new level.
This approach prevents partially completed concurrent work from
being discarded.
* New test: retry after error during expired render
* Failing test: Expiring a partially completed tree
We should not throw out a partially completed tree if it expires in the
middle of rendering. We should finish the rest of the tree without
yielding, then finish any remaining expired levels in a single batch.
* Check if there's a partial tree before restarting
If a partial render expires, we should stay in the concurrent path
(performConcurrentWorkOnRoot); we'll stop yielding, but the rest of the
behavior remains the same.
We will only revert to the sync path (performSyncWorkOnRoot) when
starting on a new level.
This approach prevents partially completed concurrent work from
being discarded.
* New test: retry after error during expired render
When owner and self are different for string refs, we can't easily convert them to callback refs. This PR adds a warning for string refs when owner and self are different to tell users to manually update these refs.
* Remove renderPhaseUpdates Map
Follow up to #17484, which was reverted due to a bug found in www.
* Failing test: Dropped updates
When resetting render phase updates after a throw, we should only clear
the pending queue of hooks that were already processed.
* Fix non-render-phase updates being dropped
Detects if a queue has been processed by whether the hook was cloned.
If we change the implementation to an array instead of a list, we'll
need some other mechanism to determine whether the hook was processed.
* Regression test: startTransition in render phase
useTransition uses the state hook as part of its implementation, so we
need to fork it in the dispatcher used for re-renders, too.
* Update Flow to 0.84
* Fix violations
* Use inexact object syntax in files from fbsource
* Fix warning extraction to use a modern parser
* Codemod inexact objects to new syntax
* Tighten types that can be exact
* Revert unintentional formatting changes from codemod
stateNode is not one of the fields that gets copied over by createWorkInProgress.
This is bad for persistent mode since it's not the same instance.
In HostComponent we happened to always end up transferring the current stateNode when we bail.
However, we didn't do this for HostText.
Moves the unit testing library for events into the `packages` directory so it can more easily be used in tests for other react packages, and mirrored internally to help with testing of event hooks we prototype in www.
Introduces a state machine around pointer events to produce more accurate mock
touch events. This allows multi-touch unit tests to be written entirely in
terms of mock pointer interactions, while producing the expected
'changedTouches', 'targetTouches', and 'touches' fields for mock touch events.
* Add feature flags
* Add Chunk type and constructor
* Wire up Chunk support in the reconciler
* Update reconciler to reconcile Chunks against the render method
This allows the query and args to be updated.
* Drop the ref. Chunks cannot have refs anyway.
* Add Chunk checks in more missing cases
* Rename secondArg
* Add test and fix lazy chunks
Not really a supported use case but for consistency I guess.
* Fix fragment test
* Favor fallthrough switch instead of case statements for work tags
Currently we're inconsistently handling tags that are only relevant
for certain flags. We should throw if the tag is not part of the built
feature flags. This should also mean that the case statements can be
eliminated.
We can achieve this effect by putting the invariant outside of the switch
and always early return in the switch. We already do this in beginWork.
This PR makes this consistent in other places.
* Fail if fundamental/scope tags are discovered without the flag on
* Replace all warning/lowPriWarning with console calls
* Replace console.warn/error with a custom wrapper at build time
* Fail the build for console.error/warn() where we can't read the stack
* Render phase updates can now be extracted from the pending queue
* Use a custom dispatcher for the second render pass
* Discard render phase updates if component throws
When aborting a render, we also need to throw out render phase updates.
Remove the updates from the queues so they do not persist to the next
render. We already did a single pass through the whole list of hooks, so
we know that any pending updates must have been dispatched during the
render phase. The ones that were dispatched before we started rendering
were already transferred to the current hook's queue.
* prep for codemod
* prep warnings
* rename lint rules
* codemod for ifs
* shim www functions
* Handle more cases in the transform
* Thanks De Morgan
* Run the codemod
* Delete the transform
* Fix up confusing conditions manually
* Fix up www shims to match expected API
* Also check for low-pri warning in the lint rule
* Refactor Update Queues to Fix Rebasing Bug
Fixes a bug related to rebasing updates. Once an update has committed,
it should never un-commit, even if interrupted by a higher priority
update. The fix includes a refactor of how update queues work.
This commit is a combination of two PRs:
- #17483 by @sebmarkbage refactors the hook update queue
- #17510 by @acdlite refactors the class and root update queue
Landing one without the other would cause state updates to sometimes be
inconsistent across components, so I've combined them into a single
commit in case they need to be reverted.
Co-authored-by: Sebastian Markbåge <sema@fb.com>
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Initialize update queue object on mount
Instead of lazily initializing update queue objects on the first update,
class and host root queues are created on mount. This simplifies the
logic for appending new updates and matches what we do for hooks.
Reverts b617db3d96.
Found some bugs when attempting to land in www. Reverting to fix master.
I'll land again *after* the change successfully land downstream.
Fixes a bug related to rebasing updates. Once an update has committed,
it should never un-commit, even if interrupted by a higher priority
update. The fix includes a refactor of how update queues work.
This commit is a combination of two PRs:
- #17483 by @sebmarkbage refactors the hook update queue
- #17510 by @acdlite refactors the class and root update queue
Landing one without the other would cause state updates to sometimes be
inconsistent across components, so I've combined them into a single
commit in case they need to be reverted.
Co-authored-by: Sebastian Markbåge <sema@fb.com>
Co-authored-by: Andrew Clark <git@andrewclark.io>
* Replace Babel plugin with an ESLint plugin
* Fix ESLint rule violations
* Move shared conditions higher
* Test formatting nits
* Tweak ESLint rule
* Bugfix: inside else branch, 'if' tests are not satisfactory
* Use a stricter check for exactly if (__DEV__)
This makes it easier to see what's going on and matches dominant style in the codebase.
* Fix remaining files after stricter check
Dan Abramov