Skip to content

Commit

Permalink
Support use in act testing API (#25523)
Browse files Browse the repository at this point in the history
`use` can avoid suspending on already resolved data by yielding to
microtasks. In a real, browser environment, we do this by scheduling a
platform task (i.e. postTask).

In a test environment, tasks are scheduled on a special internal queue
so that they can be flushed by the `act` testing API. So we need to add
support for this in `act`.

This behavior only works if you `await` the thenable returned by the
`act` call. We currently do not require that users do this. So I added a
warning, but it only fires if `use` was called. The old Suspense pattern
will not trigger a warning. This is to avoid breaking existing tests
that use Suspense.

The implementation of `act` has gotten extremely complicated because of
the subtle changes in behavior over the years, and our commitment to
maintaining backwards compatibility. We really should consider being
more restrictive in a future major release.

The changes are a bit confusing so I did my best to add inline comments
explaining how it works.

## Test plan

I ran this against Facebook's internal Jest test suite to confirm
nothing broke
  • Loading branch information
acdlite committed Oct 21, 2022
1 parent 65e32e5 commit c635807
Show file tree
Hide file tree
Showing 5 changed files with 380 additions and 118 deletions.
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactFiberWakeable.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import type {
RejectedThenable,
} from 'shared/ReactTypes';

import ReactSharedInternals from 'shared/ReactSharedInternals';
const {ReactCurrentActQueue} = ReactSharedInternals;

let suspendedThenable: Thenable<mixed> | null = null;
let adHocSuspendCount: number = 0;

Expand Down Expand Up @@ -124,6 +127,10 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
}
usedThenables[index] = thenable;
lastUsedThenable = thenable;

if (__DEV__ && ReactCurrentActQueue.current !== null) {
ReactCurrentActQueue.didUsePromise = true;
}
}

export function getPreviouslyUsedThenableAtIndex<T>(
Expand Down
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactFiberWakeable.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import type {
RejectedThenable,
} from 'shared/ReactTypes';

import ReactSharedInternals from 'shared/ReactSharedInternals';
const {ReactCurrentActQueue} = ReactSharedInternals;

let suspendedThenable: Thenable<mixed> | null = null;
let adHocSuspendCount: number = 0;

Expand Down Expand Up @@ -124,6 +127,10 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number) {
}
usedThenables[index] = thenable;
lastUsedThenable = thenable;

if (__DEV__ && ReactCurrentActQueue.current !== null) {
ReactCurrentActQueue.didUsePromise = true;
}
}

export function getPreviouslyUsedThenableAtIndex<T>(
Expand Down
157 changes: 157 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,22 @@
let React;
let ReactNoop;
let act;
let use;
let Suspense;
let DiscreteEventPriority;
let startTransition;

describe('isomorphic act()', () => {
beforeEach(() => {
React = require('react');

ReactNoop = require('react-noop-renderer');
DiscreteEventPriority = require('react-reconciler/constants')
.DiscreteEventPriority;
act = React.unstable_act;
use = React.experimental_use;
Suspense = React.Suspense;
startTransition = React.startTransition;
});

beforeEach(() => {
Expand Down Expand Up @@ -133,4 +140,154 @@ describe('isomorphic act()', () => {
expect(root).toMatchRenderedOutput('C');
});
});

// @gate __DEV__
// @gate enableUseHook
test('unwraps promises by yielding to microtasks (async act scope)', async () => {
const promise = Promise.resolve('Async');

function Fallback() {
throw new Error('Fallback should never be rendered');
}

function App() {
return use(promise);
}

const root = ReactNoop.createRoot();
await act(async () => {
startTransition(() => {
root.render(
<Suspense fallback={<Fallback />}>
<App />
</Suspense>,
);
});
});
expect(root).toMatchRenderedOutput('Async');
});

// @gate __DEV__
// @gate enableUseHook
test('unwraps promises by yielding to microtasks (non-async act scope)', async () => {
const promise = Promise.resolve('Async');

function Fallback() {
throw new Error('Fallback should never be rendered');
}

function App() {
return use(promise);
}

const root = ReactNoop.createRoot();

// Note that the scope function is not an async function
await act(() => {
startTransition(() => {
root.render(
<Suspense fallback={<Fallback />}>
<App />
</Suspense>,
);
});
});
expect(root).toMatchRenderedOutput('Async');
});

// @gate __DEV__
// @gate enableUseHook
test('warns if a promise is used in a non-awaited `act` scope', async () => {
const promise = new Promise(() => {});

function Fallback() {
throw new Error('Fallback should never be rendered');
}

function App() {
return use(promise);
}

spyOnDev(console, 'error');
const root = ReactNoop.createRoot();
act(() => {
startTransition(() => {
root.render(
<Suspense fallback={<Fallback />}>
<App />
</Suspense>,
);
});
});

// `act` warns after a few microtasks, instead of a macrotask, so that it's
// more likely to be attributed to the correct test case.
//
// The exact number of microtasks is an implementation detail; just needs
// to happen when the microtask queue is flushed.
await null;
await null;
await null;

expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Warning: A component suspended inside an `act` scope, but the `act` ' +
'call was not awaited. When testing React components that ' +
'depend on asynchronous data, you must await the result:\n\n' +
'await act(() => ...)',
);
});

// @gate __DEV__
test('does not warn when suspending via legacy `throw` API in non-awaited `act` scope', async () => {
let didResolve = false;
let resolvePromise;
const promise = new Promise(r => {
resolvePromise = () => {
didResolve = true;
r();
};
});

function Fallback() {
return 'Loading...';
}

function App() {
if (!didResolve) {
throw promise;
}
return 'Async';
}

spyOnDev(console, 'error');
const root = ReactNoop.createRoot();
act(() => {
startTransition(() => {
root.render(
<Suspense fallback={<Fallback />}>
<App />
</Suspense>,
);
});
});
expect(root).toMatchRenderedOutput('Loading...');

// `act` warns after a few microtasks, instead of a macrotask, so that it's
// more likely to be attributed to the correct test case.
//
// The exact number of microtasks is an implementation detail; just needs
// to happen when the microtask queue is flushed.
await null;
await null;
await null;

expect(console.error.calls.count()).toBe(0);

// Finish loading the data
await act(async () => {
resolvePromise();
});
expect(root).toMatchRenderedOutput('Async');
});
});
Loading

0 comments on commit c635807

Please sign in to comment.