Skip to content

Commit f9fd3f6

Browse files
acdlitelinjiajian999
authored andcommitted
Restart from root if promise pings before end of render phase (facebook#13774)
* Restart from root if promise pings before end of render phase * Test that placeholder resolves successfully even if fallback render is pending
1 parent 2411c8d commit f9fd3f6

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ import {
111111
computeAsyncExpiration,
112112
computeInteractiveExpiration,
113113
} from './ReactFiberExpirationTime';
114-
import {ConcurrentMode, ProfileMode} from './ReactTypeOfMode';
114+
import {ConcurrentMode, ProfileMode, NoContext} from './ReactTypeOfMode';
115115
import {enqueueUpdate, resetCurrentlyProcessingQueue} from './ReactUpdateQueue';
116116
import {createCapturedValue} from './ReactCapturedValue';
117117
import {
@@ -1597,6 +1597,14 @@ function retrySuspendedRoot(
15971597
markPendingPriorityLevel(root, retryTime);
15981598
}
15991599

1600+
if ((fiber.mode & ConcurrentMode) !== NoContext) {
1601+
if (root === nextRoot && nextRenderExpirationTime === suspendedTime) {
1602+
// Received a ping at the same priority level at which we're currently
1603+
// rendering. Restart from the root.
1604+
nextRoot = null;
1605+
}
1606+
}
1607+
16001608
scheduleWorkToRoot(fiber, retryTime);
16011609
const rootExpirationTime = root.expirationTime;
16021610
if (rootExpirationTime !== NoWork) {

packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,57 @@ describe('ReactSuspense', () => {
189189
expect(root).toFlushAndYield(['B']);
190190
expect(root).toMatchRenderedOutput('AB');
191191
});
192+
193+
it('interrupts current render if promise resolves before current render phase', () => {
194+
let didResolve = false;
195+
let listeners = [];
196+
197+
const thenable = {
198+
then(resolve) {
199+
if (!didResolve) {
200+
listeners.push(resolve);
201+
} else {
202+
resolve();
203+
}
204+
},
205+
};
206+
207+
function resolveThenable() {
208+
didResolve = true;
209+
listeners.forEach(l => l());
210+
}
211+
212+
function Async() {
213+
if (!didResolve) {
214+
ReactTestRenderer.unstable_yield('Suspend!');
215+
throw thenable;
216+
}
217+
ReactTestRenderer.unstable_yield('Async');
218+
return 'Async';
219+
}
220+
221+
const root = ReactTestRenderer.create(
222+
<Placeholder delayMs={1000} fallback={<Text text="Loading..." />}>
223+
<Async />
224+
<Text text="Sibling" />
225+
</Placeholder>,
226+
{
227+
unstable_isConcurrent: true,
228+
},
229+
);
230+
231+
expect(root).toFlushAndYieldThrough(['Suspend!']);
232+
233+
// The promise resolves before the current render phase has completed
234+
resolveThenable();
235+
expect(ReactTestRenderer).toHaveYielded([]);
236+
237+
// Start over from the root, instead of continuing.
238+
expect(root).toFlushAndYield([
239+
// Async renders again *before* Sibling
240+
'Async',
241+
'Sibling',
242+
]);
243+
expect(root).toMatchRenderedOutput('AsyncSibling');
244+
});
192245
});

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,22 @@ describe('ReactSuspenseWithNoopRenderer', () => {
649649
expect(ReactNoop.getChildren()).toEqual([span('Async'), span('Sync')]);
650650
});
651651

652+
it('resolves successfully even if fallback render is pending', async () => {
653+
ReactNoop.render(
654+
<Placeholder delayMs={1000} fallback={<Text text="Loading..." />}>
655+
<AsyncText text="Async" ms={3000} />
656+
</Placeholder>,
657+
);
658+
expect(ReactNoop.flushNextYield()).toEqual(['Suspend! [Async]']);
659+
await advanceTimers(1500);
660+
expect(ReactNoop.expire(1500)).toEqual([]);
661+
// Before we have a chance to flush, the promise resolves.
662+
await advanceTimers(2000);
663+
expect(ReactNoop.clearYields()).toEqual(['Promise resolved [Async]']);
664+
expect(ReactNoop.flush()).toEqual(['Async']);
665+
expect(ReactNoop.getChildren()).toEqual([span('Async')]);
666+
});
667+
652668
it('throws a helpful error when an update is suspends without a placeholder', () => {
653669
expect(() => {
654670
ReactNoop.flushSync(() =>

0 commit comments

Comments
 (0)