[Fabric] Use container node to toggle the visibility of Offscreen and…

… Suspense trees (facebook#21960)

* Fix type of Offscreen props argument

Fixes an oversight from a previous refactor. The fiber that wraps
a Suspense component's children used to be a Fragment but now it's on
Offscreen fiber, so its props type has changed. There's a special
hydration path where I forgot to update this. This isn't observable
because we don't ever end up rendering this particular fiber (because
the Suspense boundary is in its fallback state) but we should fix it
anyway to avoid a potential regression in the future.

* Extract createOffscreenFromFiber logic

...into a new method called `createWorkInProgressOffscreenFiber`. Just
for symmetry with `updateWorkInProgressOffscreenFiber`. Doesn't change
any behavior.

* [Fabric] Use container node to hide/show tree

This changes how we hide and show the contents of Offscreen boundaries
in the React Fabric renderer (persistent mode), and also Suspense
boundaries which use the same feature.=

The way it used to work was that when a boundary is hidden, in the
complete phase, instead of calling the normal `cloneInstance` method
inside `appendAllChildren`, we would call a forked method called
`cloneHiddenInstance` for each of the nearest host nodes within the
subtree. This design was largely based on how it works in React DOM
(mutation mode), where instead of cloning the nearest host nodes, we
mutate their `style.display` property.

The motivation for doing it this way in React DOM was because there's no
built-in browser API for hiding a collection of DOM nodes without
affecting their layout.

In Fabric, however, there is no such limitation, so we can instead wrap
in an extra host node and apply a hidden style.

The immediate motivation for this change is that Fabric on Android has a
view pooling mechanism for instances that relies on the assumption that
a current Fiber that is cloned and replaced by a new Fiber will never
appear in a future commit. When this assumption is broken, it may cause
crashes. In the current implementation, that can indeed happen when a
node that was previously hidden is toggled back to visible. Although
this change sidesteps the issue, we may introduce in other features in
the future that would benefit from being able to revert back to an older
node without cloning it again, such as animations.

The way I've implemented this is to insert an additional HostComponent
fiber as the child of each OffscreenComponent. The extra fiber is not
ideal — the way I'd prefer to do it is to attach the host instance to
the OffscreenComponent. However, the native Fabric implementation
currently expects a 1:1 correspondence between HostComponents and host
instances, so I've deferred that optimization to a future PR to derisk
fixing the Fabric pooling crash. I left a TODO in the host config with a
description of the remaining steps, but this alone should be sufficient
to unblock.

packages/react-native-renderer/src/ReactFabricHostConfig.js

@@ -7,6 +7,7 @@
  * @flow
  */
 
