Skip to content

Commit ba708fa

Browse files
authored
Remove ReactNoop.flushDeferredPri and flushUnitsOfWork (#14934)
* Remove ReactNoop.flushDeferredPri and flushUnitsOfWork Some of our older tests worked by counting how many times React checked whether it should yield to the main thread, instead of something publicly observable like how many times a component is rendered. Our newer tests have converged on a style where we push into a log and make assertions on the log. This pattern is less coupled to the implementation while still being sufficient to test performance optimizations, like resuming (whenever we add that back). This commit removes flushDeferredPri and flushUnitsOfWork and upgrades the affected tests. * Remove shouldYieldToRenderer indirection This wrapper is no longer necessary.
1 parent 920b0bb commit ba708fa

13 files changed

+309
-386
lines changed

packages/react-noop-renderer/src/createReactNoop.js

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -747,36 +747,27 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
747747
return NoopRenderer.findHostInstance(component);
748748
},
749749

750-
flushDeferredPri(timeout: number = Infinity): Array<mixed> {
751-
// The legacy version of this function decremented the timeout before
752-
// returning the new time.
753-
// TODO: Convert tests to use flushUnitsOfWork or flushAndYield instead.
754-
const n = timeout / 5 - 1;
755-
756-
let values = [];
750+
flush(): Array<mixed> {
751+
let values = yieldedValues || [];
752+
yieldedValues = null;
757753
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
758-
for (const value of flushUnitsOfWork(n)) {
754+
for (const value of flushUnitsOfWork(Infinity)) {
759755
values.push(...value);
760756
}
761757
return values;
762758
},
763759

764-
flush(): Array<mixed> {
765-
return ReactNoop.flushUnitsOfWork(Infinity);
766-
},
767-
768-
flushAndYield(
769-
unitsOfWork: number = Infinity,
770-
): Generator<Array<mixed>, void, void> {
771-
return flushUnitsOfWork(unitsOfWork);
772-
},
773-
774-
flushUnitsOfWork(n: number): Array<mixed> {
760+
// TODO: Should only be used via a Jest plugin (like we do with the
761+
// test renderer).
762+
unstable_flushNumberOfYields(n: number): Array<mixed> {
775763
let values = yieldedValues || [];
776764
yieldedValues = null;
777765
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
778-
for (const value of flushUnitsOfWork(n)) {
766+
for (const value of flushUnitsOfWork(Infinity)) {
779767
values.push(...value);
768+
if (values.length >= n) {
769+
break;
770+
}
780771
}
781772
return values;
782773
},
@@ -847,7 +838,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
847838
},
848839

849840
flushExpired(): Array<mixed> {
850-
return ReactNoop.flushUnitsOfWork(0);
841+
let values = yieldedValues || [];
842+
yieldedValues = null;
843+
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
844+
for (const value of flushUnitsOfWork(0)) {
845+
values.push(...value);
846+
}
847+
return values;
851848
},
852849

853850
yield(value: mixed) {

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,7 @@ function workLoop(isYieldy) {
12141214
}
12151215
} else {
12161216
// Flush asynchronous work until there's a higher priority event
1217-
while (nextUnitOfWork !== null && !shouldYieldToRenderer()) {
1217+
while (nextUnitOfWork !== null && !shouldYield()) {
12181218
nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
12191219
}
12201220
}
@@ -2007,7 +2007,7 @@ function onSuspend(
20072007
msUntilTimeout: number,
20082008
): void {
20092009
root.expirationTime = rootExpirationTime;
2010-
if (msUntilTimeout === 0 && !shouldYieldToRenderer()) {
2010+
if (msUntilTimeout === 0 && !shouldYield()) {
20112011
// Don't wait an additional tick. Commit the tree immediately.
20122012
root.pendingCommitExpirationTime = suspendedExpirationTime;
20132013
root.finishedWork = finishedWork;
@@ -2204,43 +2204,24 @@ function findHighestPriorityRoot() {
22042204
nextFlushedExpirationTime = highestPriorityWork;
22052205
}
22062206

2207-
// TODO: This wrapper exists because many of the older tests (the ones that use
2208-
// flushDeferredPri) rely on the number of times `shouldYield` is called. We
2209-
// should get rid of it.
2210-
let didYield: boolean = false;
2211-
function shouldYieldToRenderer() {
2212-
if (didYield) {
2213-
return true;
2214-
}
2215-
if (shouldYield()) {
2216-
didYield = true;
2217-
return true;
2218-
}
2219-
return false;
2220-
}
2221-
22222207
function performAsyncWork(didTimeout) {
2223-
try {
2224-
if (didTimeout) {
2225-
// The callback timed out. That means at least one update has expired.
2226-
// Iterate through the root schedule. If they contain expired work, set
2227-
// the next render expiration time to the current time. This has the effect
2228-
// of flushing all expired work in a single batch, instead of flushing each
2229-
// level one at a time.
2230-
if (firstScheduledRoot !== null) {
2231-
recomputeCurrentRendererTime();
2232-
let root: FiberRoot = firstScheduledRoot;
2233-
do {
2234-
didExpireAtExpirationTime(root, currentRendererTime);
2235-
// The root schedule is circular, so this is never null.
2236-
root = (root.nextScheduledRoot: any);
2237-
} while (root !== firstScheduledRoot);
2238-
}
2208+
if (didTimeout) {
2209+
// The callback timed out. That means at least one update has expired.
2210+
// Iterate through the root schedule. If they contain expired work, set
2211+
// the next render expiration time to the current time. This has the effect
2212+
// of flushing all expired work in a single batch, instead of flushing each
2213+
// level one at a time.
2214+
if (firstScheduledRoot !== null) {
2215+
recomputeCurrentRendererTime();
2216+
let root: FiberRoot = firstScheduledRoot;
2217+
do {
2218+
didExpireAtExpirationTime(root, currentRendererTime);
2219+
// The root schedule is circular, so this is never null.
2220+
root = (root.nextScheduledRoot: any);
2221+
} while (root !== firstScheduledRoot);
22392222
}
2240-
performWork(NoWork, true);
2241-
} finally {
2242-
didYield = false;
22432223
}
2224+
performWork(NoWork, true);
22442225
}
22452226

22462227
function performSyncWork() {
@@ -2266,7 +2247,7 @@ function performWork(minExpirationTime: ExpirationTime, isYieldy: boolean) {
22662247
nextFlushedRoot !== null &&
22672248
nextFlushedExpirationTime !== NoWork &&
22682249
minExpirationTime <= nextFlushedExpirationTime &&
2269-
!(didYield && currentRendererTime > nextFlushedExpirationTime)
2250+
!(shouldYield() && currentRendererTime > nextFlushedExpirationTime)
22702251
) {
22712252
performWorkOnRoot(
22722253
nextFlushedRoot,
@@ -2414,7 +2395,7 @@ function performWorkOnRoot(
24142395
if (finishedWork !== null) {
24152396
// We've completed the root. Check the if we should yield one more time
24162397
// before committing.
2417-
if (!shouldYieldToRenderer()) {
2398+
if (!shouldYield()) {
24182399
// Still time left. Commit the root.
24192400
completeRoot(root, finishedWork, expirationTime);
24202401
} else {

0 commit comments

Comments
 (0)