Skip to content

Commit

Permalink
Moved onCommit and onPassiveCommit behind separate feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Feb 24, 2020
1 parent f7d1ee2 commit 7c7c918
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 52 deletions.
121 changes: 80 additions & 41 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Expand Up @@ -29,6 +29,7 @@ import {
deferPassiveEffectCleanupDuringUnmount,
enableSchedulerTracing,
enableProfilerTimer,
enableProfilerCommitHooks,
enableSuspenseServerRenderer,
enableDeprecatedFlareAPI,
enableFundamentalAPI,
Expand Down Expand Up @@ -183,7 +184,11 @@ const callComponentWillUnmountWithTimer = function(current, instance) {
startPhaseTimer(current, 'componentWillUnmount');
instance.props = current.memoizedProps;
instance.state = current.memoizedState;
if (enableProfilerTimer && current.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
instance.componentWillUnmount();
Expand Down Expand Up @@ -447,7 +452,11 @@ export function commitPassiveHookEffects(finishedWork: Fiber): void {
// TODO (#17945) We should call all passive destroy functions (for all fibers)
// before calling any create functions. The current approach only serializes
// these for a single fiber.
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startPassiveEffectTimer();
commitHookEffectListUnmount(
Expand Down Expand Up @@ -480,7 +489,7 @@ export function commitPassiveEffectDurations(
finishedRoot: FiberRoot,
finishedWork: Fiber,
): void {
if (enableProfilerTimer) {
if (enableProfilerTimer && enableProfilerCommitHooks) {
// Only Profilers with work in their subtree will have an Update effect scheduled.
if ((finishedWork.effectTag & Update) !== NoEffect) {
switch (finishedWork.tag) {
Expand Down Expand Up @@ -546,7 +555,11 @@ function commitLifeCycles(
// This is done to prevent sibling component effects from interfering with each other,
// e.g. a destroy function in one component should never override a ref set
// by a create function in another component during the same commit.
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork);
Expand Down Expand Up @@ -597,7 +610,11 @@ function commitLifeCycles(
}
}
}
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
instance.componentDidMount();
Expand Down Expand Up @@ -645,7 +662,11 @@ function commitLifeCycles(
}
}
}
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
instance.componentDidUpdate(
Expand Down Expand Up @@ -773,40 +794,42 @@ function commitLifeCycles(
}
}

if (typeof onCommit === 'function') {
if (enableSchedulerTracing) {
onCommit(
finishedWork.memoizedProps.id,
current === null ? 'mount' : 'update',
effectDuration,
commitTime,
finishedRoot.memoizedInteractions,
);
} else {
onCommit(
finishedWork.memoizedProps.id,
current === null ? 'mount' : 'update',
effectDuration,
commitTime,
);
if (enableProfilerCommitHooks) {
if (typeof onCommit === 'function') {
if (enableSchedulerTracing) {
onCommit(
finishedWork.memoizedProps.id,
current === null ? 'mount' : 'update',
effectDuration,
commitTime,
finishedRoot.memoizedInteractions,
);
} else {
onCommit(
finishedWork.memoizedProps.id,
current === null ? 'mount' : 'update',
effectDuration,
commitTime,
);
}
}
}

// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
enqueuePendingPassiveProfilerEffect(finishedWork);

// Propagate layout effect durations to the next nearest Profiler ancestor.
// Do not reset these values until the next render so DevTools has a chance to read them first.
let parentFiber = finishedWork.return;
while (parentFiber !== null) {
if (parentFiber.tag === Profiler) {
const parentStateNode = parentFiber.stateNode;
parentStateNode.effectDuration += effectDuration;
break;
// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
enqueuePendingPassiveProfilerEffect(finishedWork);

// Propagate layout effect durations to the next nearest Profiler ancestor.
// Do not reset these values until the next render so DevTools has a chance to read them first.
let parentFiber = finishedWork.return;
while (parentFiber !== null) {
if (parentFiber.tag === Profiler) {
const parentStateNode = parentFiber.stateNode;
parentStateNode.effectDuration += effectDuration;
break;
}
parentFiber = parentFiber.return;
}
parentFiber = parentFiber.return;
}
}
return;
Expand Down Expand Up @@ -958,7 +981,11 @@ function commitUnmount(
if ((tag & HookPassive) !== NoHookEffect) {
enqueuePendingPassiveHookEffectUnmount(current, effect);
} else {
if (enableProfilerTimer && current.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
startLayoutEffectTimer();
safelyCallDestroy(current, destroy);
recordLayoutEffectDuration(current);
Expand Down Expand Up @@ -991,7 +1018,11 @@ function commitUnmount(
do {
const {destroy, tag} = effect;
if (destroy !== undefined) {
if (enableProfilerTimer && current.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
if ((tag & HookPassive) !== NoHookEffect) {
safelyCallDestroy(current, destroy);
} else {
Expand Down Expand Up @@ -1543,7 +1574,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
// This prevents sibling component effects from interfering with each other,
// e.g. a destroy function in one component should never override a ref set
// by a create function in another component during the same commit.
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
commitHookEffectListUnmount(
Expand Down Expand Up @@ -1598,7 +1633,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
// This prevents sibling component effects from interfering with each other,
// e.g. a destroy function in one component should never override a ref set
// by a create function in another component during the same commit.
if (enableProfilerTimer && finishedWork.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
commitHookEffectListUnmount(HookLayout | HookHasEffect, finishedWork);
Expand Down
29 changes: 23 additions & 6 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Expand Up @@ -24,6 +24,7 @@ import {
enableSuspenseServerRenderer,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
enableProfilerTimer,
enableProfilerCommitHooks,
enableSchedulerTracing,
warnAboutUnmockedScheduler,
flushSuspenseFallbacksInTests,
Expand Down Expand Up @@ -2184,7 +2185,7 @@ export function flushPassiveEffects() {
}

export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
if (enableProfilerTimer) {
if (enableProfilerTimer && enableProfilerCommitHooks) {
pendingPassiveProfilerEffects.push(fiber);
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
Expand Down Expand Up @@ -2270,7 +2271,11 @@ function flushPassiveEffectsImpl() {
if (typeof destroy === 'function') {
if (__DEV__) {
setCurrentDebugFiberInDEV(fiber);
if (enableProfilerTimer && fiber.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
fiber.mode & ProfileMode
) {
startPassiveEffectTimer();
invokeGuardedCallback(null, destroy, null);
recordPassiveEffectDuration(fiber);
Expand All @@ -2285,7 +2290,11 @@ function flushPassiveEffectsImpl() {
resetCurrentDebugFiberInDEV();
} else {
try {
if (enableProfilerTimer && fiber.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
fiber.mode & ProfileMode
) {
try {
startPassiveEffectTimer();
destroy();
Expand All @@ -2310,7 +2319,11 @@ function flushPassiveEffectsImpl() {
const fiber = ((mountEffects[i + 1]: any): Fiber);
if (__DEV__) {
setCurrentDebugFiberInDEV(fiber);
if (enableProfilerTimer && fiber.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
fiber.mode & ProfileMode
) {
startPassiveEffectTimer();
invokeGuardedCallback(null, invokePassiveEffectCreate, null, effect);
recordPassiveEffectDuration(fiber);
Expand All @@ -2326,7 +2339,11 @@ function flushPassiveEffectsImpl() {
} else {
try {
const create = effect.create;
if (enableProfilerTimer && fiber.mode & ProfileMode) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
fiber.mode & ProfileMode
) {
try {
startPassiveEffectTimer();
effect.destroy = create();
Expand Down Expand Up @@ -2373,7 +2390,7 @@ function flushPassiveEffectsImpl() {
}
}

if (enableProfilerTimer) {
if (enableProfilerTimer && enableProfilerCommitHooks) {
let profilerEffects = pendingPassiveProfilerEffects;
pendingPassiveProfilerEffects = [];
for (let i = 0; i < profilerEffects.length; i++) {
Expand Down
13 changes: 8 additions & 5 deletions packages/react-reconciler/src/ReactProfilerTimer.js
View file
Expand Up @@ -9,7 +9,10 @@

import type {Fiber} from './ReactFiber';

import {enableProfilerTimer} from 'shared/ReactFeatureFlags';
import {
enableProfilerTimer,
enableProfilerCommitHooks,
} from 'shared/ReactFeatureFlags';
import {Profiler} from 'shared/ReactWorkTags';

// Intentionally not named imports because Rollup would use dynamic dispatch for
Expand Down Expand Up @@ -81,7 +84,7 @@ function stopProfilerTimerIfRunningAndRecordDelta(
}

function recordLayoutEffectDuration(fiber: Fiber): void {
if (!enableProfilerTimer) {
if (!enableProfilerTimer || !enableProfilerCommitHooks) {
return;
}

Expand All @@ -104,7 +107,7 @@ function recordLayoutEffectDuration(fiber: Fiber): void {
}

function recordPassiveEffectDuration(fiber: Fiber): void {
if (!enableProfilerTimer) {
if (!enableProfilerTimer || !enableProfilerCommitHooks) {
return;
}

Expand Down Expand Up @@ -132,14 +135,14 @@ function recordPassiveEffectDuration(fiber: Fiber): void {
}

function startLayoutEffectTimer(): void {
if (!enableProfilerTimer) {
if (!enableProfilerTimer || !enableProfilerCommitHooks) {
return;
}
layoutEffectStartTime = now();
}

function startPassiveEffectTimer(): void {
if (!enableProfilerTimer) {
if (!enableProfilerTimer || !enableProfilerCommitHooks) {
return;
}
passiveEffectStartTime = now();
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
View file
Expand Up @@ -26,14 +26,17 @@ let resourcePromise;
function loadModules({
deferPassiveEffectCleanupDuringUnmount = false,
enableProfilerTimer = true,
enableProfilerCommitHooks = true,
enableSchedulerTracing = true,
replayFailedUnitOfWorkWithInvokeGuardedCallback = false,
useNoopRenderer = false,
} = {}) {
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.deferPassiveEffectCleanupDuringUnmount = deferPassiveEffectCleanupDuringUnmount;
ReactFeatureFlags.runAllPassiveEffectDestroysBeforeCreates = deferPassiveEffectCleanupDuringUnmount;
ReactFeatureFlags.enableProfilerTimer = enableProfilerTimer;
ReactFeatureFlags.enableProfilerCommitHooks = enableProfilerCommitHooks;
ReactFeatureFlags.enableSchedulerTracing = enableSchedulerTracing;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = replayFailedUnitOfWorkWithInvokeGuardedCallback;

Expand Down

0 comments on commit 7c7c918

Please sign in to comment.