A passive effect's cleanup function may throw after an unmount. In that event, React sometimes threw an uncaught runtime error trying to access a property on a null stateNode field. This commit fixes that (and adds a regression test).
* Get current time from performance.now in non-DOM environments
* Use local references to native APIs for Date and Performance
* Refactored to read globals directly
* Test: Don't unmount siblings of deleted node
Adds a failing regression test. Will fix in the next commit.
* Refactor to accept deleted fiber, not child list
A deleted fiber is passed to
flushPassiveUnmountEffectsInsideOfDeletedTree, but the code is written
as if it accepts the first node of a child list. This is likely because
the function was based on similar functions like
`flushPassiveUnmountEffects`, which do accept a child list.
Unfortunately, types don't help here because we use the first node
in the list to represent the whole list, so in both cases, the type
is Fiber.
Might be worth changing the other functions to also accept individual
fibers instead of a child list, to help avoid confusion.
* Add layout effect to regression test, just in case
Creates new subtree tag, PassiveStatic, that represents whether a
tree contains any passive effect hooks.
It corresponds to the PassiveStatic effect tag, which represents the
same concept for an individual fiber.
This allows us to remove the PassiveStatic effect tag from PassiveMask.
Its presence was causing us to schedule a passive effect phase callback
on every render, instead of only when something changed. That's now
fixed; this is reflected in the SchedulerProfiler tests.
(The naming is getting really confusing. Need to do some bikeshedding.)
* setCurrentFiber per fiber, instead of per effect
* Re-use safelyCallDestroy
Part of the code in flushPassiveUnmountEffects is a duplicate of the
code used for unmounting layout effects. I did some minor refactoring to
so we could use the same function in both places.
Closure will inline anyway so it doesn't affect code size or
performance, just maintainability.
* Don't check HookHasEffect during deletion
We don't need to check HookHasEffect during a deletion; all effects are
unmounted.
So we also don't have to set HookHasEffect during a deletion, either.
This allows us to remove the last remaining passive effect logic from
the synchronous layout phase.
* Remove opaque event type
* Rename type and merge files
* Use literals where we have Flow coverage
* Flowify some plugins
* Remove constants except necessary ones
The root fiber doesn't have a parent from which we can read the
`subtreeTag`, so we need to check its `effectTag` directly.
The root fiber previously did not have any pending passive effects,
but it does now that deleted fibers are cleaned up in the passive phase.
This allows us to remove a `schedulePassiveEffectCallback` call from the
synchronous unmount path.
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
Saves us from having to set a flag on `current` during the layout phase.
Could result in some redundant traversal, since PassiveStatic includes
effects that don't need clean-up. But it's worth it to remove the work
from the layout phase.
While I was editing this, I also re-arranged it so that we check the
`effectTag` check before we check the `tag`, since the `effectTag` check
is the one that's more likely to fail.
* Adds new `Passive` subtree tag value.
* Adds recursive traversal for passive effects (mounts and unmounts).
* Removes `pendingPassiveHookEffectsMount` and `pendingPassiveHookEffectsUnmount` arrays from work loop.
* Re-adds sibling and child pointer detaching (temporarily removed in previous PR).
* Addresses some minor TODO comments left over from previous PRs.
---
Co-authored-by: Luna Ruan <luna@fb.com>
* Reduce code to necessities
* Switch to postTask API
* Add SchedulerPostTask tests
* Updates from review
* Fix typo from review
* Generate build of unstable_post_task
* Add "unstbale_" prefix to mutable source APIs
* DebugHooks no longer calls useMutableSource() on init
This was causing an observable behavioral difference between experimental DEV and PROD builds.
We don't initialize stack position for other composite hooks (e.g. useDeferredValue, useTransition, useOpaqueIdentifier). If we did, it would cause the same obesrvable behavioral difference.
Tasks with SyncBatchedPriority — used by Blocking Mode — should always
be rendered by the `peformSyncWorkOnRoot` path, not
`performConcurrentWorkOnRoot`.
Currently, they go through the `performConcurrentWorkOnRoot` callback.
Then, we check `didTimeout` to see if the task expired. Since
SyncBatchedPriority translates to ImmediatePriority in the Scheduler,
`didTimeout` is always `true`, so we mark it as expired. Then it exits
and re-enters in the `performSyncWorkOnRoot` path.
Aside from being overly convoluted, we shouldn't rely on Scheduler to
tell us that SyncBatchedPriority work is synchronous. We should handle
that ourselves.
This will allow us to remove the `didTimeout` check. And it further
decouples us from the Scheduler priority, so we can eventually remove
that, too.
The old expiration times implementation used this field to infer when
the priority of a task had changed at a more granular level than a
Scheduler priority level.
Now that we have the LanePriority type, which is React-specific, we no
longer need the `callbackId` field.
Since the Lanes refactor landed, we no longer rely on this anywhere, so
we can remove it.
The `delay` option is still needed by our timer implementation
(setTimeout polyfill). We'll keep the feature, but we'll likely change
how it's exposed once we figure out the proper layering between the
various Scheduler APIs.
Persistent mode needs to clone a parent and add its children if a child has
changed.
We have an optimization in persistent mode where we don't do that if no
child could've changed. If there are no effects scheduled for any child
then there couldn't have been changes.
Instead of checking for this on firstEffect, we now check this on the
children's effectTag and subtreeTags.
This is quite unfortunate because if we could just do this check a little
bit later we would've already gotten it transferred to the completed work's
subtreeTag. Now we have to loop over all the children and if any of them
changed, we have to loop over them again. Doing at least two loops per
parent.
