Add warning if return pointer is inconsistent (#20219)
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.
Andrew Clark committed
Nov 11, 2020 at 09:06 UTC
15df051c94533c70b065d51c41cf9b6b2f1dc6a6
3 files changed
+93
packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js
new
+61
@@ -0,0 +1,61 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @emails react-core
8
+ */
9
+
10
+let React;
11
+let ReactNoop;
12
+
13
+beforeEach(() => {
14
+ React = require('react');
15
+ ReactNoop = require('react-noop-renderer');
16
+});
17
+
18
+// Don't feel too guilty if you have to delete this test.
19
+// @gate new
20
+// @gate __DEV__
21
+test('warns in DEV if return pointer is inconsistent', async () => {
22
+ const {useRef, useLayoutEffect} = React;
23
+
24
+ let ref = null;
25
+ function App({text}) {
26
+ ref = useRef(null);
27
+ return (
28
+ <>
29
+ <Sibling text={text} />
30
+ <div ref={ref}>{text}</div>
31
+ </>
32
+ );
33
+ }
34
+
35
+ function Sibling({text}) {
36
+ useLayoutEffect(() => {
37
+ if (text === 'B') {
38
+ // Mutate the return pointer of the div to point to the wrong alternate.
39
+ // This simulates the most common type of return pointer inconsistency.
40
+ const current = ref.current.fiber;
41
+ const workInProgress = current.alternate;
42
+ workInProgress.return = current.return;
43
+ }
44
+ }, [text]);
45
+ return null;
46
+ }
47
+
48
+ const root = ReactNoop.createRoot();
49
+ await ReactNoop.act(async () => {
50
+ root.render(<App text="A" />);
51
+ });
52
+
53
+ spyOnDev(console, 'error');
54
+ await ReactNoop.act(async () => {
55
+ root.render(<App text="B" />);
56
+ });
57
+ expect(console.error.calls.count()).toBe(1);
58
+ expect(console.error.calls.argsFor(0)[0]).toMatch(
59
+ 'Internal React error: Return pointer is inconsistent with parent.',
60
+ );
61
+});
packages/react-noop-renderer/src/createReactNoop.js
+5
@@ -275,6 +275,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
275
props: Props,
276
rootContainerInstance: Container,
277
hostContext: HostContext,
278
+ internalInstanceHandle: Object,
279
): Instance {
280
if (type === 'errorInCompletePhase') {
281
throw new Error('Error in host config.');
@@ -300,6 +301,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
301
value: inst.context,
302
enumerable: false,
303
});
304
+ Object.defineProperty(inst, 'fiber', {
305
+ value: internalInstanceHandle,
306
+ enumerable: false,
307
+ });
308
return inst;
309
},
310
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+27
@@ -463,6 +463,7 @@ function iterativelyCommitBeforeMutationEffects_begin() {
463
(fiber.subtreeFlags & BeforeMutationMask) !== NoFlags &&
464
child !== null
465
) {
466
+ warnIfWrongReturnPointer(fiber, child);
467
nextEffect = child;
468
} else {
469
iterativelyCommitBeforeMutationEffects_complete();
@@ -496,6 +497,7 @@ function iterativelyCommitBeforeMutationEffects_complete() {
497
498
const sibling = fiber.sibling;
499
if (sibling !== null) {
500
+ warnIfWrongReturnPointer(fiber.return, sibling);
501
nextEffect = sibling;
502
return;
503
}
@@ -713,6 +715,7 @@ function iterativelyCommitMutationEffects_begin(
715
716
const child = fiber.child;
717
if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
718
+ warnIfWrongReturnPointer(fiber, child);
719
nextEffect = child;
720
} else {
721
iterativelyCommitMutationEffects_complete(root, renderPriorityLevel);
@@ -751,6 +754,7 @@ function iterativelyCommitMutationEffects_complete(
754
755
const sibling = fiber.sibling;
756
if (sibling !== null) {
757
+ warnIfWrongReturnPointer(fiber.return, sibling);
758
nextEffect = sibling;
759
return;
760
}
@@ -1172,12 +1176,14 @@ function iterativelyCommitLayoutEffects_begin(
1176
}
1177
const sibling = finishedWork.sibling;
1178
if (sibling !== null) {
1179
+ warnIfWrongReturnPointer(finishedWork.return, sibling);
1180
nextEffect = sibling;
1181
} else {
1182
nextEffect = finishedWork.return;
1183
iterativelyCommitLayoutEffects_complete(subtreeRoot, finishedRoot);
1184
}
1185
} else {
1186
+ warnIfWrongReturnPointer(finishedWork, firstChild);
1187
nextEffect = firstChild;
1188
}
1189
} else {
@@ -1224,6 +1230,7 @@ function iterativelyCommitLayoutEffects_complete(
1230
1231
const sibling = fiber.sibling;
1232
if (sibling !== null) {
1233
+ warnIfWrongReturnPointer(fiber.return, sibling);
1234
nextEffect = sibling;
1235
return;
1236
}
@@ -1757,12 +1764,14 @@ function iterativelyCommitPassiveMountEffects_begin(
1764
}
1765
const sibling = fiber.sibling;
1766
if (sibling !== null) {
1767
+ warnIfWrongReturnPointer(fiber.return, sibling);
1768
nextEffect = sibling;
1769
} else {
1770
nextEffect = fiber.return;
1771
iterativelyCommitPassiveMountEffects_complete(subtreeRoot, root);
1772
}
1773
} else {
1774
+ warnIfWrongReturnPointer(fiber, firstChild);
1775
nextEffect = firstChild;
1776
}
1777
} else {
@@ -1808,6 +1817,7 @@ function iterativelyCommitPassiveMountEffects_complete(
1817
1818
const sibling = fiber.sibling;
1819
if (sibling !== null) {
1820
+ warnIfWrongReturnPointer(fiber.return, sibling);
1821
nextEffect = sibling;
1822
return;
1823
}
@@ -1886,6 +1896,7 @@ function iterativelyCommitPassiveUnmountEffects_begin() {
1896
}
1897
1898
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
1899
+ warnIfWrongReturnPointer(fiber, child);
1900
nextEffect = child;
1901
} else {
1902
iterativelyCommitPassiveUnmountEffects_complete();
@@ -1904,6 +1915,7 @@ function iterativelyCommitPassiveUnmountEffects_complete() {
1915
1916
const sibling = fiber.sibling;
1917
if (sibling !== null) {
1918
+ warnIfWrongReturnPointer(fiber.return, sibling);
1919
nextEffect = sibling;
1920
return;
1921
}
@@ -1941,6 +1953,7 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_begin(
1953
const fiber = nextEffect;
1954
const child = fiber.child;
1955
if ((fiber.subtreeFlags & PassiveStatic) !== NoFlags && child !== null) {
1956
+ warnIfWrongReturnPointer(fiber, child);
1957
nextEffect = child;
1958
} else {
1959
iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
@@ -1968,6 +1981,7 @@ function iterativelyCommitPassiveUnmountEffectsInsideOfDeletedTree_complete(
1981
1982
const sibling = fiber.sibling;
1983
if (sibling !== null) {
1984
+ warnIfWrongReturnPointer(fiber.return, sibling);
1985
nextEffect = sibling;
1986
return;
1987
}
@@ -3178,3 +3192,16 @@ function invokeEffectsInDev(
3192
}
3193
}
3194
}
3195
+
3196
+let didWarnWrongReturnPointer = false;
3197
+function warnIfWrongReturnPointer(returnFiber, child) {
3198
+ if (__DEV__) {
3199
+ if (!didWarnWrongReturnPointer && child.return !== returnFiber) {
3200
+ didWarnWrongReturnPointer = true;
3201
+ console.error(
3202
+ 'Internal React error: Return pointer is inconsistent ' +
3203
+ 'with parent.',
3204
+ );
3205
+ }
3206
+ }
3207
+}