Run persistent mode tests in CI (#15029)

* Add command to run tests in persistent mode

* Convert Suspense fuzz tester to use noop renderer

So we can run it in persistent mode, too.

* Don't mutate stateNode in appendAllChildren

We can't mutate the stateNode in appendAllChildren because the children
could be current.

This is a bit weird because now the child that we append is different
from the one on the fiber stateNode. I think this makes conceptual
sense, but I suspect this likely breaks an assumption in Fabric.

With this approach, we no longer need to clone to unhide the children,
so I removed those host config methods.

Fixes bug surfaced by fuzz tester. (The test case that failed was the
one that's already hard coded.)

* In persistent mode, disable test that reads a ref

Refs behave differently in persistent mode. I added a TODO to write
a persistent mode version of this test.

* Run persistent mode tests in CI

* test-persistent should skip files without noop

If a file doesn't reference react-noop-renderer, we shouldn't bother
running it in persistent mode, since the results will be identical to
the normal test run.

* Remove module constructor from placeholder tests

We don't need this now that we have the ability to run any test file in
either mutation or persistent mode.

* Revert "test-persistent should skip files without noop"

Seb objected to adding shelljs as a dep and I'm too lazy to worry about
Windows support so whatever I'll just revert this.

* Delete duplicate file

package.json

@@ -100,6 +100,7 @@
     "postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js",
     "debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/.bin/jest --config ./scripts/jest/config.source.js --runInBand",
     "test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
+    "test-persistent": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-persistent.js",
     "test-fire": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-fire.js",
     "test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js",
     "test-fire-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source-fire.js",

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

@@ -387,25 +387,6 @@ export function cloneHiddenInstance(
   };
 }
 
-export function cloneUnhiddenInstance(
-  instance: Instance,
-  type: string,
-  props: Props,
-  internalInstanceHandle: Object,
-): Instance {
-  const viewConfig = instance.canonical.viewConfig;
-  const node = instance.node;
-  const updatePayload = diff(
-    {...props, style: [props.style, {display: 'none'}]},
-    props,
-    viewConfig.validAttributes,
-  );
-  return {
-    node: cloneNodeWithNewProps(node, updatePayload),
-    canonical: instance.canonical,
-  };
-}
-
 export function cloneHiddenTextInstance(
   instance: Instance,
   text: string,
@@ -414,14 +395,6 @@ export function cloneHiddenTextInstance(
   throw new Error('Not yet implemented.');
 }
 
-export function cloneUnhiddenTextInstance(
-  instance: Instance,
-  text: string,
-  internalInstanceHandle: Object,
-): TextInstance {
-  throw new Error('Not yet implemented.');
-}
-
 export function createContainerChildSet(container: Container): ChildSet {
   return createChildNodeSet(container);
 }

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

@@ -486,26 +486,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
           return clone;
         },
 
-        cloneUnhiddenInstance(
-          instance: Instance,
-          type: string,
-          props: Props,
-          internalInstanceHandle: Object,
-        ): Instance {
-          const clone = cloneInstance(
-            instance,
-            null,
-            type,
-            props,
-            props,
-            internalInstanceHandle,
-            true,
-            null,
-          );
-          clone.hidden = props.hidden === true;
-          return clone;
-        },
-
         cloneHiddenTextInstance(
           instance: TextInstance,
           text: string,
@@ -528,29 +508,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
           });
           return clone;
         },
