OSSForks/react
1
0
Fork 0
mirror of https://github.com/zebrajr/react.git synced 2026-01-15 12:15:22 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Remove wrong return pointer warning

Browse Source 
I'm about to refactor part of the commit phase to use recursion instead
of iteration. As part of that change, we will no longer assign the
`return` pointer when traversing into a subtree. So I'm disabling
the internal warning that fires if the return pointer is not consistent
with the parent during the commit phase.

I had originally added this warning to help prevent mistakes when
traversing the tree iteratively, but since we're intentionally switching
to recursion instead, we don't need it.
This commit is contained in:
Andrew Clark
2022-04-06 23:01:07 -04:00
parent 8dcedba15a
commit ea7b2ec289
3 changed files with 24 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js vendored
View File

@@ -153,51 +153,6 @@ function resolveMostRecentTextCache(text) {
const resolveText = resolveMostRecentTextCache;
// Don't feel too guilty if you have to delete this test.
// @gate dfsEffectsRefactor
// @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 act(async () => {
root.render(<App text="A" />);
});
spyOnDev(console, 'error');
await 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.',
);
});
// @gate enableCache
// @gate enableSuspenseList
test('regression (#20932): return pointer is correct before entering deleted tree', async () => {

41
packages/react-reconciler/src/ReactFiberCommitWork.new.js
View File

@@ -360,7 +360,7 @@ function commitBeforeMutationEffects_begin() {
(fiber.subtreeFlags & BeforeMutationMask) !== NoFlags &&
child !== null
) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitBeforeMutationEffects_complete();
@@ -382,7 +382,7 @@ function commitBeforeMutationEffects_complete() {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2185,7 +2185,7 @@ function commitMutationEffects_begin(root: FiberRoot, lanes: Lanes) {
const child = fiber.child;
if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitMutationEffects_complete(root, lanes);
@@ -2207,7 +2207,7 @@ function commitMutationEffects_complete(root: FiberRoot, lanes: Lanes) {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2425,7 +2425,7 @@ function commitLayoutEffects_begin(
}
if ((fiber.subtreeFlags & LayoutMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
firstChild.return = fiber;
nextEffect = firstChild;
} else {
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
@@ -2459,7 +2459,7 @@ function commitLayoutMountEffects_complete(
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2628,7 +2628,7 @@ function commitPassiveMountEffects_begin(
const fiber = nextEffect;
const firstChild = fiber.child;
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
firstChild.return = fiber;
nextEffect = firstChild;
} else {
commitPassiveMountEffects_complete(subtreeRoot, root, committedLanes);
@@ -2662,7 +2662,7 @@ function commitPassiveMountEffects_complete(
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2849,7 +2849,7 @@ function commitPassiveUnmountEffects_begin() {
}
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitPassiveUnmountEffects_complete();
@@ -2868,7 +2868,7 @@ function commitPassiveUnmountEffects_complete() {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2923,7 +2923,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
// TODO: Only traverse subtree if it has a PassiveStatic flag. (But, if we
// do this, still need to handle `deletedTreeCleanUpLevel` correctly.)
if (child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
@@ -2961,7 +2961,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
}
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, returnFiber);
sibling.return = returnFiber;
nextEffect = sibling;
return;
}
@@ -3039,23 +3039,6 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
}
}
let didWarnWrongReturnPointer = false;
function ensureCorrectReturnPointer(fiber, expectedReturnFiber) {
if (__DEV__) {
if (!didWarnWrongReturnPointer && fiber.return !== expectedReturnFiber) {
didWarnWrongReturnPointer = true;
console.error(
'Internal React error: Return pointer is inconsistent ' +
'with parent.',
);
}
}
// TODO: Remove this assignment once we're confident that it won't break
// anything, by checking the warning logs for the above invariant
fiber.return = expectedReturnFiber;
}
// TODO: Reuse reappearLayoutEffects traversal here?
function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableStrictEffects) {

41
packages/react-reconciler/src/ReactFiberCommitWork.old.js
View File

@@ -360,7 +360,7 @@ function commitBeforeMutationEffects_begin() {
(fiber.subtreeFlags & BeforeMutationMask) !== NoFlags &&
child !== null
) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitBeforeMutationEffects_complete();
@@ -382,7 +382,7 @@ function commitBeforeMutationEffects_complete() {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2185,7 +2185,7 @@ function commitMutationEffects_begin(root: FiberRoot, lanes: Lanes) {
const child = fiber.child;
if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitMutationEffects_complete(root, lanes);
@@ -2207,7 +2207,7 @@ function commitMutationEffects_complete(root: FiberRoot, lanes: Lanes) {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2425,7 +2425,7 @@ function commitLayoutEffects_begin(
}
if ((fiber.subtreeFlags & LayoutMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
firstChild.return = fiber;
nextEffect = firstChild;
} else {
commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
@@ -2459,7 +2459,7 @@ function commitLayoutMountEffects_complete(
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2628,7 +2628,7 @@ function commitPassiveMountEffects_begin(
const fiber = nextEffect;
const firstChild = fiber.child;
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && firstChild !== null) {
ensureCorrectReturnPointer(firstChild, fiber);
firstChild.return = fiber;
nextEffect = firstChild;
} else {
commitPassiveMountEffects_complete(subtreeRoot, root, committedLanes);
@@ -2662,7 +2662,7 @@ function commitPassiveMountEffects_complete(
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2849,7 +2849,7 @@ function commitPassiveUnmountEffects_begin() {
}
if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitPassiveUnmountEffects_complete();
@@ -2868,7 +2868,7 @@ function commitPassiveUnmountEffects_complete() {
const sibling = fiber.sibling;
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, fiber.return);
sibling.return = fiber.return;
nextEffect = sibling;
return;
}
@@ -2923,7 +2923,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_begin(
// TODO: Only traverse subtree if it has a PassiveStatic flag. (But, if we
// do this, still need to handle `deletedTreeCleanUpLevel` correctly.)
if (child !== null) {
ensureCorrectReturnPointer(child, fiber);
child.return = fiber;
nextEffect = child;
} else {
commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
@@ -2961,7 +2961,7 @@ function commitPassiveUnmountEffectsInsideOfDeletedTree_complete(
}
if (sibling !== null) {
ensureCorrectReturnPointer(sibling, returnFiber);
sibling.return = returnFiber;
nextEffect = sibling;
return;
}
@@ -3039,23 +3039,6 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
}
}
let didWarnWrongReturnPointer = false;
function ensureCorrectReturnPointer(fiber, expectedReturnFiber) {
if (__DEV__) {
if (!didWarnWrongReturnPointer && fiber.return !== expectedReturnFiber) {
didWarnWrongReturnPointer = true;
console.error(
'Internal React error: Return pointer is inconsistent ' +
'with parent.',
);
}
}
// TODO: Remove this assignment once we're confident that it won't break
// anything, by checking the warning logs for the above invariant
fiber.return = expectedReturnFiber;
}
// TODO: Reuse reappearLayoutEffects traversal here?
function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableStrictEffects) {