Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't clear other flags when adding Deletion

Browse Source 
Same as #20398 but for Deletions. There's no new regression test, but in
the effects refactor, existing tests will fail without this.
This commit is contained in:
Andrew Clark
2020-12-17 16:31:50 -06:00
parent 5687864eb7
commit ff17fc176f
13 changed files with 88 additions and 343 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-reconciler/src/ReactChildFiber.new.js
+7 -2
View File
@@ -13,7 +13,12 @@ import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import getComponentName from 'shared/getComponentName';
import {Deletion, ChildDeletion, Placement} from './ReactFiberFlags';
import {
Deletion,
ChildDeletion,
Placement,
StaticMask,
} from './ReactFiberFlags';
import {
getIteratorFn,
REACT_ELEMENT_TYPE,
@@ -275,7 +280,7 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.firstEffect = returnFiber.lastEffect = childToDelete;
}
childToDelete.nextEffect = null;
childToDelete.flags = Deletion;
childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion;
let deletions = returnFiber.deletions;
if (deletions === null) {
packages/react-reconciler/src/ReactChildFiber.old.js
+7 -2
View File
@@ -13,7 +13,12 @@ import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.old';
import getComponentName from 'shared/getComponentName';
import {Deletion, ChildDeletion, Placement} from './ReactFiberFlags';
import {
Deletion,
ChildDeletion,
Placement,
StaticMask,
} from './ReactFiberFlags';
import {
getIteratorFn,
REACT_ELEMENT_TYPE,
@@ -275,7 +280,7 @@ function ChildReconciler(shouldTrackSideEffects) {
returnFiber.firstEffect = returnFiber.lastEffect = childToDelete;
}
childToDelete.nextEffect = null;
childToDelete.flags = Deletion;
childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion;
let deletions = returnFiber.deletions;
if (deletions === null) {
packages/react-reconciler/src/ReactFiberBeginWork.new.js
+3 -2
View File
@@ -2007,7 +2007,8 @@ function updateSuspensePrimaryChildren(
if (currentFallbackChildFragment !== null) {
// Delete the fallback child fragment
currentFallbackChildFragment.nextEffect = null;
currentFallbackChildFragment.flags = Deletion;
currentFallbackChildFragment.flags =
(currentFallbackChildFragment.flags & StaticMask) | Deletion;
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
let deletions = workInProgress.deletions;
if (deletions === null) {
@@ -2998,7 +2999,7 @@ function remountFiber(
returnFiber.firstEffect = returnFiber.lastEffect = current;
}
current.nextEffect = null;
current.flags = Deletion;
current.flags = (current.flags & StaticMask) | Deletion;
let deletions = returnFiber.deletions;
if (deletions === null) {
packages/react-reconciler/src/ReactFiberBeginWork.old.js
+3 -2
View File
@@ -2007,7 +2007,8 @@ function updateSuspensePrimaryChildren(
if (currentFallbackChildFragment !== null) {
// Delete the fallback child fragment
currentFallbackChildFragment.nextEffect = null;
currentFallbackChildFragment.flags = Deletion;
currentFallbackChildFragment.flags =
(currentFallbackChildFragment.flags & StaticMask) | Deletion;
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment;
let deletions = workInProgress.deletions;
if (deletions === null) {
@@ -2998,7 +2999,7 @@ function remountFiber(
returnFiber.firstEffect = returnFiber.lastEffect = current;
}
current.nextEffect = null;
current.flags = Deletion;
current.flags = (current.flags & StaticMask) | Deletion;
let deletions = returnFiber.deletions;
if (deletions === null) {
packages/react-reconciler/src/ReactFiberClassComponent.old.js
+6 -39
View File
@@ -12,14 +12,13 @@ import type {Lanes} from './ReactFiberLane.old';
import type {UpdateQueue} from './ReactUpdateQueue.old';
import * as React from 'react';
import {Update, Snapshot, MountLayoutDev} from './ReactFiberFlags';
import {Update, Snapshot} from './ReactFiberFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
enableDebugTracing,
enableSchedulingProfiler,
warnAboutDeprecatedLifecycles,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.old';
import {isMounted} from './ReactFiberTreeReflection';
@@ -30,7 +29,7 @@ import invariant from 'shared/invariant';
import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols';
import {resolveDefaultProps} from './ReactFiberLazyComponent.old';
import {DebugTracingMode, NoMode, StrictMode} from './ReactTypeOfMode';
import {DebugTracingMode, StrictMode} from './ReactTypeOfMode';
import {
enqueueUpdate,
@@ -891,15 +890,7 @@ function mountClassInstance(
}
if (typeof instance.componentDidMount === 'function') {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & StrictMode) !== NoMode
) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
workInProgress.flags |= Update;
}
}
@@ -969,15 +960,7 @@ function resumeMountClassInstance(
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & StrictMode) !== NoMode
) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
workInProgress.flags |= Update;
}
return false;
}
@@ -1020,29 +1003,13 @@ function resumeMountClassInstance(
}
}
if (typeof instance.componentDidMount === 'function') {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & StrictMode) !== NoMode
) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
workInProgress.flags |= Update;
}
} else {
// If an update was already in progress, we should schedule an Update
// effect even though we're bailing out, so that cWU/cDU are called.
if (typeof instance.componentDidMount === 'function') {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & StrictMode) !== NoMode
) {
workInProgress.flags |= MountLayoutDev | Update;
} else {
workInProgress.flags |= Update;
}
workInProgress.flags |= Update;
}
// If shouldComponentUpdate returned false, we should still update the
packages/react-reconciler/src/ReactFiberCommitWork.old.js
-115
View File
@@ -35,7 +35,6 @@ import {
enableFundamentalAPI,
enableSuspenseCallback,
enableScopeAPI,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import {
FunctionComponent,
@@ -1799,116 +1798,6 @@ function commitResetTextContent(current: Fiber) {
resetTextContent(current.stateNode);
}
function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookLayout | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
case ClassComponent: {
const instance = fiber.stateNode;
invokeGuardedCallback(null, instance.componentDidMount, instance);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
}
}
}
function invokePassiveEffectMountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListMount,
null,
HookPassive | HookHasEffect,
fiber,
);
if (hasCaughtError()) {
const mountError = clearCaughtError();
captureCommitPhaseError(fiber, mountError);
}
break;
}
}
}
}
function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookLayout | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, unmountError);
}
break;
}
case ClassComponent: {
const instance = fiber.stateNode;
if (typeof instance.componentWillUnmount === 'function') {
safelyCallComponentWillUnmount(fiber, instance);
}
break;
}
}
}
}
function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
if (__DEV__ && enableDoubleInvokingEffects) {
switch (fiber.tag) {
case FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invokeGuardedCallback(
null,
commitHookEffectListUnmount,
null,
HookPassive | HookHasEffect,
fiber,
fiber.return,
);
if (hasCaughtError()) {
const unmountError = clearCaughtError();
captureCommitPhaseError(fiber, unmountError);
}
break;
}
}
}
}
export {
commitBeforeMutationLifeCycles,
commitResetTextContent,
@@ -1918,8 +1807,4 @@ export {
commitLifeCycles,
commitAttachRef,
commitDetachRef,
invokeLayoutEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
invokePassiveEffectMountInDEV,
invokePassiveEffectUnmountInDEV,
};
packages/react-reconciler/src/ReactFiberHooks.old.js
+16 -87
View File
@@ -28,15 +28,9 @@ import {
enableCache,
decoupleUpdatePriorityFromScheduler,
enableUseRefAccessWarning,
enableDoubleInvokingEffects,
} from 'shared/ReactFeatureFlags';
import {
NoMode,
BlockingMode,
DebugTracingMode,
StrictMode,
} from './ReactTypeOfMode';
import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode';
import {
NoLane,
NoLanes,
@@ -55,8 +49,6 @@ import {readContext} from './ReactFiberNewContext.old';
import {
Update as UpdateEffect,
Passive as PassiveEffect,
MountLayoutDev as MountLayoutDevEffect,
MountPassiveDev as MountPassiveDevEffect,
} from './ReactFiberFlags';
import {
HasEffect as HookHasEffect,
@@ -475,20 +467,7 @@ export function bailoutHooks(
lanes: Lanes,
) {
workInProgress.updateQueue = current.updateQueue;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(workInProgress.mode & StrictMode) !== NoMode
) {
workInProgress.flags &= ~(
MountLayoutDevEffect |
MountPassiveDevEffect |
PassiveEffect |
UpdateEffect
);
} else {
workInProgress.flags &= ~(PassiveEffect | UpdateEffect);
}
workInProgress.flags &= ~(PassiveEffect | UpdateEffect);
current.lanes = removeLanes(current.lanes, lanes);
}
@@ -1324,26 +1303,12 @@ function mountEffect(
warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber);
}
}
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
) {
return mountEffectImpl(
UpdateEffect | PassiveEffect | MountPassiveDevEffect,
HookPassive,
create,
deps,
);
} else {
return mountEffectImpl(
UpdateEffect | PassiveEffect,
HookPassive,
create,
deps,
);
}
return mountEffectImpl(
UpdateEffect | PassiveEffect,
HookPassive,
create,
deps,
);
}
function updateEffect(
@@ -1368,20 +1333,7 @@ function mountLayoutEffect(
create: () => (() => void) | void,
deps: Array<mixed> | void | null,
): void {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
) {
return mountEffectImpl(
UpdateEffect | MountLayoutDevEffect,
HookLayout,
create,
deps,
);
} else {
return mountEffectImpl(UpdateEffect, HookLayout, create, deps);
}
return mountEffectImpl(UpdateEffect, HookLayout, create, deps);
}
function updateLayoutEffect(
@@ -1440,25 +1392,12 @@ function mountImperativeHandle<T>(
const effectDeps =
deps !== null && deps !== undefined ? deps.concat([ref]) : null;
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
) {
return mountEffectImpl(
UpdateEffect | MountLayoutDevEffect,
HookLayout,
imperativeHandleEffect.bind(null, create, ref),
effectDeps,
);
} else {
return mountEffectImpl(
UpdateEffect,
HookLayout,
imperativeHandleEffect.bind(null, create, ref),
effectDeps,
);
}
return mountEffectImpl(
UpdateEffect,
HookLayout,
imperativeHandleEffect.bind(null, create, ref),
effectDeps,
);
}
function updateImperativeHandle<T>(
@@ -1739,17 +1678,7 @@ function mountOpaqueIdentifier(): OpaqueIDType | void {
const setId = mountState(id)[1];
if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) {
if (
__DEV__ &&
enableDoubleInvokingEffects &&
(currentlyRenderingFiber.mode & StrictMode) !== NoMode
) {
currentlyRenderingFiber.flags |=
UpdateEffect | PassiveEffect | MountPassiveDevEffect;
} else {
currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect;
}
currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect;
pushEffect(
HookHasEffect | HookPassive,
() => {
packages/react-reconciler/src/ReactFiberHydrationContext.new.js
+8 -2
View File
@@ -24,7 +24,13 @@ import {
HostRoot,
SuspenseComponent,
} from './ReactWorkTags';
import {Deletion, ChildDeletion, Placement, Hydrating} from './ReactFiberFlags';
import {
Deletion,
ChildDeletion,
Placement,
Hydrating,
StaticMask,
} from './ReactFiberFlags';
import invariant from 'shared/invariant';
import {
@@ -124,7 +130,7 @@ function deleteHydratableInstance(
const childToDelete = createFiberFromHostInstanceForDeletion();
childToDelete.stateNode = instance;
childToDelete.return = returnFiber;
childToDelete.flags = Deletion;
childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion;
// This might seem like it belongs on progressedFirstDeletion. However,
// these children are not part of the reconciliation list of children.
packages/react-reconciler/src/ReactFiberHydrationContext.old.js
+8 -2
View File
@@ -24,7 +24,13 @@ import {
HostRoot,
SuspenseComponent,
} from './ReactWorkTags';
import {Deletion, ChildDeletion, Placement, Hydrating} from './ReactFiberFlags';
import {
Deletion,
ChildDeletion,
Placement,
Hydrating,
StaticMask,
} from './ReactFiberFlags';
import invariant from 'shared/invariant';
import {
@@ -124,7 +130,7 @@ function deleteHydratableInstance(
const childToDelete = createFiberFromHostInstanceForDeletion();
childToDelete.stateNode = instance;
childToDelete.return = returnFiber;
childToDelete.flags = Deletion;
childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion;
// This might seem like it belongs on progressedFirstDeletion. However,
// these children are not part of the reconciliation list of children.
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+21 -80
View File
@@ -14,7 +14,6 @@ import type {ReactPriorityLevel} from './ReactInternalTypes';
import type {Interaction} from 'scheduler/src/Tracing';
import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
import type {Effect as HookEffect} from './ReactFiberHooks.old';
import type {Flags} from './ReactFiberFlags';
import type {StackCursor} from './ReactFiberStack.old';
import {
@@ -32,7 +31,6 @@ import {
enableDebugTracing,
enableSchedulingProfiler,
enableScopeAPI,
enableDoubleInvokingEffects,
disableSchedulerTimeoutInWorkLoop,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
@@ -134,8 +132,6 @@ import {
HostEffectMask,
Hydrating,
HydratingAndUpdate,
MountPassiveDev,
MountLayoutDev,
} from './ReactFiberFlags';
import {
NoLanePriority,
@@ -196,10 +192,6 @@ import {
commitPassiveEffectDurations,
commitResetTextContent,
isSuspenseBoundaryBeingHidden,
invokeLayoutEffectMountInDEV,
invokePassiveEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
invokePassiveEffectUnmountInDEV,
} from './ReactFiberCommitWork.old';
import {enqueueUpdate} from './ReactUpdateQueue.old';
import {resetContextDependencies} from './ReactFiberNewContext.old';
@@ -2134,10 +2126,6 @@ function commitRootImpl(root, renderPriorityLevel) {
pendingPassiveEffectsLanes = lanes;
pendingPassiveEffectsRenderPriority = renderPriorityLevel;
} else {
if (__DEV__ && enableDoubleInvokingEffects) {
commitDoubleInvokeEffectsInDEV(root, false);
}
// We are done with the effect chain at this point so let's clear the
// nextEffect pointers to assist with GC. If we have passive effects, we'll
// clear this in flushPassiveEffects.
@@ -2685,29 +2673,6 @@ function flushPassiveEffectsImpl() {
}
}
if (enableProfilerTimer && enableProfilerCommitHooks) {
const profilerEffects = pendingPassiveProfilerEffects;
pendingPassiveProfilerEffects = [];
for (let i = 0; i < profilerEffects.length; i++) {
const fiber = ((profilerEffects[i]: any): Fiber);
commitPassiveEffectDurations(root, fiber);
}
}
if (__DEV__) {
if (enableDebugTracing) {
logPassiveEffectsStopped();
}
}
if (enableSchedulingProfiler) {
markPassiveEffectsStopped();
}
if (__DEV__ && enableDoubleInvokingEffects) {
commitDoubleInvokeEffectsInDEV(root, true);
}
// Note: This currently assumes there are no passive effects on the root fiber
// because the root is not part of its own effect list.
// This could change in the future.
@@ -2722,8 +2687,13 @@ function flushPassiveEffectsImpl() {
effect = nextNextEffect;
}
if (__DEV__) {
isFlushingPassiveEffects = false;
if (enableProfilerTimer && enableProfilerCommitHooks) {
const profilerEffects = pendingPassiveProfilerEffects;
pendingPassiveProfilerEffects = [];
for (let i = 0; i < profilerEffects.length; i++) {
const fiber = ((profilerEffects[i]: any): Fiber);
commitPassiveEffectDurations(root, fiber);
}
}
if (enableSchedulerTracing) {
@@ -2731,6 +2701,20 @@ function flushPassiveEffectsImpl() {
finishPendingInteractions(root, lanes);
}
if (__DEV__) {
isFlushingPassiveEffects = false;
}
if (__DEV__) {
if (enableDebugTracing) {
logPassiveEffectsStopped();
}
}
if (enableSchedulingProfiler) {
markPassiveEffectsStopped();
}
executionContext = prevExecutionContext;
flushSyncCallbackQueue();
@@ -3014,49 +2998,6 @@ function flushRenderPhaseStrictModeWarningsInDEV() {
}
}
function commitDoubleInvokeEffectsInDEV(
root: FiberRoot,
hasPassiveEffects: boolean,
) {
if (__DEV__ && enableDoubleInvokingEffects) {
invokeEffectsInDev(root, MountLayoutDev, invokeLayoutEffectUnmountInDEV);
if (hasPassiveEffects) {
invokeEffectsInDev(
root,
MountPassiveDev,
invokePassiveEffectUnmountInDEV,
);
}
invokeEffectsInDev(root, MountLayoutDev, invokeLayoutEffectMountInDEV);
if (hasPassiveEffects) {
invokeEffectsInDev(root, MountPassiveDev, invokePassiveEffectMountInDEV);
}
}
}
function invokeEffectsInDev(
root: FiberRoot,
fiberFlags: Flags,
invokeEffectFn: (fiber: Fiber) => void,
): void {
if (__DEV__ && enableDoubleInvokingEffects) {
// Note: This currently assumes there are no passive effects on the root fiber
// because the root is not part of its own effect list.
// This could change in the future.
let currentEffect = root.current.firstEffect;
while (currentEffect !== null) {
const flags = currentEffect.flags;
if ((flags & fiberFlags) !== NoFlags) {
setCurrentDebugFiberInDEV(currentEffect);
invokeEffectFn(currentEffect);
resetCurrentDebugFiberInDEV();
}
currentEffect = currentEffect.nextEffect;
}
}
}
let didWarnStateUpdateForNotYetMountedComponent: Set<string> | null = null;
function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) {
if (__DEV__) {
packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js
+2 -3
View File
@@ -15,9 +15,8 @@ let ReactNoop;
let Scheduler;
function shouldDoubleInvokingEffects() {
// For now, this feature only exists in the old fork (while the new fork is being bisected).
// Eventually we'll land it in both forks.
return __DEV__ && !__VARIANT__;
// Reverted temporarily. Need to re-land.
return false;
}
describe('ReactDoubleInvokeEvents', () => {
packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js
Vendored
+3 -1
View File
@@ -26,7 +26,9 @@ describe('ReactDoubleInvokeEvents', () => {
function supportsDoubleInvokeEffects() {
return gate(
flags =>
flags.build === 'development' && flags.enableDoubleInvokingEffects,
flags.build === 'development' &&
flags.enableDoubleInvokingEffects &&
flags.dfsEffectsRefactor,
);
}
packages/react/src/__tests__/ReactProfiler-test.internal.js
+4 -6
View File
@@ -4874,8 +4874,10 @@ describe('Profiler', () => {
});
if (__DEV__) {
// @gate dfsEffectsRefactor
// @gate enableDoubleInvokingEffects
it('double invoking does not disconnect wrapped async work', () => {
ReactFeatureFlags.enableDoubleInvokingEffects = !__VARIANT__;
ReactFeatureFlags.enableDoubleInvokingEffects = true;
const callback = jest.fn(() => {
const wrappedInteractions = SchedulerTracing.unstable_getCurrent();
@@ -4913,11 +4915,7 @@ describe('Profiler', () => {
jest.runAllTimers();
if (ReactFeatureFlags.enableDoubleInvokingEffects) {
expect(callback).toHaveBeenCalledTimes(4); // 2x per effect
} else {
expect(callback).toHaveBeenCalledTimes(2);
}
expect(callback).toHaveBeenCalledTimes(4); // 2x per effect
expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1);
});