-
-        cloneUnhiddenTextInstance(
-          instance: TextInstance,
-          text: string,
-          internalInstanceHandle: Object,
-        ): TextInstance {
-          const clone = {
-            text: instance.text,
-            id: instanceCounter++,
-            hidden: false,
-            context: instance.context,
-          };
-          // Hide from unit tests
-          Object.defineProperty(clone, 'id', {
-            value: clone.id,
-            enumerable: false,
-          });
-          Object.defineProperty(clone, 'context', {
-            value: clone.context,
-            enumerable: false,
-          });
-          return clone;
-        },
       };
 
   const NoopRenderer = reconciler(hostConfig);

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -59,9 +59,7 @@ import {
   supportsPersistence,
   cloneInstance,
   cloneHiddenInstance,
-  cloneUnhiddenInstance,
   cloneHiddenTextInstance,
-  cloneUnhiddenTextInstance,
   createContainerChildSet,
   appendChildToContainerChildSet,
   finalizeContainerChildren,
@@ -209,31 +207,19 @@ if (supportsMutation) {
       // eslint-disable-next-line no-labels
       branches: if (node.tag === HostComponent) {
         let instance = node.stateNode;
-        if (needsVisibilityToggle) {
+        if (needsVisibilityToggle && isHidden) {
+          // This child is inside a timed out tree. Hide it.
           const props = node.memoizedProps;
           const type = node.type;
-          if (isHidden) {
-            // This child is inside a timed out tree. Hide it.
-            instance = cloneHiddenInstance(instance, type, props, node);
-          } else {
-            // This child was previously inside a timed out tree. If it was not
-            // updated during this render, it may need to be unhidden. Clone
-            // again to be sure.
-            instance = cloneUnhiddenInstance(instance, type, props, node);
-          }
-          node.stateNode = instance;
+          instance = cloneHiddenInstance(instance, type, props, node);
         }
         appendInitialChild(parent, instance);
       } else if (node.tag === HostText) {
         let instance = node.stateNode;
-        if (needsVisibilityToggle) {
+        if (needsVisibilityToggle && isHidden) {
+          // This child is inside a timed out tree. Hide it.
           const text = node.memoizedProps;
-          if (isHidden) {
-            instance = cloneHiddenTextInstance(instance, text, node);
-          } else {
-            instance = cloneUnhiddenTextInstance(instance, text, node);
-          }
-          node.stateNode = instance;
+          instance = cloneHiddenTextInstance(instance, text, node);
         }
         appendInitialChild(parent, instance);
       } else if (node.tag === HostPortal) {
@@ -247,15 +233,22 @@ if (supportsMutation) {
           if (newIsHidden) {
             const primaryChildParent = node.child;
             if (primaryChildParent !== null) {
-              appendAllChildren(parent, primaryChildParent, true, newIsHidden);
-              node = primaryChildParent.sibling;
-              continue;
+              if (primaryChildParent.child !== null) {
+                primaryChildParent.child.return = primaryChildParent;
+                appendAllChildren(
+                  parent,
+                  primaryChildParent,
+                  true,
+                  newIsHidden,
+                );
+              }
+              const fallbackChildParent = primaryChildParent.sibling;
+              if (fallbackChildParent !== null) {
+                fallbackChildParent.return = node;
+                node = fallbackChildParent;
+                continue;
+              }
             }
-          } else {
-            const primaryChildParent = node;
-            appendAllChildren(parent, primaryChildParent, true, newIsHidden);
-            // eslint-disable-next-line no-labels
-            break branches;
           }
         }
         if (node.child !== null) {
@@ -299,31 +292,19 @@ if (supportsMutation) {
       // eslint-disable-next-line no-labels
       branches: if (node.tag === HostComponent) {
         let instance = node.stateNode;
-        if (needsVisibilityToggle) {
+        if (needsVisibilityToggle && isHidden) {
+          // This child is inside a timed out tree. Hide it.
           const props = node.memoizedProps;
           const type = node.type;
-          if (isHidden) {
-            // This child is inside a timed out tree. Hide it.
-            instance = cloneHiddenInstance(instance, type, props, node);
-          } else {
-            // This child was previously inside a timed out tree. If it was not
-            // updated during this render, it may need to be unhidden. Clone
-            // again to be sure.
-            instance = cloneUnhiddenInstance(instance, type, props, node);
-          }
-          node.stateNode = instance;
+          instance = cloneHiddenInstance(instance, type, props, node);
         }
         appendChildToContainerChildSet(containerChildSet, instance);
       } else if (node.tag === HostText) {
         let instance = node.stateNode;
-        if (needsVisibilityToggle) {
+        if (needsVisibilityToggle && isHidden) {
+          // This child is inside a timed out tree. Hide it.
           const text = node.memoizedProps;
-          if (isHidden) {
-            instance = cloneHiddenTextInstance(instance, text, node);
-          } else {
-            instance = cloneUnhiddenTextInstance(instance, text, node);
-          }
-          node.stateNode = instance;
+          instance = cloneHiddenTextInstance(instance, text, node);
         }
         appendChildToContainerChildSet(containerChildSet, instance);
       } else if (node.tag === HostPortal) {
@@ -337,25 +318,22 @@ if (supportsMutation) {
           if (newIsHidden) {
             const primaryChildParent = node.child;
             if (primaryChildParent !== null) {
-              appendAllChildrenToContainer(
-                containerChildSet,
-                primaryChildParent,
-                true,
-                newIsHidden,
-              );
-              node = primaryChildParent.sibling;
-              continue;
+              if (primaryChildParent.child !== null) {
+                primaryChildParent.child.return = primaryChildParent;
+                appendAllChildrenToContainer(
+                  containerChildSet,
+                  primaryChildParent,
+                  true,
+                  newIsHidden,
+                );
+              }
+              const fallbackChildParent = primaryChildParent.sibling;
+              if (fallbackChildParent !== null) {
+                fallbackChildParent.return = node;
+                node = fallbackChildParent;
+                continue;
+              }
             }
-          } else {
-            const primaryChildParent = node;
-            appendAllChildrenToContainer(
-              containerChildSet,
-              primaryChildParent,
-              true,
-              newIsHidden,
-            );
-            // eslint-disable-next-line no-labels
-            break branches;
           }
         }
         if (node.child !== null) {
@@ -714,11 +692,23 @@ function completeWork(
         }
       }
 
-      if (nextDidTimeout || prevDidTimeout) {
-        // If the children are hidden, or if they were previous hidden, schedule
-        // an effect to toggle their visibility. This is also used to attach a
-        // retry listener to the promise.
-        workInProgress.effectTag |= Update;
+      if (supportsPersistence) {
+        if (nextDidTimeout) {
+          // If this boundary just timed out, schedule an effect to attach a
+          // retry listener to the proimse. This flag is also used to hide the
+          // primary children.
+          workInProgress.effectTag |= Update;
+        }
+      }
+      if (supportsMutation) {
+        if (nextDidTimeout || prevDidTimeout) {
+          // If this boundary just timed out, schedule an effect to attach a
+          // retry listener to the proimse. This flag is also used to hide the
+          // primary children. In mutation mode, we also need the flag to
+          // *unhide* children that were previously hidden, so check if the
+          // is currently timed out, too.
+          workInProgress.effectTag |= Update;
+        }
       }
       break;
     }

packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js

@@ -1,6 +1,6 @@
 let React;
 let Suspense;
-let ReactTestRenderer;
+let ReactNoop;
 let Scheduler;
 let ReactFeatureFlags;
 let Random;
@@ -26,7 +26,7 @@ describe('ReactSuspenseFuzz', () => {
     ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
     React = require('react');
     Suspense = React.Suspense;
-    ReactTestRenderer = require('react-test-renderer');
+    ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');
     Random = require('random-seed');
   });
@@ -143,54 +143,49 @@ describe('ReactSuspenseFuzz', () => {
       return resolvedText;
     }
 
-    function renderToRoot(
-      root,
-      children,
-      {shouldSuspend} = {shouldSuspend: true},
-    ) {
-      root.update(
-        <ShouldSuspendContext.Provider value={shouldSuspend}>
-          {children}
-        </ShouldSuspendContext.Provider>,
-      );
+    function resolveAllTasks() {
       Scheduler.unstable_flushWithoutYielding();
-
       let elapsedTime = 0;
       while (pendingTasks && pendingTasks.size > 0) {
         if ((elapsedTime += 1000) > 1000000) {
           throw new Error('Something did not resolve properly.');
         }
-        ReactTestRenderer.act(() => jest.advanceTimersByTime(1000));
+        ReactNoop.act(() => jest.advanceTimersByTime(1000));
         Scheduler.unstable_flushWithoutYielding();
       }
-
-      return root.toJSON();
     }
 
     function testResolvedOutput(unwrappedChildren) {
       const children = (
         <Suspense fallback="Loading...">{unwrappedChildren}</Suspense>
       );
 
-      const expectedRoot = ReactTestRenderer.create(null);
-      const expectedOutput = renderToRoot(expectedRoot, children, {
-        shouldSuspend: false,
-      });
-      expectedRoot.unmount();
+      resetCache();
+      ReactNoop.renderToRootWithID(
+        <ShouldSuspendContext.Provider value={false}>
+          {children}
+        </ShouldSuspendContext.Provider>,
+        'expected',
+      );
+      resolveAllTasks();
+      const expectedOutput = ReactNoop.getChildrenAsJSX('expected');
+      ReactNoop.renderToRootWithID(null, 'expected');
+      Scheduler.unstable_flushWithoutYielding();
 
       resetCache();
-      const syncRoot = ReactTestRenderer.create(null);
-      const syncOutput = renderToRoot(syncRoot, children);
+      ReactNoop.renderLegacySyncRoot(children);
+      resolveAllTasks();
+      const syncOutput = ReactNoop.getChildrenAsJSX();
       expect(syncOutput).toEqual(expectedOutput);
-      syncRoot.unmount();
+      ReactNoop.renderLegacySyncRoot(null);
 
       resetCache();
-      const concurrentRoot = ReactTestRenderer.create(null, {
-        unstable_isConcurrent: true,
-      });
-      const concurrentOutput = renderToRoot(concurrentRoot, children);
+      ReactNoop.renderToRootWithID(children, 'concurrent');
+      Scheduler.unstable_flushWithoutYielding();
+      resolveAllTasks();
+      const concurrentOutput = ReactNoop.getChildrenAsJSX('concurrent');
       expect(concurrentOutput).toEqual(expectedOutput);
-      concurrentRoot.unmount();
+      ReactNoop.renderToRootWithID(null, 'concurrent');
       Scheduler.unstable_flushWithoutYielding();
     }