Move passive logic out of layout phase (#19500)
* setCurrentFiber per fiber, instead of per effect * Re-use safelyCallDestroy Part of the code in flushPassiveUnmountEffects is a duplicate of the code used for unmounting layout effects. I did some minor refactoring to so we could use the same function in both places. Closure will inline anyway so it doesn't affect code size or performance, just maintainability. * Don't check HookHasEffect during deletion We don't need to check HookHasEffect during a deletion; all effects are unmounted. So we also don't have to set HookHasEffect during a deletion, either. This allows us to remove the last remaining passive effect logic from the synchronous layout phase.
Andrew Clark committed
Jul 30, 2020 at 23:43 UTC
ede9170648d07a63cd282e6acb3ea1fe9e22ded9
2 files changed
+32
-57
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+2
-6
@@ -122,7 +122,6 @@ import {
122
NoEffect as NoHookEffect,
123
HasEffect as HookHasEffect,
124
Layout as HookLayout,
125
- Passive as HookPassive,
125
} from './ReactHookEffectTags';
126
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
127
import {
@@ -206,7 +205,7 @@ function safelyDetachRef(current: Fiber) {
205
}
206
}
207
209
-function safelyCallDestroy(current, destroy) {
208
+export function safelyCallDestroy(current: Fiber, destroy: () => void) {
209
if (__DEV__) {
210
invokeGuardedCallback(null, destroy, null);
211
if (hasCaughtError()) {
@@ -876,10 +875,7 @@ function commitUnmount(
875
do {
876
const {destroy, tag} = effect;
877
if (destroy !== undefined) {
879
- if ((tag & HookPassive) !== NoHookEffect) {
880
- // TODO: Consider if we can move this block out of the synchronous commit phase
881
- effect.tag |= HookHasEffect;
882
- } else {
878
+ if ((tag & HookLayout) !== NoHookEffect) {
879
if (
880
enableProfilerTimer &&
881
enableProfilerCommitHooks &&
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+30
-51
@@ -15,6 +15,7 @@ import type {Interaction} from 'scheduler/src/Tracing';
15
import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
16
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
17
import type {Effect as HookEffect} from './ReactFiberHooks.new';
18
+import type {HookEffectTag} from './ReactHookEffectTags';
19
import type {StackCursor} from './ReactFiberStack.new';
20
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
21
@@ -209,6 +210,7 @@ import {
210
commitPassiveEffectDurations,
211
commitResetTextContent,
212
isSuspenseBoundaryBeingHidden,
213
+ safelyCallDestroy,
214
} from './ReactFiberCommitWork.new';
215
import {enqueueUpdate} from './ReactUpdateQueue.new';
216
import {resetContextDependencies} from './ReactFiberNewContext.new';
@@ -2702,6 +2704,8 @@ function flushPassiveMountEffects(firstChild: Fiber): void {
2704
}
2705
2706
function flushPassiveMountEffectsImpl(fiber: Fiber): void {
2707
+ setCurrentDebugFiberInDEV(fiber);
2708
+
2709
const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any);
2710
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
2711
if (lastEffect !== null) {
@@ -2715,7 +2719,6 @@ function flushPassiveMountEffectsImpl(fiber: Fiber): void {
2719
(tag & HookHasEffect) !== NoHookEffect
2720
) {
2721
if (__DEV__) {
2718
- setCurrentDebugFiberInDEV(fiber);
2722
if (
2723
enableProfilerTimer &&
2724
enableProfilerCommitHooks &&
@@ -2742,7 +2745,6 @@ function flushPassiveMountEffectsImpl(fiber: Fiber): void {
2745
const error = clearCaughtError();
2746
captureCommitPhaseError(fiber, error);
2747
}
2745
- resetCurrentDebugFiberInDEV();
2748
} else {
2749
try {
2750
const create = effect.create;
@@ -2769,6 +2771,8 @@ function flushPassiveMountEffectsImpl(fiber: Fiber): void {
2771
2772
effect = next;
2773
} while (effect !== firstEffect);
2774
+
2775
+ resetCurrentDebugFiberInDEV();
2776
}
2777
}
2778
@@ -2813,7 +2817,7 @@ function flushPassiveUnmountEffects(firstChild: Fiber): void {
2817
case Block: {
2818
const primaryEffectTag = fiber.effectTag & Passive;
2819
if (primaryEffectTag !== NoEffect) {
2816
- flushPassiveUnmountEffectsImpl(fiber);
2820
+ flushPassiveUnmountEffectsImpl(fiber, HookPassive | HookHasEffect);
2821
}
2822
}
2823
}
@@ -2845,7 +2849,7 @@ function flushPassiveUnmountEffectsInsideOfDeletedTree(
2849
case ForwardRef:
2850
case SimpleMemoComponent:
2851
case Block: {
2848
- flushPassiveUnmountEffectsImpl(fiber);
2852
+ flushPassiveUnmountEffectsImpl(fiber, HookPassive);
2853
}
2854
}
2855
}
@@ -2854,67 +2858,42 @@ function flushPassiveUnmountEffectsInsideOfDeletedTree(
2858
}
2859
}
2860
2857
-function flushPassiveUnmountEffectsImpl(fiber: Fiber): void {
2861
+function flushPassiveUnmountEffectsImpl(
2862
+ fiber: Fiber,
2863
+ // Tags to check for when deciding whether to unmount. e.g. to skip over
2864
+ // layout effects
2865
+ hookEffectTag: HookEffectTag,
2866
+): void {
2867
const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any);
2868
const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null;
2869
if (lastEffect !== null) {
2870
+ setCurrentDebugFiberInDEV(fiber);
2871
+
2872
const firstEffect = lastEffect.next;
2873
let effect = firstEffect;
2874
do {
2875
const {next, tag} = effect;
2865
- if (
2866
- (tag & HookPassive) !== NoHookEffect &&
2867
- (tag & HookHasEffect) !== NoHookEffect
2868
- ) {
2876
+ if ((tag & hookEffectTag) === hookEffectTag) {
2877
const destroy = effect.destroy;
2870
- effect.destroy = undefined;
2871
-
2872
- if (typeof destroy === 'function') {
2873
- if (__DEV__) {
2874
- setCurrentDebugFiberInDEV(fiber);
2875
- if (
2876
- enableProfilerTimer &&
2877
- enableProfilerCommitHooks &&
2878
- fiber.mode & ProfileMode
2879
- ) {
2880
- startPassiveEffectTimer();
2881
- invokeGuardedCallback(null, destroy, null);
2882
- recordPassiveEffectDuration(fiber);
2883
- } else {
2884
- invokeGuardedCallback(null, destroy, null);
2885
- }
2886
- if (hasCaughtError()) {
2887
- invariant(fiber !== null, 'Should be working on an effect.');
2888
- const error = clearCaughtError();
2889
- captureCommitPhaseError(fiber, error);
2890
- }
2891
- resetCurrentDebugFiberInDEV();
2878
+ if (destroy !== undefined) {
2879
+ effect.destroy = undefined;
2880
+ if (
2881
+ enableProfilerTimer &&
2882
+ enableProfilerCommitHooks &&
2883
+ fiber.mode & ProfileMode
2884
+ ) {
2885
+ startPassiveEffectTimer();
2886
+ safelyCallDestroy(fiber, destroy);
2887
+ recordPassiveEffectDuration(fiber);
2888
} else {
2893
- try {
2894
- if (
2895
- enableProfilerTimer &&
2896
- enableProfilerCommitHooks &&
2897
- fiber.mode & ProfileMode
2898
- ) {
2899
- try {
2900
- startPassiveEffectTimer();
2901
- destroy();
2902
- } finally {
2903
- recordPassiveEffectDuration(fiber);
2904
- }
2905
- } else {
2906
- destroy();
2907
- }
2908
- } catch (error) {
2909
- invariant(fiber !== null, 'Should be working on an effect.');
2910
- captureCommitPhaseError(fiber, error);
2911
- }
2889
+ safelyCallDestroy(fiber, destroy);
2890
}
2891
}
2892
}
2915
-
2893
effect = next;
2894
} while (effect !== firstEffect);
2895
+
2896
+ resetCurrentDebugFiberInDEV();
2897
}
2898
}
2899