Skip to content

Commit

Permalink
Fixed yielding/incremental bug
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed May 18, 2019
1 parent f0562c7 commit 0602449
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 85 deletions.
8 changes: 5 additions & 3 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import {
NoWork,
computeAsyncExpirationNoBucket,
} from './ReactFiberExpirationTime';
import {onCommitUnmount} from './ReactFiberDevToolsHook';
import {isDevToolsPresent, onCommitUnmount} from './ReactFiberDevToolsHook';
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
import {logCapturedError} from './ReactFiberErrorLogger';
Expand Down Expand Up @@ -1341,8 +1341,10 @@ function commitSuspenseComponent(
retry = Schedule_tracing_wrap(retry);
}
if (enableUpdaterTracking) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(finishedRoot, committedExpirationTime);
if (isDevToolsPresent) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(finishedRoot, committedExpirationTime);
}
}
retryCache.add(thenable);
thenable.then(retry, retry);
Expand Down
81 changes: 57 additions & 24 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ import {
hasCaughtError,
clearCaughtError,
} from 'shared/ReactErrorUtils';
import {onCommitRoot} from './ReactFiberDevToolsHook';
import {isDevToolsPresent, onCommitRoot} from './ReactFiberDevToolsHook';

const ceil = Math.ceil;

Expand Down Expand Up @@ -339,13 +339,15 @@ export function scheduleUpdateOnFiber(
}

if (enableUpdaterTracking) {
const pendingUpdatersMap = root.pendingUpdatersMap;
let updaters = pendingUpdatersMap.get(expirationTime);
if (updaters == null) {
updaters = new Set();
pendingUpdatersMap.set(expirationTime, updaters);
if (isDevToolsPresent) {
const pendingUpdatersMap = root.pendingUpdatersMap;
let updaters = pendingUpdatersMap.get(expirationTime);
if (updaters == null) {
updaters = new Set();
pendingUpdatersMap.set(expirationTime, updaters);
}
updaters.add(fiber);
}
updaters.add(fiber);
}

root.pingTime = NoWork;
Expand Down Expand Up @@ -813,6 +815,17 @@ function renderRoot(
Interaction,
>);
}

if (enableUpdaterTracking) {
if (isDevToolsPresent) {
// If we didn't finish the current work, restore pending updaters for the next pass.
if (root.memoizedUpdaters.size > 0) {
restorePendingUpdaters(root, expirationTime);
root.memoizedUpdaters.clear();
}
}
}

return renderRoot.bind(null, root, currentTime);
}
}
Expand Down Expand Up @@ -878,6 +891,17 @@ function renderRoot(
if (expirationTime !== Sync) {
startRequestCallbackTimer();
}

if (enableUpdaterTracking) {
if (isDevToolsPresent) {
// If we didn't finish the current work, restore pending updaters for the next pass.
if (root.memoizedUpdaters.size > 0) {
restorePendingUpdaters(root, expirationTime);
root.memoizedUpdaters.clear();
}
}
}

return renderRoot.bind(null, root, expirationTime);
}
}
Expand Down Expand Up @@ -1305,8 +1329,10 @@ function commitRootImpl(root) {
root.lastPendingTime = firstPendingTimeBeforeCommit;

if (enableUpdaterTracking) {
if (firstPendingTimeBeforeCommit !== NoWork) {
restorePendingUpdaters(root, root.lastPendingTime);
if (isDevToolsPresent) {
if (firstPendingTimeBeforeCommit !== NoWork) {
restorePendingUpdaters(root, root.lastPendingTime);
}
}
}
}
Expand Down Expand Up @@ -1515,6 +1541,12 @@ function commitRootImpl(root) {

onCommitRoot(finishedWork.stateNode);

if (enableUpdaterTracking) {
if (isDevToolsPresent) {
root.memoizedUpdaters.clear();
}
}

if (remainingExpirationTime === Sync) {
// Count the number of times the root synchronously re-renders without
// finishing. If there are too many, it indicates an infinite update loop.
Expand Down Expand Up @@ -2191,7 +2223,7 @@ export function restorePendingUpdaters(
root: FiberRoot,
expirationTime: ExpirationTime,
): void {
if (!enableUpdaterTracking) {
if (!enableUpdaterTracking || !isDevToolsPresent) {
return;
}
const pendingUpdatersMap = root.pendingUpdatersMap;
Expand Down Expand Up @@ -2323,20 +2355,21 @@ function startWorkOnPendingInteraction(root, expirationTime) {
// This is called when new work is started on a root.

if (enableUpdaterTracking) {
const memoizedUpdaters: Set<Fiber> = new Set();
const pendingUpdatersMap = root.pendingUpdatersMap;
pendingUpdatersMap.forEach((updaters, scheduledExpirationTime) => {
if (scheduledExpirationTime >= expirationTime) {
pendingUpdatersMap.delete(scheduledExpirationTime);
updaters.forEach(fiber => memoizedUpdaters.add(fiber));
}
});

// Store the current set of interactions on the FiberRoot for a few reasons:
// We can re-use it in hot functions like renderRoot() without having to
// recalculate it. This also provides DevTools with a way to access it when
// the onCommitRoot() hook is called.
root.memoizedUpdaters = memoizedUpdaters;
if (isDevToolsPresent) {
// Store the current set of interactions on the FiberRoot for a few reasons:
// We can re-use it in hot functions like renderRoot() without having to
// recalculate it. This also provides DevTools with a way to access it when
// the onCommitRoot() hook is called.
const pendingUpdatersMap = root.pendingUpdatersMap;
pendingUpdatersMap.forEach((updaters, scheduledExpirationTime) => {
if (scheduledExpirationTime >= expirationTime) {
pendingUpdatersMap.delete(scheduledExpirationTime);
updaters.forEach(fiber => {
root.memoizedUpdaters.add(fiber);
});
}
});
}
}

if (enableSchedulerTracing) {
Expand Down
13 changes: 9 additions & 4 deletions packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import {
checkForWrongSuspensePriorityInDEV,
restorePendingUpdaters,
} from './ReactFiberScheduler';
import {isDevToolsPresent} from './ReactFiberDevToolsHook';

import invariant from 'shared/invariant';

Expand Down Expand Up @@ -190,8 +191,10 @@ function attachPingListener(
ping = Schedule_tracing_wrap(ping);
}
if (enableUpdaterTracking) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(root, renderExpirationTime);
if (isDevToolsPresent) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(root, renderExpirationTime);
}
}
thenable.then(ping, ping);
}
Expand All @@ -210,8 +213,10 @@ function throwException(
sourceFiber.firstEffect = sourceFiber.lastEffect = null;

if (enableUpdaterTracking) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(root, renderExpirationTime);
if (isDevToolsPresent) {
// If we have pending work still, restore the original updaters
restorePendingUpdaters(root, renderExpirationTime);
}
}

if (
Expand Down
Loading

0 comments on commit 0602449

Please sign in to comment.