Skip to content

Commit 3df7e8f

Browse files
acdliteacdlite
committed
Move flag check into each switch case
The fiber tag is more specific than the effect flag, so we should always refine the type of work first, to minimize redundant checks.
1 parent b8c96b1 commit 3df7e8f

File tree

2 files changed

+302
-284
lines changed

2 files changed

+302
-284
lines changed

‎packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 151 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -711,11 +711,12 @@ function commitLayoutEffectOnFiber(
711711
finishedWork: Fiber,
712712
committedLanes: Lanes,
713713
): void {
714-
if ((finishedWork.flags & LayoutMask) !== NoFlags) {
715-
switch (finishedWork.tag) {
716-
case FunctionComponent:
717-
case ForwardRef:
718-
case SimpleMemoComponent: {
714+
const flags = finishedWork.flags;
715+
switch (finishedWork.tag) {
716+
case FunctionComponent:
717+
case ForwardRef:
718+
case SimpleMemoComponent: {
719+
if (flags & Update) {
719720
if (!offscreenSubtreeWasHidden) {
720721
// At this point layout effects have already been destroyed (during mutation phase).
721722
// This is done to prevent sibling component effects from interfering with each other,
@@ -739,128 +740,128 @@ function commitLayoutEffectOnFiber(
739740
commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
740741
}
741742
}
742-
break;
743743
}
744-
case ClassComponent: {
745-
const instance = finishedWork.stateNode;
746-
if (finishedWork.flags & Update) {
747-
if (!offscreenSubtreeWasHidden) {
748-
if (current === null) {
749-
// We could update instance props and state here,
750-
// but instead we rely on them being set during last render.
751-
// TODO: revisit this when we implement resuming.
752-
if (__DEV__) {
753-
if (
754-
finishedWork.type === finishedWork.elementType &&
755-
!didWarnAboutReassigningProps
756-
) {
757-
if (instance.props !== finishedWork.memoizedProps) {
758-
console.error(
759-
'Expected %s props to match memoized props before ' +
760-
'componentDidMount. ' +
761-
'This might either be because of a bug in React, or because ' +
762-
'a component reassigns its own `this.props`. ' +
763-
'Please file an issue.',
764-
getComponentNameFromFiber(finishedWork) || 'instance',
765-
);
766-
}
767-
if (instance.state !== finishedWork.memoizedState) {
768-
console.error(
769-
'Expected %s state to match memoized state before ' +
770-
'componentDidMount. ' +
771-
'This might either be because of a bug in React, or because ' +
772-
'a component reassigns its own `this.state`. ' +
773-
'Please file an issue.',
774-
getComponentNameFromFiber(finishedWork) || 'instance',
775-
);
776-
}
777-
}
778-
}
744+
break;
745+
}
746+
case ClassComponent: {
747+
if (flags & Update) {
748+
if (!offscreenSubtreeWasHidden) {
749+
const instance = finishedWork.stateNode;
750+
if (current === null) {
751+
// We could update instance props and state here,
752+
// but instead we rely on them being set during last render.
753+
// TODO: revisit this when we implement resuming.
754+
if (__DEV__) {
779755
if (
780-
enableProfilerTimer &&
781-
enableProfilerCommitHooks &&
782-
finishedWork.mode & ProfileMode
756+
finishedWork.type === finishedWork.elementType &&
757+
!didWarnAboutReassigningProps
783758
) {
784-
try {
785-
startLayoutEffectTimer();
786-
instance.componentDidMount();
787-
} finally {
788-
recordLayoutEffectDuration(finishedWork);
759+
if (instance.props !== finishedWork.memoizedProps) {
760+
console.error(
761+
'Expected %s props to match memoized props before ' +
762+
'componentDidMount. ' +
763+
'This might either be because of a bug in React, or because ' +
764+
'a component reassigns its own `this.props`. ' +
765+
'Please file an issue.',
766+
getComponentNameFromFiber(finishedWork) || 'instance',
767+
);
768+
}
769+
if (instance.state !== finishedWork.memoizedState) {
770+
console.error(
771+
'Expected %s state to match memoized state before ' +
772+
'componentDidMount. ' +
773+
'This might either be because of a bug in React, or because ' +
774+
'a component reassigns its own `this.state`. ' +
775+
'Please file an issue.',
776+
getComponentNameFromFiber(finishedWork) || 'instance',
777+
);
789778
}
790-
} else {
779+
}
780+
}
781+
if (
782+
enableProfilerTimer &&
783+
enableProfilerCommitHooks &&
784+
finishedWork.mode & ProfileMode
785+
) {
786+
try {
787+
startLayoutEffectTimer();
791788
instance.componentDidMount();
789+
} finally {
790+
recordLayoutEffectDuration(finishedWork);
792791
}
793792
} else {
794-
const prevProps =
795-
finishedWork.elementType === finishedWork.type
796-
? current.memoizedProps
797-
: resolveDefaultProps(
798-
finishedWork.type,
799-
current.memoizedProps,
800-
);
801-
const prevState = current.memoizedState;
802-
// We could update instance props and state here,
803-
// but instead we rely on them being set during last render.
804-
// TODO: revisit this when we implement resuming.
805-
if (__DEV__) {
806-
if (
807-
finishedWork.type === finishedWork.elementType &&
808-
!didWarnAboutReassigningProps
809-
) {
810-
if (instance.props !== finishedWork.memoizedProps) {
811-
console.error(
812-
'Expected %s props to match memoized props before ' +
813-
'componentDidUpdate. ' +
814-
'This might either be because of a bug in React, or because ' +
815-
'a component reassigns its own `this.props`. ' +
816-
'Please file an issue.',
817-
getComponentNameFromFiber(finishedWork) || 'instance',
818-
);
819-
}
820-
if (instance.state !== finishedWork.memoizedState) {
821-
console.error(
822-
'Expected %s state to match memoized state before ' +
823-
'componentDidUpdate. ' +
824-
'This might either be because of a bug in React, or because ' +
825-
'a component reassigns its own `this.state`. ' +
826-
'Please file an issue.',
827-
getComponentNameFromFiber(finishedWork) || 'instance',
828-
);
829-
}
830-
}
831-
}
793+
instance.componentDidMount();
794+
}
795+
} else {
796+
const prevProps =
797+
finishedWork.elementType === finishedWork.type
798+
? current.memoizedProps
799+
: resolveDefaultProps(finishedWork.type, current.memoizedProps);
800+
const prevState = current.memoizedState;
801+
// We could update instance props and state here,
802+
// but instead we rely on them being set during last render.
803+
// TODO: revisit this when we implement resuming.
804+
if (__DEV__) {
832805
if (
833-
enableProfilerTimer &&
834-
enableProfilerCommitHooks &&
835-
finishedWork.mode & ProfileMode
806+
finishedWork.type === finishedWork.elementType &&
807+
!didWarnAboutReassigningProps
836808
) {
837-
try {
838-
startLayoutEffectTimer();
839-
instance.componentDidUpdate(
840-
prevProps,
841-
prevState,
842-
instance.__reactInternalSnapshotBeforeUpdate,
809+
if (instance.props !== finishedWork.memoizedProps) {
810+
console.error(
811+
'Expected %s props to match memoized props before ' +
812+
'componentDidUpdate. ' +
813+
'This might either be because of a bug in React, or because ' +
814+
'a component reassigns its own `this.props`. ' +
815+
'Please file an issue.',
816+
getComponentNameFromFiber(finishedWork) || 'instance',
843817
);
844-
} finally {
845-
recordLayoutEffectDuration(finishedWork);
846818
}
847-
} else {
819+
if (instance.state !== finishedWork.memoizedState) {
820+
console.error(
821+
'Expected %s state to match memoized state before ' +
822+
'componentDidUpdate. ' +
823+
'This might either be because of a bug in React, or because ' +
824+
'a component reassigns its own `this.state`. ' +
825+
'Please file an issue.',
826+
getComponentNameFromFiber(finishedWork) || 'instance',
827+
);
828+
}
829+
}
830+
}
831+
if (
832+
enableProfilerTimer &&
833+
enableProfilerCommitHooks &&
834+
finishedWork.mode & ProfileMode
835+
) {
836+
try {
837+
startLayoutEffectTimer();
848838
instance.componentDidUpdate(
849839
prevProps,
850840
prevState,
851841
instance.__reactInternalSnapshotBeforeUpdate,
852842
);
843+
} finally {
844+
recordLayoutEffectDuration(finishedWork);
853845
}
846+
} else {
847+
instance.componentDidUpdate(
848+
prevProps,
849+
prevState,
850+
instance.__reactInternalSnapshotBeforeUpdate,
851+
);
854852
}
855853
}
856854
}
855+
}
857856

857+
if (flags & Callback) {
858858
// TODO: I think this is now always non-null by the time it reaches the
859859
// commit phase. Consider removing the type check.
860860
const updateQueue: UpdateQueue<
861861
*,
862862
> | null = (finishedWork.updateQueue: any);
863-
if (finishedWork.flags & Callback && updateQueue !== null) {
863+
if (updateQueue !== null) {
864+
const instance = finishedWork.stateNode;
864865
if (__DEV__) {
865866
if (
866867
finishedWork.type === finishedWork.elementType &&
@@ -893,21 +894,23 @@ function commitLayoutEffectOnFiber(
893894
// TODO: revisit this when we implement resuming.
894895
commitCallbacks(updateQueue, instance);
895896
}
897+
}
896898

897-
if (finishedWork.flags & Ref) {
898-
if (!offscreenSubtreeWasHidden) {
899-
commitAttachRef(finishedWork);
900-
}
899+
if (flags & Ref) {
900+
if (!offscreenSubtreeWasHidden) {
901+
commitAttachRef(finishedWork);
901902
}
902-
break;
903903
}
904-
case HostRoot: {
904+
break;
905+
}
906+
case HostRoot: {
907+
if (flags & Callback) {
905908
// TODO: I think this is now always non-null by the time it reaches the
906909
// commit phase. Consider removing the type check.
907910
const updateQueue: UpdateQueue<
908911
*,
909912
> | null = (finishedWork.updateQueue: any);
910-
if (finishedWork.flags & Callback && updateQueue !== null) {
913+
if (updateQueue !== null) {
911914
let instance = null;
912915
if (finishedWork.child !== null) {
913916
switch (finishedWork.child.tag) {
@@ -921,9 +924,11 @@ function commitLayoutEffectOnFiber(
921924
}
922925
commitCallbacks(updateQueue, instance);
923926
}
924-
break;
925927
}
926-
case HostComponent: {
928+
break;
929+
}
930+
case HostComponent: {
931+
if (flags & Update) {
927932
const instance: Instance = finishedWork.stateNode;
928933

929934
// Renderers may schedule work to be done after host components are mounted
@@ -935,24 +940,26 @@ function commitLayoutEffectOnFiber(
935940
const props = finishedWork.memoizedProps;
936941
commitMount(instance, type, props, finishedWork);
937942
}
943+
}
938944

939-
if (finishedWork.flags & Ref) {
940-
if (!offscreenSubtreeWasHidden) {
941-
commitAttachRef(finishedWork);
942-
}
945+
if (flags & Ref) {
946+
if (!offscreenSubtreeWasHidden) {
947+
commitAttachRef(finishedWork);
943948
}
944-
break;
945-
}
946-
case HostText: {
947-
// We have no life-cycles associated with text.
948-
break;
949949
}
950-
case HostPortal: {
951-
// We have no life-cycles associated with portals.
952-
break;
953-
}
954-
case Profiler: {
955-
if (enableProfilerTimer) {
950+
break;
951+
}
952+
case HostText: {
953+
// We have no life-cycles associated with text.
954+
break;
955+
}
956+
case HostPortal: {
957+
// We have no life-cycles associated with portals.
958+
break;
959+
}
960+
case Profiler: {
961+
if (enableProfilerTimer) {
962+
if (flags & Update) {
956963
const {onCommit, onRender} = finishedWork.memoizedProps;
957964
const {effectDuration} = finishedWork.stateNode;
958965

@@ -1009,27 +1016,29 @@ function commitLayoutEffectOnFiber(
10091016
}
10101017
}
10111018
}
1012-
break;
10131019
}
1014-
case SuspenseComponent: {
1020+
break;
1021+
}
1022+
case SuspenseComponent: {
1023+
if (flags & Update) {
10151024
commitSuspenseHydrationCallbacks(finishedRoot, finishedWork);
1016-
break;
10171025
}
1018-
case SuspenseListComponent:
1019-
case IncompleteClassComponent:
1020-
case ScopeComponent:
1021-
case OffscreenComponent:
1022-
case LegacyHiddenComponent:
1023-
case TracingMarkerComponent: {
1024-
break;
1025-
}
1026-
1027-
default:
1028-
throw new Error(
1029-
'This unit of work tag should not have side-effects. This error is ' +
1030-
'likely caused by a bug in React. Please file an issue.',
1031-
);
1026+
break;
1027+
}
1028+
case SuspenseListComponent:
1029+
case IncompleteClassComponent:
1030+
case ScopeComponent:
1031+
case OffscreenComponent:
1032+
case LegacyHiddenComponent:
1033+
case TracingMarkerComponent: {
1034+
break;
10321035
}
1036+
1037+
default:
1038+
throw new Error(
1039+
'This unit of work tag should not have side-effects. This error is ' +
1040+
'likely caused by a bug in React. Please file an issue.',
1041+
);
10331042
}
10341043
}
10351044

0 commit comments

Comments
 (0)