Skip to content

Commit

Permalink
flush fallbacks in tests
Browse files Browse the repository at this point in the history
In this PR, for tests (specifically, code inside an `act()` scope), we immediately trigger work that would have otherwise required a timeout. This makes it simpler to tests loading/spinner states, and makes tests resilient to changes in React.

For some of our tests(specifically, ReactSuspenseWithNoopRenderer-test.internal), we _don't_ want fallbacks to immediately trigger, because we're testing intermediate states and such. Added a feature flag `flushSuspenseFallbacksInTests` to disable this behaviour on a per case basis.
  • Loading branch information
threepointone committed Jul 29, 2019
1 parent 75ab53b commit b6a9cd4
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 3 deletions.
23 changes: 20 additions & 3 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
enableProfilerTimer,
enableSchedulerTracing,
revertPassiveEffectsChange,
flushSuspenseFallbacksInTests,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -800,6 +801,21 @@ function prepareFreshStack(root, expirationTime) {
}
}

// a wrapper around scheduleTimeout, that immediately
// runs the callback when inside an act() scope
function scheduleWorkTimeout(callback, period) {
if (flushSuspenseFallbacksInTests === true) {
if (IsThisRendererActing.current === true) {
callback();
return noTimeout;
// what else could I return? does it matter? maybe an arbit counter?
}
// should it warn if you tried to suspend outside an act()?
}

return scheduleTimeout(callback, period);
}

function renderRoot(
root: FiberRoot,
expirationTime: ExpirationTime,
Expand Down Expand Up @@ -1049,7 +1065,7 @@ function renderRoot(
// The render is suspended, it hasn't timed out, and there's no lower
// priority work to do. Instead of committing the fallback
// immediately, wait for more data to arrive.
root.timeoutHandle = scheduleTimeout(
root.timeoutHandle = scheduleWorkTimeout(
commitRoot.bind(null, root),
msUntilTimeout,
);
Expand Down Expand Up @@ -1117,7 +1133,7 @@ function renderRoot(
// The render is suspended, it hasn't timed out, and there's no lower
// priority work to do. Instead of committing the fallback
// immediately, wait for more data to arrive.
root.timeoutHandle = scheduleTimeout(
root.timeoutHandle = scheduleWorkTimeout(
commitRoot.bind(null, root),
msUntilTimeout,
);
Expand All @@ -1143,7 +1159,7 @@ function renderRoot(
workInProgressRootCanSuspendUsingConfig,
);
if (msUntilTimeout > 10) {
root.timeoutHandle = scheduleTimeout(
root.timeoutHandle = scheduleWorkTimeout(
commitRoot.bind(null, root),
msUntilTimeout,
);
Expand Down Expand Up @@ -2435,6 +2451,7 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
}
}

// a 'shared' variable that changes when act() opens/closes in tests.
export const IsThisRendererActing = {current: (false: boolean)};

export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
React = require('react');
Fragment = React.Fragment;
ReactNoop = require('react-noop-renderer');
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export const warnAboutMissingMockScheduler = false;
// Temporary flag to revert the fix in #15650
export const revertPassiveEffectsChange = false;

// For tests, we flush suspense fallbacks in an act scope;
// *except* in some of our own tests, where we test incremental loading states.
export const flushSuspenseFallbacksInTests = true;

// Changes priority of some events like mousemove to user-blocking priority,
// but without making them discrete. The flag exists in case it causes
// starvation problems.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const enableFundamentalAPI = false;
export const enableJSXTransformAPI = false;
export const warnAboutMissingMockScheduler = true;
export const revertPassiveEffectsChange = false;
export const flushSuspenseFallbacksInTests = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
export const enableJSXTransformAPI = false;
export const warnAboutMissingMockScheduler = false;
export const revertPassiveEffectsChange = false;
export const flushSuspenseFallbacksInTests = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
export const enableJSXTransformAPI = false;
export const warnAboutMissingMockScheduler = true;
export const revertPassiveEffectsChange = false;
export const flushSuspenseFallbacksInTests = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const enableFundamentalAPI = false;
export const enableJSXTransformAPI = false;
export const warnAboutMissingMockScheduler = false;
export const revertPassiveEffectsChange = false;
export const flushSuspenseFallbacksInTests = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const enableFlareAPI = true;
export const enableFundamentalAPI = false;
export const enableJSXTransformAPI = true;
export const warnAboutMissingMockScheduler = true;
export const flushSuspenseFallbacksInTests = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = true;
export const warnAboutDefaultPropsOnFunctionComponents = false;
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ export const enableSuspenseCallback = true;

export const warnAboutDefaultPropsOnFunctionComponents = false;

export const flushSuspenseFallbacksInTests = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
type Check<_X, Y: _X, X: Y = _X> = null;
Expand Down

0 comments on commit b6a9cd4

Please sign in to comment.