Skip to content

Commit

Permalink
Move legacy hidden API to new internal Fiber type (#18782)
Browse files Browse the repository at this point in the history
* Unhide Suspense trees without entanglement

When a Suspense boundary is in its fallback state, you cannot switch
back to the main content without also finishing any updates inside the
tree that might have been skipped. That would be a form of tearing.

Before we fixed this in #18411, the way this bug manifested was that a
boundary was suspended by an update that originated from a child
component (as opposed to props from a parent). While the fallback was
showing, it received another update, this time at high priority. React
would render the high priority update without also including the
original update. That would cause the fallback to switch back to the
main content, since the update that caused the tree to suspend was no
longer part of the render. But then, React would immediately try to
render the original update, which would again suspend and show the
fallback, leading to a momentary flicker in the UI.

The approach added in #18411 is, when receiving a high priority update
to a Suspense tree that's in its fallback state is to bail out, keep
showing the fallback and finish the update in the rest of the tree.
After that commits, render again at the original priority. Because low
priority expiration times are inclusive of higher priority expiration
times, this ensures that all the updates are committed together.

The new approach in this commit is to turn `renderExpirationTime` into a
context-like value that lives on the stack. Then, when unhiding the
Suspense boundary, we can push a new `renderExpirationTime` that is
inclusive of both the high pri update and the original update that
suspended. Then the boundary can be unblocked in a single render pass.

An advantage of the old approach is that by deferring the work of
unhiding, there's less work to do in the high priority update.

The key advantage of the new approach is that it solves the consistency
problem without having to entangle the entire root.

* Create internal LegacyHidden type

This only exists so we can clean up the internal implementation of
`<div hidden={isHidden} />`, which is not a stable feature. The goal
is to move everything to the new Offscreen type instead. However,
Offscreen has different semantics, so before we can remove the legacy
API, we have to migrate our internal usage at Facebook. So we'll need
to maintain both temporarily.

In this initial commit, I've only added the type. It's not used
anywhere. The next step is to use it to implement `hidden`.

* Use LegacyHidden to implement old hidden API

If a host component receives a `hidden` prop, we wrap its children in
an Offscreen fiber. This is similar to what we do for Suspense children.

The LegacyHidden type happens to share the same implementation as the
new Offscreen type, for now, but using separate types allows us to fork
the behavior later when we implement our planned changes to the
Offscreen API.

There are two subtle semantic changes here. One is that the children of
the host component will have their visibility toggled using the same
mechanism we use for Offscreen and Suspense: find the nearest host node
children and give them a style of `display: none`. We didn't used to do
this in the old API, because the `hidden` DOM attribute on the parent
already hides them. So with this change, we're actually "overhiding" the
children. I considered addressing this, but I figure I'll leave it as-is
in case we want to expose the LegacyHidden component type temporarily
to ease migration of Facebook's internal callers to the Offscreen type.

The other subtle semantic change is that, because of the extra fiber
that wraps around the children, this pattern will cause the children
to lose state:

```js
return isHidden ? <div hidden={true} /> : <div />;
```

The reason is that I didn't want to wrap every single host component
in an extra fiber. So I only wrap them if a `hidden` prop exists. In
the above example, that means the children are conditionally wrapped
in an extra fiber, so they don't line up during reconciliation, so
they get remounted every time `isHidden` changes.

The fix is to rewrite to:

```js
return <div hidden={isHidden} />;
```

I don't anticipate this will be a problem at Facebook, especially since
we're only supposed to use `hidden` via a userspace wrapper component.
(And since the bad pattern isn't very React-y, anyway.)

Again, the eventual goal is to delete this completely and replace it
with Offscreen.
  • Loading branch information
acdlite committed May 1, 2020
1 parent ac533fd commit 914b57b
Show file tree
Hide file tree
Showing 11 changed files with 328 additions and 145 deletions.
27 changes: 27 additions & 0 deletions packages/react-reconciler/src/ReactFiber.new.js
Expand Up @@ -56,6 +56,7 @@ import {
ScopeComponent,
Block,
OffscreenComponent,
LegacyHiddenComponent,
} from './ReactWorkTags';
import getComponentName from 'shared/getComponentName';

Expand Down Expand Up @@ -90,6 +91,7 @@ import {
REACT_SCOPE_TYPE,
REACT_BLOCK_TYPE,
REACT_OFFSCREEN_TYPE,
REACT_LEGACY_HIDDEN_TYPE,
} from 'shared/ReactSymbols';

export type {Fiber};
Expand Down Expand Up @@ -521,6 +523,13 @@ export function createFiberFromTypeAndProps(
expirationTime,
key,
);
case REACT_LEGACY_HIDDEN_TYPE:
return createFiberFromLegacyHidden(
pendingProps,
mode,
expirationTime,
key,
);
default: {
if (typeof type === 'object' && type !== null) {
switch (type.$$typeof) {
Expand Down Expand Up @@ -756,6 +765,24 @@ export function createFiberFromOffscreen(
return fiber;
}

export function createFiberFromLegacyHidden(
pendingProps: OffscreenProps,
mode: TypeOfMode,
expirationTime: ExpirationTimeOpaque,
key: null | string,
) {
const fiber = createFiber(LegacyHiddenComponent, pendingProps, key, mode);
// TODO: The LegacyHidden fiber shouldn't have a type. It has a tag.
// This needs to be fixed in getComponentName so that it relies on the tag
// instead.
if (__DEV__) {
fiber.type = REACT_LEGACY_HIDDEN_TYPE;
}
fiber.elementType = REACT_LEGACY_HIDDEN_TYPE;
fiber.expirationTime_opaque = expirationTime;
return fiber;
}

export function createFiberFromText(
content: string,
mode: TypeOfMode,
Expand Down

0 comments on commit 914b57b

Please sign in to comment.