@samitouri / QOS-React / commits / 34600f4fad

Refactor "reappear" logic into its own traversal (#21898)

When a Suspense boundary switches to its fallback state — or similarly, when an Offscreen boundary switches from visible to hidden — we unmount all its layout effects. When it resolves — or when Offscreen switches back to visible — we mount them again. This "reappearing" logic currently happens in the same commit phase traversal where we perform normal layout effects. I've changed it so that the "reappear" logic happens in its own recurisve traversal that is separate from the commit phase one. In the next step, I will do the same for the "disappear" logic that currently lives in the `hideOrUnhideAllChildren` function. There are a few reasons to model it this way, related to future Offscreen features that we have planned. For example, we intend to provide an imperative API to "appear" and "reappear" all the effects within an Offscreen boundary. This API would be called from outside the commit phase, during an arbitrary event. Which means it can't rely on the regular commit phase — it's not part of a commit. This isn't the only motivation but it illustrates why the separation makes sense.

Andrew Clark committed Jul 16, 2021 at 18:05 UTC 34600f4fadf526028a21c94030510fbed20e4665
3 files changed +458 -368
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+224 -173
@@ -620,131 +620,144 @@ function commitLayoutEffectOnFiber(
620 case FunctionComponent:
621 case ForwardRef:
622 case SimpleMemoComponent: {
623 - // At this point layout effects have already been destroyed (during mutation phase).
624 - // This is done to prevent sibling component effects from interfering with each other,
625 - // e.g. a destroy function in one component should never override a ref set
626 - // by a create function in another component during the same commit.
623 if (
628 - enableProfilerTimer &&
629 - enableProfilerCommitHooks &&
630 - finishedWork.mode & ProfileMode
624 + !enableSuspenseLayoutEffectSemantics ||
625 + !offscreenSubtreeWasHidden
626 ) {
632 - try {
633 - startLayoutEffectTimer();
627 + // At this point layout effects have already been destroyed (during mutation phase).
628 + // This is done to prevent sibling component effects from interfering with each other,
629 + // e.g. a destroy function in one component should never override a ref set
630 + // by a create function in another component during the same commit.
631 + if (
632 + enableProfilerTimer &&
633 + enableProfilerCommitHooks &&
634 + finishedWork.mode & ProfileMode
635 + ) {
636 + try {
637 + startLayoutEffectTimer();
638 + commitHookEffectListMount(
639 + HookLayout | HookHasEffect,
640 + finishedWork,
641 + );
642 + } finally {
643 + recordLayoutEffectDuration(finishedWork);
644 + }
645 + } else {
646 commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
635 - } finally {
636 - recordLayoutEffectDuration(finishedWork);
647 }
638 - } else {
639 - commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
648 }
649 break;
650 }
651 case ClassComponent: {
652 const instance = finishedWork.stateNode;
653 if (finishedWork.flags & Update) {
646 - if (current === null) {
647 - // We could update instance props and state here,
648 - // but instead we rely on them being set during last render.
649 - // TODO: revisit this when we implement resuming.
650 - if (__DEV__) {
654 + if (!offscreenSubtreeWasHidden) {
655 + if (current === null) {
656 + // We could update instance props and state here,
657 + // but instead we rely on them being set during last render.
658 + // TODO: revisit this when we implement resuming.
659 + if (__DEV__) {
660 + if (
661 + finishedWork.type === finishedWork.elementType &&
662 + !didWarnAboutReassigningProps
663 + ) {
664 + if (instance.props !== finishedWork.memoizedProps) {
665 + console.error(
666 + 'Expected %s props to match memoized props before ' +
667 + 'componentDidMount. ' +
668 + 'This might either be because of a bug in React, or because ' +
669 + 'a component reassigns its own `this.props`. ' +
670 + 'Please file an issue.',
671 + getComponentNameFromFiber(finishedWork) || 'instance',
672 + );
673 + }
674 + if (instance.state !== finishedWork.memoizedState) {
675 + console.error(
676 + 'Expected %s state to match memoized state before ' +
677 + 'componentDidMount. ' +
678 + 'This might either be because of a bug in React, or because ' +
679 + 'a component reassigns its own `this.state`. ' +
680 + 'Please file an issue.',
681 + getComponentNameFromFiber(finishedWork) || 'instance',
682 + );
683 + }
684 + }
685 + }
686 if (
652 - finishedWork.type === finishedWork.elementType &&
653 - !didWarnAboutReassigningProps
687 + enableProfilerTimer &&
688 + enableProfilerCommitHooks &&
689 + finishedWork.mode & ProfileMode
690 ) {
655 - if (instance.props !== finishedWork.memoizedProps) {
656 - console.error(
657 - 'Expected %s props to match memoized props before ' +
658 - 'componentDidMount. ' +
659 - 'This might either be because of a bug in React, or because ' +
660 - 'a component reassigns its own `this.props`. ' +
661 - 'Please file an issue.',
662 - getComponentNameFromFiber(finishedWork) || 'instance',
663 - );
664 - }
665 - if (instance.state !== finishedWork.memoizedState) {
666 - console.error(
667 - 'Expected %s state to match memoized state before ' +
668 - 'componentDidMount. ' +
669 - 'This might either be because of a bug in React, or because ' +
670 - 'a component reassigns its own `this.state`. ' +
671 - 'Please file an issue.',
672 - getComponentNameFromFiber(finishedWork) || 'instance',
673 - );
691 + try {
692 + startLayoutEffectTimer();
693 + instance.componentDidMount();
694 + } finally {
695 + recordLayoutEffectDuration(finishedWork);
696 }
675 - }
676 - }
677 - if (
678 - enableProfilerTimer &&
679 - enableProfilerCommitHooks &&
680 - finishedWork.mode & ProfileMode
681 - ) {
682 - try {
683 - startLayoutEffectTimer();
697 + } else {
698 instance.componentDidMount();
685 - } finally {
686 - recordLayoutEffectDuration(finishedWork);
699 }
700 } else {
689 - instance.componentDidMount();
690 - }
691 - } else {
692 - const prevProps =
693 - finishedWork.elementType === finishedWork.type
694 - ? current.memoizedProps
695 - : resolveDefaultProps(finishedWork.type, current.memoizedProps);
696 - const prevState = current.memoizedState;
697 - // We could update instance props and state here,
698 - // but instead we rely on them being set during last render.
699 - // TODO: revisit this when we implement resuming.
700 - if (__DEV__) {
701 + const prevProps =
702 + finishedWork.elementType === finishedWork.type
703 + ? current.memoizedProps
704 + : resolveDefaultProps(
705 + finishedWork.type,
706 + current.memoizedProps,
707 + );
708 + const prevState = current.memoizedState;
709 + // We could update instance props and state here,
710 + // but instead we rely on them being set during last render.
711 + // TODO: revisit this when we implement resuming.
712 + if (__DEV__) {
713 + if (
714 + finishedWork.type === finishedWork.elementType &&
715 + !didWarnAboutReassigningProps
716 + ) {
717 + if (instance.props !== finishedWork.memoizedProps) {
718 + console.error(
719 + 'Expected %s props to match memoized props before ' +
720 + 'componentDidUpdate. ' +
721 + 'This might either be because of a bug in React, or because ' +
722 + 'a component reassigns its own `this.props`. ' +
723 + 'Please file an issue.',
724 + getComponentNameFromFiber(finishedWork) || 'instance',
725 + );
726 + }
727 + if (instance.state !== finishedWork.memoizedState) {
728 + console.error(
729 + 'Expected %s state to match memoized state before ' +
730 + 'componentDidUpdate. ' +
731 + 'This might either be because of a bug in React, or because ' +
732 + 'a component reassigns its own `this.state`. ' +
733 + 'Please file an issue.',
734 + getComponentNameFromFiber(finishedWork) || 'instance',
735 + );
736 + }
737 + }
738 + }
739 if (
702 - finishedWork.type === finishedWork.elementType &&
703 - !didWarnAboutReassigningProps
740 + enableProfilerTimer &&
741 + enableProfilerCommitHooks &&
742 + finishedWork.mode & ProfileMode
743 ) {
705 - if (instance.props !== finishedWork.memoizedProps) {
706 - console.error(
707 - 'Expected %s props to match memoized props before ' +
708 - 'componentDidUpdate. ' +
709 - 'This might either be because of a bug in React, or because ' +
710 - 'a component reassigns its own `this.props`. ' +
711 - 'Please file an issue.',
712 - getComponentNameFromFiber(finishedWork) || 'instance',
713 - );
714 - }
715 - if (instance.state !== finishedWork.memoizedState) {
716 - console.error(
717 - 'Expected %s state to match memoized state before ' +
718 - 'componentDidUpdate. ' +
719 - 'This might either be because of a bug in React, or because ' +
720 - 'a component reassigns its own `this.state`. ' +
721 - 'Please file an issue.',
722 - getComponentNameFromFiber(finishedWork) || 'instance',
744 + try {
745 + startLayoutEffectTimer();
746 + instance.componentDidUpdate(
747 + prevProps,
748 + prevState,
749 + instance.__reactInternalSnapshotBeforeUpdate,
750 );
751 + } finally {
752 + recordLayoutEffectDuration(finishedWork);
753 }
725 - }
726 - }
727 - if (
728 - enableProfilerTimer &&
729 - enableProfilerCommitHooks &&
730 - finishedWork.mode & ProfileMode
731 - ) {
732 - try {
733 - startLayoutEffectTimer();
754 + } else {
755 instance.componentDidUpdate(
756 prevProps,
757 prevState,
758 instance.__reactInternalSnapshotBeforeUpdate,
759 );
739 - } finally {
740 - recordLayoutEffectDuration(finishedWork);
760 }
742 - } else {
743 - instance.componentDidUpdate(
744 - prevProps,
745 - prevState,
746 - instance.__reactInternalSnapshotBeforeUpdate,
747 - );
761 }
762 }
763 }
@@ -913,15 +926,55 @@ function commitLayoutEffectOnFiber(
926 }
927 }
928
916 - if (enableScopeAPI) {
917 - // TODO: This is a temporary solution that allowed us to transition away
918 - // from React Flare on www.
919 - if (finishedWork.flags & Ref && finishedWork.tag !== ScopeComponent) {
920 - commitAttachRef(finishedWork);
929 + if (!enableSuspenseLayoutEffectSemantics || !offscreenSubtreeWasHidden) {
930 + if (enableScopeAPI) {
931 + // TODO: This is a temporary solution that allowed us to transition away
932 + // from React Flare on www.
933 + if (finishedWork.flags & Ref && finishedWork.tag !== ScopeComponent) {
934 + commitAttachRef(finishedWork);
935 + }
936 + } else {
937 + if (finishedWork.flags & Ref) {
938 + commitAttachRef(finishedWork);
939 + }
940 }
922 - } else {
923 - if (finishedWork.flags & Ref) {
924 - commitAttachRef(finishedWork);
941 + }
942 +}
943 +
944 +function reappearLayoutEffectsOnFiber(node: Fiber) {
945 + // Turn on layout effects in a tree that previously disappeared.
946 + // TODO (Offscreen) Check: flags & LayoutStatic
947 + switch (node.tag) {
948 + case FunctionComponent:
949 + case ForwardRef:
950 + case SimpleMemoComponent: {
951 + if (
952 + enableProfilerTimer &&
953 + enableProfilerCommitHooks &&
954 + node.mode & ProfileMode
955 + ) {
956 + try {
957 + startLayoutEffectTimer();
958 + safelyCallCommitHookLayoutEffectListMount(node, node.return);
959 + } finally {
960 + recordLayoutEffectDuration(node);
961 + }
962 + } else {
963 + safelyCallCommitHookLayoutEffectListMount(node, node.return);
964 + }
965 + break;
966 + }
967 + case ClassComponent: {
968 + const instance = node.stateNode;
969 + if (typeof instance.componentDidMount === 'function') {
970 + safelyCallComponentDidMount(node, node.return, instance);
971 + }
972 + safelyAttachRef(node, node.return);
973 + break;
974 + }
975 + case HostComponent: {
976 + safelyAttachRef(node, node.return);
977 + break;
978 }
979 }
980 }
@@ -2255,6 +2308,14 @@ function commitLayoutEffects_begin(
2308 // Traverse the Offscreen subtree with the current Offscreen as the root.
2309 offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2310 offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2311 +
2312 + if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
2313 + // This is the root of a reappearing boundary. Turn its layout effects
2314 + // back on.
2315 + nextEffect = fiber;
2316 + reappearLayoutEffects_begin(fiber);
2317 + }
2318 +
2319 let child = firstChild;
2320 while (child !== null) {
2321 nextEffect = child;
@@ -2280,21 +2341,6 @@ function commitLayoutEffects_begin(
2341 ensureCorrectReturnPointer(firstChild, fiber);
2342 nextEffect = firstChild;
2343 } else {
2283 - if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
2284 - const visibilityChanged =
2285 - !offscreenSubtreeIsHidden && offscreenSubtreeWasHidden;
2286 -
2287 - // TODO (Offscreen) Also check: subtreeFlags & LayoutStatic
2288 - if (visibilityChanged && firstChild !== null) {
2289 - // We've just shown or hidden a Offscreen tree that contains layout effects.
2290 - // We only enter this code path for subtrees that are updated,
2291 - // because newly mounted ones would pass the LayoutMask check above.
2292 - ensureCorrectReturnPointer(firstChild, fiber);
2293 - nextEffect = firstChild;
2294 - continue;
2295 - }
2296 - }
2297 -
2344 commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
2345 }
2346 }
@@ -2305,59 +2351,9 @@ function commitLayoutMountEffects_complete(
2351 root: FiberRoot,
2352 committedLanes: Lanes,
2353 ) {
2308 - // Suspense layout effects semantics don't change for legacy roots.
2309 - const isModernRoot = (subtreeRoot.mode & ConcurrentMode) !== NoMode;
2310 -
2354 while (nextEffect !== null) {
2355 const fiber = nextEffect;
2313 -
2314 - if (
2315 - enableSuspenseLayoutEffectSemantics &&
2316 - isModernRoot &&
2317 - offscreenSubtreeWasHidden &&
2318 - !offscreenSubtreeIsHidden
2319 - ) {
2320 - // Inside of an Offscreen subtree that changed visibility during this commit.
2321 - // If this subtree was hidden, layout effects will have already been destroyed (during mutation phase)
2322 - // but if it was just shown, we need to (re)create the effects now.
2323 - // TODO (Offscreen) Check: flags & LayoutStatic
2324 - switch (fiber.tag) {
2325 - case FunctionComponent:
2326 - case ForwardRef:
2327 - case SimpleMemoComponent: {
2328 - if (
2329 - enableProfilerTimer &&
2330 - enableProfilerCommitHooks &&
2331 - fiber.mode & ProfileMode
2332 - ) {
2333 - try {
2334 - startLayoutEffectTimer();
2335 - safelyCallCommitHookLayoutEffectListMount(fiber, fiber.return);
2336 - } finally {
2337 - recordLayoutEffectDuration(fiber);
2338 - }
2339 - } else {
2340 - safelyCallCommitHookLayoutEffectListMount(fiber, fiber.return);
2341 - }
2342 - break;
2343 - }
2344 - case ClassComponent: {
2345 - const instance = fiber.stateNode;
2346 - if (typeof instance.componentDidMount === 'function') {
2347 - safelyCallComponentDidMount(fiber, fiber.return, instance);
2348 - }
2349 - break;
2350 - }
2351 - }
2352 -
2353 - // TODO (Offscreen) Check flags & RefStatic
2354 - switch (fiber.tag) {
2355 - case ClassComponent:
2356 - case HostComponent:
2357 - safelyAttachRef(fiber, fiber.return);
2358 - break;
2359 - }
2360 - } else if ((fiber.flags & LayoutMask) !== NoFlags) {
2356 + if ((fiber.flags & LayoutMask) !== NoFlags) {
2357 const current = fiber.alternate;
2358 setCurrentDebugFiberInDEV(fiber);
2359 try {
@@ -2385,6 +2381,60 @@ function commitLayoutMountEffects_complete(
2381 }
2382 }
2383
2384 +function reappearLayoutEffects_begin(subtreeRoot: Fiber) {
2385 + while (nextEffect !== null) {
2386 + const fiber = nextEffect;
2387 + const firstChild = fiber.child;
2388 +
2389 + if (fiber.tag === OffscreenComponent) {
2390 + const isHidden = fiber.memoizedState !== null;
2391 + if (isHidden) {
2392 + // Nested Offscreen tree is still hidden. Don't re-appear its effects.
2393 + reappearLayoutEffects_complete(subtreeRoot);
2394 + continue;
2395 + }
2396 + }
2397 +
2398 + // TODO (Offscreen) Check: subtreeFlags & LayoutStatic
2399 + if (firstChild !== null) {
2400 + ensureCorrectReturnPointer(firstChild, fiber);
2401 + nextEffect = firstChild;
2402 + } else {
2403 + reappearLayoutEffects_complete(subtreeRoot);
2404 + }
2405 + }
2406 +}
2407 +
2408 +function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
2409 + while (nextEffect !== null) {
2410 + const fiber = nextEffect;
2411 +
2412 + // TODO (Offscreen) Check: flags & LayoutStatic
2413 + setCurrentDebugFiberInDEV(fiber);
2414 + try {
2415 + reappearLayoutEffectsOnFiber(fiber);
2416 + } catch (error) {
2417 + reportUncaughtErrorInDEV(error);
2418 + captureCommitPhaseError(fiber, fiber.return, error);
2419 + }
2420 + resetCurrentDebugFiberInDEV();
2421 +
2422 + if (fiber === subtreeRoot) {
2423 + nextEffect = null;
2424 + return;
2425 + }
2426 +
2427 + const sibling = fiber.sibling;
2428 + if (sibling !== null) {
2429 + ensureCorrectReturnPointer(sibling, fiber.return);
2430 + nextEffect = sibling;
2431 + return;
2432 + }
2433 +
2434 + nextEffect = fiber.return;
2435 + }
2436 +}
2437 +
2438 export function commitPassiveMountEffects(
2439 root: FiberRoot,
2440 finishedWork: Fiber,
@@ -2689,6 +2739,7 @@ function ensureCorrectReturnPointer(fiber, expectedReturnFiber) {
2739 fiber.return = expectedReturnFiber;
2740 }
2741
2742 +// TODO: Reuse reappearLayoutEffects traversal here?
2743 function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2744 if (__DEV__ && enableStrictEffects) {
2745 // We don't need to re-check StrictEffectsMode here.
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+224 -173
@@ -620,131 +620,144 @@ function commitLayoutEffectOnFiber(
620 case FunctionComponent:
621 case ForwardRef:
622 case SimpleMemoComponent: {
623 - // At this point layout effects have already been destroyed (during mutation phase).
624 - // This is done to prevent sibling component effects from interfering with each other,
625 - // e.g. a destroy function in one component should never override a ref set
626 - // by a create function in another component during the same commit.
623 if (
628 - enableProfilerTimer &&
629 - enableProfilerCommitHooks &&
630 - finishedWork.mode & ProfileMode
624 + !enableSuspenseLayoutEffectSemantics ||
625 + !offscreenSubtreeWasHidden
626 ) {
632 - try {
633 - startLayoutEffectTimer();
627 + // At this point layout effects have already been destroyed (during mutation phase).
628 + // This is done to prevent sibling component effects from interfering with each other,
629 + // e.g. a destroy function in one component should never override a ref set
630 + // by a create function in another component during the same commit.
631 + if (
632 + enableProfilerTimer &&
633 + enableProfilerCommitHooks &&
634 + finishedWork.mode & ProfileMode
635 + ) {
636 + try {
637 + startLayoutEffectTimer();
638 + commitHookEffectListMount(
639 + HookLayout | HookHasEffect,
640 + finishedWork,
641 + );
642 + } finally {
643 + recordLayoutEffectDuration(finishedWork);
644 + }
645 + } else {
646 commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
635 - } finally {
636 - recordLayoutEffectDuration(finishedWork);
647 }
638 - } else {
639 - commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
648 }
649 break;
650 }
651 case ClassComponent: {
652 const instance = finishedWork.stateNode;
653 if (finishedWork.flags & Update) {
646 - if (current === null) {
647 - // We could update instance props and state here,
648 - // but instead we rely on them being set during last render.
649 - // TODO: revisit this when we implement resuming.
650 - if (__DEV__) {
654 + if (!offscreenSubtreeWasHidden) {
655 + if (current === null) {
656 + // We could update instance props and state here,
657 + // but instead we rely on them being set during last render.
658 + // TODO: revisit this when we implement resuming.
659 + if (__DEV__) {
660 + if (
661 + finishedWork.type === finishedWork.elementType &&
662 + !didWarnAboutReassigningProps
663 + ) {
664 + if (instance.props !== finishedWork.memoizedProps) {
665 + console.error(
666 + 'Expected %s props to match memoized props before ' +
667 + 'componentDidMount. ' +
668 + 'This might either be because of a bug in React, or because ' +
669 + 'a component reassigns its own `this.props`. ' +
670 + 'Please file an issue.',
671 + getComponentNameFromFiber(finishedWork) || 'instance',
672 + );
673 + }
674 + if (instance.state !== finishedWork.memoizedState) {
675 + console.error(
676 + 'Expected %s state to match memoized state before ' +
677 + 'componentDidMount. ' +
678 + 'This might either be because of a bug in React, or because ' +
679 + 'a component reassigns its own `this.state`. ' +
680 + 'Please file an issue.',
681 + getComponentNameFromFiber(finishedWork) || 'instance',
682 + );
683 + }
684 + }
685 + }
686 if (
652 - finishedWork.type === finishedWork.elementType &&
653 - !didWarnAboutReassigningProps
687 + enableProfilerTimer &&
688 + enableProfilerCommitHooks &&
689 + finishedWork.mode & ProfileMode
690 ) {
655 - if (instance.props !== finishedWork.memoizedProps) {
656 - console.error(
657 - 'Expected %s props to match memoized props before ' +
658 - 'componentDidMount. ' +
659 - 'This might either be because of a bug in React, or because ' +
660 - 'a component reassigns its own `this.props`. ' +
661 - 'Please file an issue.',
662 - getComponentNameFromFiber(finishedWork) || 'instance',
663 - );
664 - }
665 - if (instance.state !== finishedWork.memoizedState) {
666 - console.error(
667 - 'Expected %s state to match memoized state before ' +
668 - 'componentDidMount. ' +
669 - 'This might either be because of a bug in React, or because ' +
670 - 'a component reassigns its own `this.state`. ' +
671 - 'Please file an issue.',
672 - getComponentNameFromFiber(finishedWork) || 'instance',
673 - );
691 + try {
692 + startLayoutEffectTimer();
693 + instance.componentDidMount();
694 + } finally {
695 + recordLayoutEffectDuration(finishedWork);
696 }
675 - }
676 - }
677 - if (
678 - enableProfilerTimer &&
679 - enableProfilerCommitHooks &&
680 - finishedWork.mode & ProfileMode
681 - ) {
682 - try {
683 - startLayoutEffectTimer();
697 + } else {
698 instance.componentDidMount();
685 - } finally {
686 - recordLayoutEffectDuration(finishedWork);
699 }
700 } else {
689 - instance.componentDidMount();
690 - }
691 - } else {
692 - const prevProps =
693 - finishedWork.elementType === finishedWork.type
694 - ? current.memoizedProps
695 - : resolveDefaultProps(finishedWork.type, current.memoizedProps);
696 - const prevState = current.memoizedState;
697 - // We could update instance props and state here,
698 - // but instead we rely on them being set during last render.
699 - // TODO: revisit this when we implement resuming.
700 - if (__DEV__) {
701 + const prevProps =
702 + finishedWork.elementType === finishedWork.type
703 + ? current.memoizedProps
704 + : resolveDefaultProps(
705 + finishedWork.type,
706 + current.memoizedProps,
707 + );
708 + const prevState = current.memoizedState;
709 + // We could update instance props and state here,
710 + // but instead we rely on them being set during last render.
711 + // TODO: revisit this when we implement resuming.
712 + if (__DEV__) {
713 + if (
714 + finishedWork.type === finishedWork.elementType &&
715 + !didWarnAboutReassigningProps
716 + ) {
717 + if (instance.props !== finishedWork.memoizedProps) {
718 + console.error(
719 + 'Expected %s props to match memoized props before ' +
720 + 'componentDidUpdate. ' +
721 + 'This might either be because of a bug in React, or because ' +
722 + 'a component reassigns its own `this.props`. ' +
723 + 'Please file an issue.',
724 + getComponentNameFromFiber(finishedWork) || 'instance',
725 + );
726 + }
727 + if (instance.state !== finishedWork.memoizedState) {
728 + console.error(
729 + 'Expected %s state to match memoized state before ' +
730 + 'componentDidUpdate. ' +
731 + 'This might either be because of a bug in React, or because ' +
732 + 'a component reassigns its own `this.state`. ' +
733 + 'Please file an issue.',
734 + getComponentNameFromFiber(finishedWork) || 'instance',
735 + );
736 + }
737 + }
738 + }
739 if (
702 - finishedWork.type === finishedWork.elementType &&
703 - !didWarnAboutReassigningProps
740 + enableProfilerTimer &&
741 + enableProfilerCommitHooks &&
742 + finishedWork.mode & ProfileMode
743 ) {
705 - if (instance.props !== finishedWork.memoizedProps) {
706 - console.error(
707 - 'Expected %s props to match memoized props before ' +
708 - 'componentDidUpdate. ' +
709 - 'This might either be because of a bug in React, or because ' +
710 - 'a component reassigns its own `this.props`. ' +
711 - 'Please file an issue.',
712 - getComponentNameFromFiber(finishedWork) || 'instance',
713 - );
714 - }
715 - if (instance.state !== finishedWork.memoizedState) {
716 - console.error(
717 - 'Expected %s state to match memoized state before ' +
718 - 'componentDidUpdate. ' +
719 - 'This might either be because of a bug in React, or because ' +
720 - 'a component reassigns its own `this.state`. ' +
721 - 'Please file an issue.',
722 - getComponentNameFromFiber(finishedWork) || 'instance',
744 + try {
745 + startLayoutEffectTimer();
746 + instance.componentDidUpdate(
747 + prevProps,
748 + prevState,
749 + instance.__reactInternalSnapshotBeforeUpdate,
750 );
751 + } finally {
752 + recordLayoutEffectDuration(finishedWork);
753 }
725 - }
726 - }
727 - if (
728 - enableProfilerTimer &&
729 - enableProfilerCommitHooks &&
730 - finishedWork.mode & ProfileMode
731 - ) {
732 - try {
733 - startLayoutEffectTimer();
754 + } else {
755 instance.componentDidUpdate(
756 prevProps,
757 prevState,
758 instance.__reactInternalSnapshotBeforeUpdate,
759 );
739 - } finally {
740 - recordLayoutEffectDuration(finishedWork);
760 }
742 - } else {
743 - instance.componentDidUpdate(
744 - prevProps,
745 - prevState,
746 - instance.__reactInternalSnapshotBeforeUpdate,
747 - );
761 }
762 }
763 }
@@ -913,15 +926,55 @@ function commitLayoutEffectOnFiber(
926 }
927 }
928
916 - if (enableScopeAPI) {
917 - // TODO: This is a temporary solution that allowed us to transition away
918 - // from React Flare on www.
919 - if (finishedWork.flags & Ref && finishedWork.tag !== ScopeComponent) {
920 - commitAttachRef(finishedWork);
929 + if (!enableSuspenseLayoutEffectSemantics || !offscreenSubtreeWasHidden) {
930 + if (enableScopeAPI) {
931 + // TODO: This is a temporary solution that allowed us to transition away
932 + // from React Flare on www.
933 + if (finishedWork.flags & Ref && finishedWork.tag !== ScopeComponent) {
934 + commitAttachRef(finishedWork);
935 + }
936 + } else {
937 + if (finishedWork.flags & Ref) {
938 + commitAttachRef(finishedWork);
939 + }
940 }
922 - } else {
923 - if (finishedWork.flags & Ref) {
924 - commitAttachRef(finishedWork);
941 + }
942 +}
943 +
944 +function reappearLayoutEffectsOnFiber(node: Fiber) {
945 + // Turn on layout effects in a tree that previously disappeared.
946 + // TODO (Offscreen) Check: flags & LayoutStatic
947 + switch (node.tag) {
948 + case FunctionComponent:
949 + case ForwardRef:
950 + case SimpleMemoComponent: {
951 + if (
952 + enableProfilerTimer &&
953 + enableProfilerCommitHooks &&
954 + node.mode & ProfileMode
955 + ) {
956 + try {
957 + startLayoutEffectTimer();
958 + safelyCallCommitHookLayoutEffectListMount(node, node.return);
959 + } finally {
960 + recordLayoutEffectDuration(node);
961 + }
962 + } else {
963 + safelyCallCommitHookLayoutEffectListMount(node, node.return);
964 + }
965 + break;
966 + }
967 + case ClassComponent: {
968 + const instance = node.stateNode;
969 + if (typeof instance.componentDidMount === 'function') {
970 + safelyCallComponentDidMount(node, node.return, instance);
971 + }
972 + safelyAttachRef(node, node.return);
973 + break;
974 + }
975 + case HostComponent: {
976 + safelyAttachRef(node, node.return);
977 + break;
978 }
979 }
980 }
@@ -2255,6 +2308,14 @@ function commitLayoutEffects_begin(
2308 // Traverse the Offscreen subtree with the current Offscreen as the root.
2309 offscreenSubtreeIsHidden = newOffscreenSubtreeIsHidden;
2310 offscreenSubtreeWasHidden = newOffscreenSubtreeWasHidden;
2311 +
2312 + if (offscreenSubtreeWasHidden && !prevOffscreenSubtreeWasHidden) {
2313 + // This is the root of a reappearing boundary. Turn its layout effects
2314 + // back on.
2315 + nextEffect = fiber;
2316 + reappearLayoutEffects_begin(fiber);
2317 + }
2318 +
2319 let child = firstChild;
2320 while (child !== null) {
2321 nextEffect = child;
@@ -2280,21 +2341,6 @@ function commitLayoutEffects_begin(
2341 ensureCorrectReturnPointer(firstChild, fiber);
2342 nextEffect = firstChild;
2343 } else {
2283 - if (enableSuspenseLayoutEffectSemantics && isModernRoot) {
2284 - const visibilityChanged =
2285 - !offscreenSubtreeIsHidden && offscreenSubtreeWasHidden;
2286 -
2287 - // TODO (Offscreen) Also check: subtreeFlags & LayoutStatic
2288 - if (visibilityChanged && firstChild !== null) {
2289 - // We've just shown or hidden a Offscreen tree that contains layout effects.
2290 - // We only enter this code path for subtrees that are updated,
2291 - // because newly mounted ones would pass the LayoutMask check above.
2292 - ensureCorrectReturnPointer(firstChild, fiber);
2293 - nextEffect = firstChild;
2294 - continue;
2295 - }
2296 - }
2297 -
2344 commitLayoutMountEffects_complete(subtreeRoot, root, committedLanes);
2345 }
2346 }
@@ -2305,59 +2351,9 @@ function commitLayoutMountEffects_complete(
2351 root: FiberRoot,
2352 committedLanes: Lanes,
2353 ) {
2308 - // Suspense layout effects semantics don't change for legacy roots.
2309 - const isModernRoot = (subtreeRoot.mode & ConcurrentMode) !== NoMode;
2310 -
2354 while (nextEffect !== null) {
2355 const fiber = nextEffect;
2313 -
2314 - if (
2315 - enableSuspenseLayoutEffectSemantics &&
2316 - isModernRoot &&
2317 - offscreenSubtreeWasHidden &&
2318 - !offscreenSubtreeIsHidden
2319 - ) {
2320 - // Inside of an Offscreen subtree that changed visibility during this commit.
2321 - // If this subtree was hidden, layout effects will have already been destroyed (during mutation phase)
2322 - // but if it was just shown, we need to (re)create the effects now.
2323 - // TODO (Offscreen) Check: flags & LayoutStatic
2324 - switch (fiber.tag) {
2325 - case FunctionComponent:
2326 - case ForwardRef:
2327 - case SimpleMemoComponent: {
2328 - if (
2329 - enableProfilerTimer &&
2330 - enableProfilerCommitHooks &&
2331 - fiber.mode & ProfileMode
2332 - ) {
2333 - try {
2334 - startLayoutEffectTimer();
2335 - safelyCallCommitHookLayoutEffectListMount(fiber, fiber.return);
2336 - } finally {
2337 - recordLayoutEffectDuration(fiber);
2338 - }
2339 - } else {
2340 - safelyCallCommitHookLayoutEffectListMount(fiber, fiber.return);
2341 - }
2342 - break;
2343 - }
2344 - case ClassComponent: {
2345 - const instance = fiber.stateNode;
2346 - if (typeof instance.componentDidMount === 'function') {
2347 - safelyCallComponentDidMount(fiber, fiber.return, instance);
2348 - }
2349 - break;
2350 - }
2351 - }
2352 -
2353 - // TODO (Offscreen) Check flags & RefStatic
2354 - switch (fiber.tag) {
2355 - case ClassComponent:
2356 - case HostComponent:
2357 - safelyAttachRef(fiber, fiber.return);
2358 - break;
2359 - }
2360 - } else if ((fiber.flags & LayoutMask) !== NoFlags) {
2356 + if ((fiber.flags & LayoutMask) !== NoFlags) {
2357 const current = fiber.alternate;
2358 setCurrentDebugFiberInDEV(fiber);
2359 try {
@@ -2385,6 +2381,60 @@ function commitLayoutMountEffects_complete(
2381 }
2382 }
2383
2384 +function reappearLayoutEffects_begin(subtreeRoot: Fiber) {
2385 + while (nextEffect !== null) {
2386 + const fiber = nextEffect;
2387 + const firstChild = fiber.child;
2388 +
2389 + if (fiber.tag === OffscreenComponent) {
2390 + const isHidden = fiber.memoizedState !== null;
2391 + if (isHidden) {
2392 + // Nested Offscreen tree is still hidden. Don't re-appear its effects.
2393 + reappearLayoutEffects_complete(subtreeRoot);
2394 + continue;
2395 + }
2396 + }
2397 +
2398 + // TODO (Offscreen) Check: subtreeFlags & LayoutStatic
2399 + if (firstChild !== null) {
2400 + ensureCorrectReturnPointer(firstChild, fiber);
2401 + nextEffect = firstChild;
2402 + } else {
2403 + reappearLayoutEffects_complete(subtreeRoot);
2404 + }
2405 + }
2406 +}
2407 +
2408 +function reappearLayoutEffects_complete(subtreeRoot: Fiber) {
2409 + while (nextEffect !== null) {
2410 + const fiber = nextEffect;
2411 +
2412 + // TODO (Offscreen) Check: flags & LayoutStatic
2413 + setCurrentDebugFiberInDEV(fiber);
2414 + try {
2415 + reappearLayoutEffectsOnFiber(fiber);
2416 + } catch (error) {
2417 + reportUncaughtErrorInDEV(error);
2418 + captureCommitPhaseError(fiber, fiber.return, error);
2419 + }
2420 + resetCurrentDebugFiberInDEV();
2421 +
2422 + if (fiber === subtreeRoot) {
2423 + nextEffect = null;
2424 + return;
2425 + }
2426 +
2427 + const sibling = fiber.sibling;
2428 + if (sibling !== null) {
2429 + ensureCorrectReturnPointer(sibling, fiber.return);
2430 + nextEffect = sibling;
2431 + return;
2432 + }
2433 +
2434 + nextEffect = fiber.return;
2435 + }
2436 +}
2437 +
2438 export function commitPassiveMountEffects(
2439 root: FiberRoot,
2440 finishedWork: Fiber,
@@ -2689,6 +2739,7 @@ function ensureCorrectReturnPointer(fiber, expectedReturnFiber) {
2739 fiber.return = expectedReturnFiber;
2740 }
2741
2742 +// TODO: Reuse reappearLayoutEffects traversal here?
2743 function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2744 if (__DEV__ && enableStrictEffects) {
2745 // We don't need to re-check StrictEffectsMode here.
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+10 -22
@@ -501,30 +501,18 @@ describe('ReactSuspenseWithNoopRenderer', () => {
501
502 await rejectText('Result', new Error('Failed to load: Result'));
503
504 - gate(flags => {
505 - if (flags.enableSuspenseLayoutEffectSemantics) {
506 - expect(Scheduler).toFlushAndYield([
507 - 'Error! [Result]',
508 -
509 - // React retries one more time
510 - 'Error! [Result]',
511 - ]);
512 - expect(ReactNoop.getChildren()).toEqual([]);
513 - } else {
514 - expect(Scheduler).toFlushAndYield([
515 - 'Error! [Result]',
504 + expect(Scheduler).toFlushAndYield([
505 + 'Error! [Result]',
506
517 - // React retries one more time
518 - 'Error! [Result]',
507 + // React retries one more time
508 + 'Error! [Result]',
509
520 - // Errored again on retry. Now handle it.
521 - 'Caught error: Failed to load: Result',
522 - ]);
523 - expect(ReactNoop.getChildren()).toEqual([
524 - span('Caught error: Failed to load: Result'),
525 - ]);
526 - }
527 - });
510 + // Errored again on retry. Now handle it.
511 + 'Caught error: Failed to load: Result',
512 + ]);
513 + expect(ReactNoop.getChildren()).toEqual([
514 + span('Caught error: Failed to load: Result'),
515 + ]);
516 });
517
518 // @gate enableCache