Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[act] flush suspense fallbacks in tests #16240

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 98 additions & 6 deletions packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,36 +32,63 @@ describe('ReactTestUtils.act()', () => {
concurrentRoot = ReactDOM.unstable_createRoot(dom);
concurrentRoot.render(el);
}

function unmountConcurrent(_dom) {
if (concurrentRoot !== null) {
concurrentRoot.unmount();
concurrentRoot = null;
}
}
runActTests('concurrent mode', renderConcurrent, unmountConcurrent);

function rerenderConcurrent(el) {
concurrentRoot.render(el);
}

runActTests(
'concurrent mode',
renderConcurrent,
unmountConcurrent,
rerenderConcurrent,
);

// and then in sync mode

let syncDom = null;
function renderSync(el, dom) {
syncDom = dom;
ReactDOM.render(el, dom);
}

function unmountSync(dom) {
syncDom = null;
ReactDOM.unmountComponentAtNode(dom);
}
runActTests('legacy sync mode', renderSync, unmountSync);

function rerenderSync(el) {
ReactDOM.render(el, syncDom);
}

runActTests('legacy sync mode', renderSync, unmountSync, rerenderSync);

// and then in batched mode
let batchedRoot;
function renderBatched(el, dom) {
batchedRoot = ReactDOM.unstable_createSyncRoot(dom);
batchedRoot.render(el);
}

function unmountBatched(dom) {
if (batchedRoot !== null) {
batchedRoot.unmount();
batchedRoot = null;
}
}
runActTests('batched mode', renderBatched, unmountBatched);

function rerenderBatched(el) {
batchedRoot.render(el);
}

runActTests('batched mode', renderBatched, unmountBatched, rerenderBatched);

describe('unacted effects', () => {
function App() {
Expand Down Expand Up @@ -117,7 +144,7 @@ describe('ReactTestUtils.act()', () => {
});
});

function runActTests(label, render, unmount) {
function runActTests(label, render, unmount, rerender) {
describe(label, () => {
beforeEach(() => {
jest.resetModules();
Expand Down Expand Up @@ -546,7 +573,7 @@ function runActTests(label, render, unmount) {
expect(interactions.size).toBe(1);
expectedInteraction = Array.from(interactions)[0];

render(<Component />, container);
rerender(<Component />);
},
);
});
Expand Down Expand Up @@ -576,7 +603,7 @@ function runActTests(label, render, unmount) {
expect(interactions.size).toBe(1);
expectedInteraction = Array.from(interactions)[0];

render(<Component />, secondContainer);
rerender(<Component />);
});
},
);
Expand Down Expand Up @@ -693,5 +720,70 @@ function runActTests(label, render, unmount) {
}
});
});

describe('suspense', () => {
it('triggers fallbacks if available', async () => {
let resolved = false;
let resolve;
const promise = new Promise(_resolve => {
resolve = _resolve;
});

function Suspends() {
if (resolved) {
return 'was suspended';
}
throw promise;
}

function App(props) {
return (
<React.Suspense
fallback={<span data-test-id="spinner">loading...</span>}>
{props.suspend ? <Suspends /> : 'content'}
</React.Suspense>
);
}

// render something so there's content
act(() => {
render(<App suspend={false} />, container);
});

// trigger a suspendy update
act(() => {
rerender(<App suspend={true} />);
});
expect(document.querySelector('[data-test-id=spinner]')).not.toBeNull();

// now render regular content again
act(() => {
rerender(<App suspend={false} />);
});
expect(document.querySelector('[data-test-id=spinner]')).toBeNull();

// trigger a suspendy update with a delay
React.unstable_withSuspenseConfig(
() => {
act(() => {
rerender(<App suspend={true} />);
});
},
{timeout: 5000},
);
// the spinner shows up regardless
expect(document.querySelector('[data-test-id=spinner]')).not.toBeNull();

// resolve the promise
await act(async () => {
resolved = true;
resolve();
});

// spinner gone, content showing
expect(document.querySelector('[data-test-id=spinner]')).toBeNull();
expect(container.textContent).toBe('was suspended');
});
});
});
}
19 changes: 16 additions & 3 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
enableSchedulerTracing,
revertPassiveEffectsChange,
warnAboutUnmockedScheduler,
flushSuspenseFallbacksInTests,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -993,7 +994,7 @@ function renderRoot(
case RootIncomplete: {
invariant(false, 'Should have a work-in-progress.');
}
// Flow knows about invariant, so it compains if I add a break statement,
// Flow knows about invariant, so it complains if I add a break statement,
// but eslint doesn't know about invariant, so it complains if I do.
// eslint-disable-next-line no-fallthrough
case RootErrored: {
Expand Down Expand Up @@ -1027,7 +1028,12 @@ function renderRoot(
// possible.
const hasNotProcessedNewUpdates =
workInProgressRootLatestProcessedExpirationTime === Sync;
if (hasNotProcessedNewUpdates && !isSync) {
if (
hasNotProcessedNewUpdates &&
!isSync &&
// do not delay if we're inside an act() scope
!(flushSuspenseFallbacksInTests && IsThisRendererActing.current)
) {
// If we have not processed any new updates during this pass, then this is
// either a retry of an existing fallback state or a hidden tree.
// Hidden trees shouldn't be batched with other work and after that's
Expand Down Expand Up @@ -1064,7 +1070,11 @@ function renderRoot(
return commitRoot.bind(null, root);
}
case RootSuspendedWithDelay: {
if (!isSync) {
if (
!isSync &&
// do not delay if we're inside an act() scope
!(flushSuspenseFallbacksInTests && IsThisRendererActing.current)
) {
// We're suspended in a state that should be avoided. We'll try to avoid committing
// it for as long as the timeouts let us.
if (workInProgressRootHasPendingPing) {
Expand Down Expand Up @@ -1135,6 +1145,8 @@ function renderRoot(
// The work completed. Ready to commit.
if (
!isSync &&
// do not delay if we're inside an act() scope
!(flushSuspenseFallbacksInTests && IsThisRendererActing.current) &&
workInProgressRootLatestProcessedExpirationTime !== Sync &&
workInProgressRootCanSuspendUsingConfig !== null
) {
Expand Down Expand Up @@ -2439,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 warnAboutUnmockedScheduler = 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 warnAboutUnmockedScheduler = 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 warnAboutUnmockedScheduler = 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 warnAboutUnmockedScheduler = 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 warnAboutUnmockedScheduler = 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 warnAboutUnmockedScheduler = 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