Skip to content

Commit

Permalink
Internal act: Flush timers at end of scope
Browse files Browse the repository at this point in the history
If there are any suspended fallbacks at the end of the `act` scope,
force them to display by running the pending timers (i.e. `setTimeout`).

The public implementation of `act` achieves the same behavior with an
extra check in the work loop (`shouldForceFlushFallbacks`). Since our
internal `act` needs to work in both development and production, without
additional runtime checks, we instead rely on Jest's mock timers.

This doesn't not affect refresh transitions, which are meant to delay
indefinitely, because in that case we exit the work loop without
posting a timer.
  • Loading branch information
acdlite committed Sep 8, 2020
1 parent d17086c commit 34f1d9d
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 35 deletions.
4 changes: 2 additions & 2 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ export function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
}

function flushActWork(resolve, reject) {
// TODO: Run timers to flush suspended fallbacks
// jest.runOnlyPendingTimers();
// Flush suspended fallbacks
jest.runOnlyPendingTimers();
enqueueTask(() => {
try {
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
Expand Down
4 changes: 2 additions & 2 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1175,8 +1175,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}

function flushActWork(resolve, reject) {
// TODO: Run timers to flush suspended fallbacks
// jest.runOnlyPendingTimers();
// Flush suspended fallbacks
jest.runOnlyPendingTimers();
enqueueTask(() => {
try {
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
Expand Down
18 changes: 13 additions & 5 deletions packages/react-reconciler/src/__tests__/ReactBlocks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let useState;
let Suspense;
let block;
let readString;
let resolvePromises;
let Scheduler;

describe('ReactBlocks', () => {
Expand All @@ -28,15 +29,16 @@ describe('ReactBlocks', () => {
useState = React.useState;
Suspense = React.Suspense;
const cache = new Map();
let unresolved = [];
readString = function(text) {
let entry = cache.get(text);
if (!entry) {
entry = {
promise: new Promise(resolve => {
setTimeout(() => {
unresolved.push(() => {
entry.resolved = true;
resolve();
}, 100);
});
}),
resolved: false,
};
Expand All @@ -47,6 +49,12 @@ describe('ReactBlocks', () => {
}
return text;
};

resolvePromises = () => {
const res = unresolved;
unresolved = [];
res.forEach(r => r());
};
});

// @gate experimental
Expand Down Expand Up @@ -144,7 +152,7 @@ describe('ReactBlocks', () => {
expect(ReactNoop).toMatchRenderedOutput('Loading...');

await ReactNoop.act(async () => {
jest.advanceTimersByTime(1000);
resolvePromises();
});

expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
Expand Down Expand Up @@ -291,7 +299,7 @@ describe('ReactBlocks', () => {
ReactNoop.render(<App Page={loadParent('Sebastian')} />);
});
await ReactNoop.act(async () => {
jest.advanceTimersByTime(1000);
resolvePromises();
});
expect(ReactNoop).toMatchRenderedOutput(<span>Name: Sebastian</span>);
});
Expand Down Expand Up @@ -336,7 +344,7 @@ describe('ReactBlocks', () => {
});
await ReactNoop.act(async () => {
_setSuspend(false);
jest.advanceTimersByTime(1000);
resolvePromises();
});
expect(ReactNoop).toMatchRenderedOutput(<span>Sebastian</span>);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
<Suspense fallback="Loading...">
<Text text="Sibling" />
{shouldSuspend ? (
<AsyncText ms={10000} text={'Step ' + step} />
<AsyncText text={'Step ' + step} />
) : (
<Text text={'Step ' + step} />
)}
Expand Down Expand Up @@ -2595,7 +2595,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
}
return (
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText text={page} ms={5000} />
<AsyncText text={page} />
</Suspense>
);
}
Expand All @@ -2616,8 +2616,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
});

// Later we load the data.
Scheduler.unstable_advanceTime(5000);
await advanceTimers(5000);
await resolveText('A');
expect(Scheduler).toHaveYielded(['Promise resolved [A]']);
expect(Scheduler).toFlushAndYield(['A']);
expect(ReactNoop.getChildren()).toEqual([span('A')]);
Expand All @@ -2635,8 +2634,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
});

// Later we load the data.
Scheduler.unstable_advanceTime(3000);
await advanceTimers(3000);
await resolveText('B');
expect(Scheduler).toHaveYielded(['Promise resolved [B]']);
expect(Scheduler).toFlushAndYield(['B']);
expect(ReactNoop.getChildren()).toEqual([span('B')]);
Expand Down Expand Up @@ -2754,12 +2752,14 @@ describe('ReactSuspenseWithNoopRenderer', () => {
);
});

// TODO: This test is specifically about avoided commits that suspend for a
// JND. We may remove this behavior.
it("suspended commit remains suspended even if there's another update at same expiration", async () => {
// Regression test
function App({text}) {
return (
<Suspense fallback="Loading...">
<AsyncText ms={2000} text={text} />
<AsyncText text={text} />
</Suspense>
);
}
Expand All @@ -2768,34 +2768,28 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await ReactNoop.act(async () => {
root.render(<App text="Initial" />);
});
expect(Scheduler).toHaveYielded(['Suspend! [Initial]']);

// Resolve initial render
await ReactNoop.act(async () => {
Scheduler.unstable_advanceTime(2000);
await advanceTimers(2000);
await resolveText('Initial');
});
expect(Scheduler).toHaveYielded([
'Suspend! [Initial]',
'Promise resolved [Initial]',
'Initial',
]);
expect(Scheduler).toHaveYielded(['Promise resolved [Initial]', 'Initial']);
expect(root).toMatchRenderedOutput(<span prop="Initial" />);

// Update. Since showing a fallback would hide content that's already
// visible, it should suspend for a bit without committing.
await ReactNoop.act(async () => {
// Update. Since showing a fallback would hide content that's already
// visible, it should suspend for a JND without committing.
root.render(<App text="First update" />);

expect(Scheduler).toFlushAndYield(['Suspend! [First update]']);

// Should not display a fallback
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
});

// Update again. This should also suspend for a bit.
await ReactNoop.act(async () => {
// Update again. This should also suspend for a JND.
root.render(<App text="Second update" />);

expect(Scheduler).toFlushAndYield(['Suspend! [Second update]']);

// Should not display a fallback
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
});
Expand Down Expand Up @@ -3989,9 +3983,6 @@ describe('ReactSuspenseWithNoopRenderer', () => {
await ReactNoop.act(async () => {
root.render(<App show={true} />);
});
// TODO: `act` should have already flushed the placeholder, so this
// runAllTimers call should be unnecessary.
jest.runAllTimers();
expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...']);
expect(root).toMatchRenderedOutput(
<>
Expand Down
5 changes: 3 additions & 2 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,9 @@ function unstable_concurrentAct(scope: () => Thenable<mixed> | void) {
}

function flushActWork(resolve, reject) {
// TODO: Run timers to flush suspended fallbacks
// jest.runOnlyPendingTimers();
// Flush suspended fallbacks
// $FlowFixMe - `jest` is not typed. We check for its existence above, though.
jest.runOnlyPendingTimers();
enqueueTask(() => {
try {
const didFlushWork = Scheduler.unstable_flushAllWithoutAsserting();
Expand Down

0 comments on commit 34f1d9d

Please sign in to comment.