Restart from root if promise pings before end of render phase (#13774)
* Restart from root if promise pings before end of render phase * Test that placeholder resolves successfully even if fallback render is pending
Andrew Clark committed
Oct 4, 2018 at 12:55 UTC
acc7f404ce68f9eaef5724b1b340e487013440c2
3 files changed
+78
-1
packages/react-reconciler/src/ReactFiberScheduler.js
+9
-1
@@ -111,7 +111,7 @@ import {
111
computeAsyncExpiration,
112
computeInteractiveExpiration,
113
} from './ReactFiberExpirationTime';
114
-import {ConcurrentMode, ProfileMode} from './ReactTypeOfMode';
114
+import {ConcurrentMode, ProfileMode, NoContext} from './ReactTypeOfMode';
115
import {enqueueUpdate, resetCurrentlyProcessingQueue} from './ReactUpdateQueue';
116
import {createCapturedValue} from './ReactCapturedValue';
117
import {
@@ -1597,6 +1597,14 @@ function retrySuspendedRoot(
1597
markPendingPriorityLevel(root, retryTime);
1598
}
1599
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
+
1608
scheduleWorkToRoot(fiber, retryTime);
1609
const rootExpirationTime = root.expirationTime;
1610
if (rootExpirationTime !== NoWork) {
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+53
@@ -189,4 +189,57 @@ describe('ReactSuspense', () => {
189
expect(root).toFlushAndYield(['B']);
190
expect(root).toMatchRenderedOutput('AB');
191
});
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
+ });
245
});
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+16
@@ -649,6 +649,22 @@ describe('ReactSuspenseWithNoopRenderer', () => {
649
expect(ReactNoop.getChildren()).toEqual([span('Async'), span('Sync')]);
650
});
651
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
+
668
it('throws a helpful error when an update is suspends without a placeholder', () => {
669
expect(() => {
670
ReactNoop.flushSync(() =>