+import type {ReactNodeList, OffscreenMode} from 'shared/ReactTypes';
 import type {ElementRef} from 'react';
 import type {
   HostComponent,
@@ -301,6 +302,9 @@ export function getChildHostContext(
     type === 'RCTText' ||
     type === 'RCTVirtualText';
 
+  // TODO: If this is an offscreen host container, we should reuse the
+  // parent context.
+
   if (prevIsInAParentText !== isInAParentText) {
     return {isInAParentText};
   } else {
@@ -413,30 +417,32 @@ export function cloneInstance(
   };
 }
 
-export function cloneHiddenInstance(
-  instance: Instance,
-  type: string,
-  props: Props,
-  internalInstanceHandle: Object,
-): Instance {
-  const viewConfig = instance.canonical.viewConfig;
-  const node = instance.node;
-  const updatePayload = create(
-    {style: {display: 'none'}},
-    viewConfig.validAttributes,
-  );
-  return {
-    node: cloneNodeWithNewProps(node, updatePayload),
-    canonical: instance.canonical,
-  };
+// TODO: These two methods should be replaced with `createOffscreenInstance` and
+// `cloneOffscreenInstance`. I did it this way for now because the offscreen
+// instance is stored on an extra HostComponent fiber instead of the
+// OffscreenComponent fiber, and I didn't want to add an extra check to the
+// generic HostComponent path. Instead we should use the OffscreenComponent
+// fiber, but currently Fabric expects a 1:1 correspondence between Fabric
+// instances and host fibers, so I'm leaving this optimization for later once
+// we can confirm this won't break any downstream expectations.
+export function getOffscreenContainerType(): string {
+  return 'RCTView';
 }
 
-export function cloneHiddenTextInstance(
-  instance: Instance,
-  text: string,
-  internalInstanceHandle: Object,
-): TextInstance {
-  throw new Error('Not yet implemented.');
+export function getOffscreenContainerProps(
+  mode: OffscreenMode,
+  children: ReactNodeList,
+): Props {
+  if (mode === 'hidden') {
+    return {
+      children,
+      style: {display: 'none'},
+    };
+  } else {
+    return {
+      children,
+    };
+  }
 }
 
 export function createContainerChildSet(container: Container): ChildSet {

packages/react-noop-renderer/src/createReactNoop.js

@@ -16,7 +16,7 @@
 
 import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
 import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue';
-import type {ReactNodeList} from 'shared/ReactTypes';
+import type {ReactNodeList, OffscreenMode} from 'shared/ReactTypes';
 import type {RootTag} from 'react-reconciler/src/ReactRootTags';
 
 import * as Scheduler from 'scheduler/unstable_mock';
@@ -258,6 +258,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       type: string,
       rootcontainerInstance: Container,
     ) {
+      if (type === 'offscreen') {
+        return parentHostContext;
+      }
       if (type === 'uppercase') {
         return UPPERCASE_CONTEXT;
       }
@@ -539,47 +542,18 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
           container.children = newChildren;
         },
 
-        cloneHiddenInstance(
-          instance: Instance,
-          type: string,
-          props: Props,
-          internalInstanceHandle: Object,
-        ): Instance {
-          const clone = cloneInstance(
-            instance,
-            null,
-            type,
-            props,
-            props,
-            internalInstanceHandle,
-            true,
-            null,
-          );
-          clone.hidden = true;
-          return clone;
+        getOffscreenContainerType(): string {
+          return 'offscreen';
         },
 
-        cloneHiddenTextInstance(
-          instance: TextInstance,
-          text: string,
-          internalInstanceHandle: Object,
-        ): TextInstance {
-          const clone = {
-            text: instance.text,
-            id: instanceCounter++,
-            hidden: true,
-            context: instance.context,
+        getOffscreenContainerProps(
+          mode: OffscreenMode,
+          children: ReactNodeList,
+        ): Props {
+          return {
+            hidden: mode === 'hidden',
+            children,
           };
-          // Hide from unit tests
-          Object.defineProperty(clone, 'id', {
-            value: clone.id,
-            enumerable: false,
-          });
-          Object.defineProperty(clone, 'context', {
-            value: clone.context,
-            enumerable: false,
-          });
-          return clone;
         },
       };
 
@@ -646,20 +620,164 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
 
   function getChildren(root) {
     if (root) {
-      return root.children;
+      return useMutation
+        ? root.children
+        : removeOffscreenContainersFromChildren(root.children, false);
     } else {
       return null;
     }
   }
 
   function getPendingChildren(root) {
     if (root) {
-      return root.pendingChildren;
+      return useMutation
+        ? root.children
+        : removeOffscreenContainersFromChildren(root.pendingChildren, false);
     } else {
       return null;
     }
   }
 
+  function removeOffscreenContainersFromChildren(children, hideNearestNode) {
+    // Mutation mode and persistent mode have different outputs for Offscreen
+    // and Suspense trees. Persistent mode adds an additional host node wrapper,
+    // whereas mutation mode does not.
+    //
+    // This function removes the offscreen host wrappers so that the output is
+    // consistent. If the offscreen node is hidden, it transfers the hiddenness
+    // to the child nodes, to mimic how it works in mutation mode. That way our
+    // tests don't have to fork tree assertions.
+    //
+    // So, it takes a tree that looks like this:
+    //
+    //    <offscreen hidden={true}>
+    //      <span>A</span>
+    //      <span>B</span>
+    //    </offscren>
+    //
+    // And turns it into this:
+    //
+    //   <span hidden={true}>A</span>
+    //   <span hidden={true}>B</span>
+    //
+    // We don't mutate the original tree, but instead return a copy.
+    //
+    // This function is only used by our test assertions, via the `getChildren`
+    // and `getChildrenAsJSX` methods.
+    let didClone = false;
+    const newChildren = [];
+    for (let i = 0; i < children.length; i++) {
+      const child = children[i];
+      const innerChildren = child.children;
+      if (innerChildren !== undefined) {
+        // This is a host instance instance
+        const instance: Instance = (child: any);
+        if (instance.type === 'offscreen') {
+          // This is an offscreen wrapper instance. Remove it from the tree
+          // and recursively return its children, as if it were a fragment.
+          didClone = true;
+          if (instance.text !== null) {
+            // If this offscreen tree contains only text, we replace it with
+            // a text child. Related to `shouldReplaceTextContent` feature.
+            const offscreenTextInstance: TextInstance = {
+              text: instance.text,
+              id: instanceCounter++,
+              hidden: hideNearestNode || instance.hidden,
+              context: instance.context,
+            };
+            // Hide from unit tests
+            Object.defineProperty(offscreenTextInstance, 'id', {
+              value: offscreenTextInstance.id,
+              enumerable: false,
+            });
+            Object.defineProperty(offscreenTextInstance, 'context', {
+              value: offscreenTextInstance.context,
+              enumerable: false,
+            });
+            newChildren.push(offscreenTextInstance);
+          } else {
+            // Skip the offscreen node and replace it with its children
+            const offscreenChildren = removeOffscreenContainersFromChildren(
+              innerChildren,
+              hideNearestNode || instance.hidden,
+            );
+            newChildren.push.apply(newChildren, offscreenChildren);
+          }
+        } else {
+          // This is a regular (non-offscreen) instance. If the nearest
+          // offscreen boundary is hidden, hide this node.
+          const hidden = hideNearestNode ? true : instance.hidden;
+          const clonedChildren = removeOffscreenContainersFromChildren(
+            instance.children,
+            // We never need to hide the children of this node, since if we're
+            // inside a hidden tree, then the hidden style will be applied to
+            // this node.
+            false,
+          );
+          if (
+            clonedChildren === instance.children &&
+            hidden === instance.hidden
+          ) {
+            // No changes. Reuse the original instance without cloning.
+            newChildren.push(instance);
+          } else {
+            didClone = true;
+            const clone: Instance = {
+              id: instance.id,
+              type: instance.type,
+              children: clonedChildren,
+              text: instance.text,
+              prop: instance.prop,
+              hidden: hideNearestNode ? true : instance.hidden,
+              context: instance.context,
+            };
+            Object.defineProperty(clone, 'id', {
+              value: clone.id,
+              enumerable: false,
+            });
+            Object.defineProperty(clone, 'text', {
+              value: clone.text,
+              enumerable: false,
+            });
+            Object.defineProperty(clone, 'context', {
+              value: clone.context,
+              enumerable: false,
+            });
+            newChildren.push(clone);
+          }
+        }
+      } else {
+        // This is a text instance
+        const textInstance: TextInstance = (child: any);
+        if (hideNearestNode) {
+          didClone = true;
+          const clone = {
+            text: textInstance.text,
+            id: textInstance.id,
+            hidden: textInstance.hidden || hideNearestNode,
+            context: textInstance.context,
+          };
+          Object.defineProperty(clone, 'id', {
+            value: clone.id,
+            enumerable: false,
+          });
+          Object.defineProperty(clone, 'context', {
+            value: clone.context,
+            enumerable: false,
+          });
+
+          newChildren.push(clone);
+        } else {
+          newChildren.push(textInstance);
+        }
+      }
+    }
+    // There are some tests that assume reference equality, so preserve it
+    // when possible. Alternatively, we could update the tests to compare the
+    // ids instead.
+    return didClone ? newChildren : children;
+  }
+
   function getChildrenAsJSX(root) {
     const children = childToJSX(getChildren(root), null);
     if (children === null) {

packages/react-reconciler/src/ReactFiber.new.js

@@ -14,7 +14,7 @@ import type {RootTag} from './ReactRootTags';
 import type {WorkTag} from './ReactWorkTags';
 import type {TypeOfMode} from './ReactTypeOfMode';
 import type {Lanes} from './ReactFiberLane.new';
-import type {SuspenseInstance} from './ReactFiberHostConfig';
+import type {SuspenseInstance, Props} from './ReactFiberHostConfig';
 import type {OffscreenProps} from './ReactFiberOffscreenComponent';
 
 import invariant from 'shared/invariant';
@@ -27,6 +27,10 @@ import {
   enableSyncDefaultUpdates,
   allowConcurrentByDefault,
 } from 'shared/ReactFeatureFlags';
+import {
+  supportsPersistence,
+  getOffscreenContainerType,
+} from './ReactFiberHostConfig';
 import {NoFlags, Placement, StaticMask} from './ReactFiberFlags';
 import {ConcurrentRoot} from './ReactRootTags';
 import {
@@ -585,6 +589,25 @@ export function createFiberFromTypeAndProps(
   return fiber;
 }
 
+export function createOffscreenHostContainerFiber(
+  props: Props,
+  fiberMode: TypeOfMode,
+  lanes: Lanes,
+  key: null | string,
+): Fiber {
+  if (supportsPersistence) {
+    const type = getOffscreenContainerType();
+    const fiber = createFiber(HostComponent, props, key, fiberMode);
+    fiber.elementType = type;
+    fiber.type = type;
+    fiber.lanes = lanes;
+    return fiber;
+  } else {
+    // Only implemented in persistent mode
+    invariant(false, 'Not implemented.');
+  }
+}
+
 export function createFiberFromElement(
   element: ReactElement,
   mode: TypeOfMode,