Add warning if return pointer is inconsistent (#20219)

acdlite · web-flow · commit 15df051c9453 · 2020-11-11T09:06:37.000-06:00
Bugs caused by inconsistent return pointers are tricky to diagnose
because the source of the error is often in a different part of the
codebase from the actual mistake. For example, you might forget to set a
return pointer during the render phase, which later causes a crash in
the commit phase.

This adds a dev-only invariant to the commit phase to check for
inconsistencies. With this in place, we'll hopefully catch return
pointer errors quickly during local development, when we have the most
context for what might have caused it.
diff --git a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js
@@ -0,0 +1,61 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @emails react-core
+ */
+
+let React;
+let ReactNoop;
+
+beforeEach(() => {
+  React = require('react');
+  ReactNoop = require('react-noop-renderer');
+});
+
+// Don't feel too guilty if you have to delete this test.
+// @gate new
+// @gate __DEV__
+test('warns in DEV if return pointer is inconsistent', async () => {
+  const {useRef, useLayoutEffect} = React;
+
+  let ref = null;
+  function App({text}) {
+    ref = useRef(null);
+    return (
+      <>
+        <Sibling text={text} />
+        <div ref={ref}>{text}</div>
+      </>
+    );
+  }
+
+  function Sibling({text}) {
+    useLayoutEffect(() => {
+      if (text === 'B') {
+        // Mutate the return pointer of the div to point to the wrong alternate.
+        // This simulates the most common type of return pointer inconsistency.
+        const current = ref.current.fiber;
+        const workInProgress = current.alternate;
+        workInProgress.return = current.return;
+      }
+    }, [text]);
+    return null;
+  }
+
+  const root = ReactNoop.createRoot();
+  await ReactNoop.act(async () => {
+    root.render(<App text="A" />);
+  });
+
+  spyOnDev(console, 'error');
+  await ReactNoop.act(async () => {
+    root.render(<App text="B" />);
+  });
+  expect(console.error.calls.count()).toBe(1);
+  expect(console.error.calls.argsFor(0)[0]).toMatch(
+    'Internal React error: Return pointer is inconsistent with parent.',
+  );
+});
diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -275,6 +275,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       props: Props,
       rootContainerInstance: Container,
       hostContext: HostContext,
+      internalInstanceHandle: Object,
     ): Instance {
       if (type === 'errorInCompletePhase') {
         throw new Error('Error in host config.');
@@ -300,6 +301,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         value: inst.context,
         enumerable: false,
       });
+      Object.defineProperty(inst, 'fiber', {
+        value: internalInstanceHandle,
+        enumerable: false,
+      });
       return inst;
     },
 
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -463,6 +463,7 @@ function iterativelyCommitBeforeMutationEffects_begin() {
       (fiber.subtreeFlags & BeforeMutationMask) !== NoFlags &&
       child !== null
     ) {
+      warnIfWrongReturnPointer(fiber, child);
       nextEffect = child;
     } else {
       iterativelyCommitBeforeMutationEffects_complete();
@@ -496,6 +497,7 @@ function iterativelyCommitBeforeMutationEffects_complete() {
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -713,6 +715,7 @@ function iterativelyCommitMutationEffects_begin(
 
     const child = fiber.child;
     if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
+      warnIfWrongReturnPointer(fiber, child);
       nextEffect = child;
     } else {
       iterativelyCommitMutationEffects_complete(root, renderPriorityLevel);
@@ -751,6 +754,7 @@ function iterativelyCommitMutationEffects_complete(
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -1172,12 +1176,14 @@ function iterativelyCommitLayoutEffects_begin(
         }
         const sibling = finishedWork.sibling;
         if (sibling !== null) {
+          warnIfWrongReturnPointer(finishedWork.return, sibling);
           nextEffect = sibling;
         } else {
           nextEffect = finishedWork.return;
           iterativelyCommitLayoutEffects_complete(subtreeRoot, finishedRoot);
         }
       } else {
+        warnIfWrongReturnPointer(finishedWork, firstChild);
         nextEffect = firstChild;
       }
     } else {
@@ -1224,6 +1230,7 @@ function iterativelyCommitLayoutEffects_complete(
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -1757,12 +1764,14 @@ function iterativelyCommitPassiveMountEffects_begin(
         }
         const sibling = fiber.sibling;
         if (sibling !== null) {
+          warnIfWrongReturnPointer(fiber.return, sibling);
           nextEffect = sibling;
         } else {
           nextEffect = fiber.return;
           iterativelyCommitPassiveMountEffects_complete(subtreeRoot, root);
         }
       } else {
+        warnIfWrongReturnPointer(fiber, firstChild);
         nextEffect = firstChild;
       }
     } else {
@@ -1808,6 +1817,7 @@ function iterativelyCommitPassiveMountEffects_complete(
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -1886,6 +1896,7 @@ function iterativelyCommitPassiveUnmountEffects_begin() {
     }
 
     if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
+      warnIfWrongReturnPointer(fiber, child);
       nextEffect = child;
     } else {
       iterativelyCommitPassiveUnmountEffects_complete();
@@ -1904,6 +1915,7 @@ function iterativelyCommitPassiveUnmountEffects_complete() {
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -1941,6 +1953,7 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_begin(
     const fiber = nextEffect;
     const child = fiber.child;
     if ((fiber.subtreeFlags & PassiveStatic) !== NoFlags && child !== null) {
+      warnIfWrongReturnPointer(fiber, child);
       nextEffect = child;
     } else {
       iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
@@ -1968,6 +1981,7 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
 
     const sibling = fiber.sibling;
     if (sibling !== null) {
+      warnIfWrongReturnPointer(fiber.return, sibling);
       nextEffect = sibling;
       return;
     }
@@ -3178,3 +3192,16 @@ function invokeEffectsInDev(
     }
   }
 }
+
+let didWarnWrongReturnPointer = false;
+function warnIfWrongReturnPointer(returnFiber, child) {
+  if (__DEV__) {
+    if (!didWarnWrongReturnPointer && child.return !== returnFiber) {
+      didWarnWrongReturnPointer = true;
+      console.error(
+        'Internal React error: Return pointer is inconsistent ' +
+          'with parent.',
+      );
+    }
+  }
+}
