@samitouri / QOS-React / commits / 0fc0446798

Class component can suspend without losing state outside concurrent mode (#13899)

Outside of concurrent mode, schedules a force update on a suspended class component to force it to prevent it from bailing out and reusing the current fiber, which we know to be inconsistent.

Andrew Clark committed Oct 19, 2018 at 18:41 UTC 0fc04467987c187d557744f27c8ba90ea0a06cad
4 files changed +169 -37
packages/react-reconciler/src/ReactFiberBeginWork.js
+33 -24
@@ -401,32 +401,41 @@ function updateClassComponent(
401 }
402 prepareToReadContext(workInProgress, renderExpirationTime);
403
404 + const instance = workInProgress.stateNode;
405 let shouldUpdate;
405 - if (current === null) {
406 - if (workInProgress.stateNode === null) {
407 - // In the initial pass we might need to construct the instance.
408 - constructClassInstance(
409 - workInProgress,
410 - Component,
411 - nextProps,
412 - renderExpirationTime,
413 - );
414 - mountClassInstance(
415 - workInProgress,
416 - Component,
417 - nextProps,
418 - renderExpirationTime,
419 - );
420 - shouldUpdate = true;
421 - } else {
422 - // In a resume, we'll already have an instance we can reuse.
423 - shouldUpdate = resumeMountClassInstance(
424 - workInProgress,
425 - Component,
426 - nextProps,
427 - renderExpirationTime,
428 - );
406 + if (instance === null) {
407 + if (current !== null) {
408 + // An class component without an instance only mounts if it suspended
409 + // inside a non- concurrent tree, in an inconsistent state. We want to
410 + // tree it like a new mount, even though an empty version of it already
411 + // committed. Disconnect the alternate pointers.
412 + current.alternate = null;
413 + workInProgress.alternate = null;
414 + // Since this is conceptually a new fiber, schedule a Placement effect
415 + workInProgress.effectTag |= Placement;
416 }
417 + // In the initial pass we might need to construct the instance.
418 + constructClassInstance(
419 + workInProgress,
420 + Component,
421 + nextProps,
422 + renderExpirationTime,
423 + );
424 + mountClassInstance(
425 + workInProgress,
426 + Component,
427 + nextProps,
428 + renderExpirationTime,
429 + );
430 + shouldUpdate = true;
431 + } else if (current === null) {
432 + // In a resume, we'll already have an instance we can reuse.
433 + shouldUpdate = resumeMountClassInstance(
434 + workInProgress,
435 + Component,
436 + nextProps,
437 + renderExpirationTime,
438 + );
439 } else {
440 shouldUpdate = updateClassInstance(
441 current,
packages/react-reconciler/src/ReactFiberScheduler.js
+18 -1
@@ -111,7 +111,12 @@ import {
111 computeInteractiveExpiration,
112 } from './ReactFiberExpirationTime';
113 import {ConcurrentMode, ProfileMode, NoContext} from './ReactTypeOfMode';
114 -import {enqueueUpdate, resetCurrentlyProcessingQueue} from './ReactUpdateQueue';
114 +import {
115 + enqueueUpdate,
116 + resetCurrentlyProcessingQueue,
117 + ForceUpdate,
118 + createUpdate,
119 +} from './ReactUpdateQueue';
120 import {createCapturedValue} from './ReactCapturedValue';
121 import {
122 isContextProvider as isLegacyContextProvider,
@@ -1604,6 +1609,18 @@ function retrySuspendedRoot(
1609 // fiber, too, since it already committed in an inconsistent state and
1610 // therefore does not have any pending work.
1611 scheduleWorkToRoot(sourceFiber, retryTime);
1612 + const sourceTag = sourceFiber.tag;
1613 + if (
1614 + (sourceTag === ClassComponent || sourceFiber === ClassComponentLazy) &&
1615 + sourceFiber.stateNode !== null
1616 + ) {
1617 + // When we try rendering again, we should not reuse the current fiber,
1618 + // since it's known to be in an inconsistent state. Use a force updte to
1619 + // prevent a bail out.
1620 + const update = createUpdate(retryTime);
1621 + update.tag = ForceUpdate;
1622 + enqueueUpdate(sourceFiber, update);
1623 + }
1624 }
1625
1626 const rootExpirationTime = root.expirationTime;
packages/react-reconciler/src/ReactFiberUnwindWork.js
+3 -12
@@ -18,7 +18,6 @@ import {unstable_wrap as Schedule_tracing_wrap} from 'scheduler/tracing';
18 import getComponentName from 'shared/getComponentName';
19 import warningWithoutStack from 'shared/warningWithoutStack';
20 import {
21 - FunctionComponent,
21 ClassComponent,
22 ClassComponentLazy,
23 HostRoot,
@@ -71,10 +70,6 @@ import {
70 import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority';
71 import {reconcileChildren} from './ReactFiberBeginWork';
72
74 -function NoopComponent() {
75 - return null;
76 -}
77 -
73 function createRootErrorUpdate(
74 fiber: Fiber,
75 errorInfo: CapturedValue<mixed>,
@@ -262,13 +257,9 @@ function throwException(
257 // callbacks. Remove all lifecycle effect tags.
258 sourceFiber.effectTag &= ~LifecycleEffectMask;
259 if (sourceFiber.alternate === null) {
265 - // We're about to mount a class component that doesn't have an
266 - // instance. Turn this into a dummy function component instead,
267 - // to prevent type errors. This is a bit weird but it's an edge
268 - // case and we're about to synchronously delete this
269 - // component, anyway.
270 - sourceFiber.tag = FunctionComponent;
271 - sourceFiber.type = NoopComponent;
260 + // Set the instance back to null. We use this as a heuristic to
261 + // detect that the fiber mounted in an inconsistent state.
262 + sourceFiber.stateNode = null;
263 }
264 }
265
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+115
@@ -321,4 +321,119 @@ describe('ReactSuspense', () => {
321 );
322 expect(ReactTestRenderer).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
323 });
324 +
325 + describe('outside concurrent mode', () => {
326 + it('a mounted class component can suspend without losing state', () => {
327 + class TextWithLifecycle extends React.Component {
328 + componentDidMount() {
329 + ReactTestRenderer.unstable_yield(`Mount [${this.props.text}]`);
330 + }
331 + componentDidUpdate() {
332 + ReactTestRenderer.unstable_yield(`Update [${this.props.text}]`);
333 + }
334 + componentWillUnmount() {
335 + ReactTestRenderer.unstable_yield(`Unmount [${this.props.text}]`);
336 + }
337 + render() {
338 + return <Text {...this.props} />;
339 + }
340 + }
341 +
342 + let instance;
343 + class AsyncTextWithLifecycle extends React.Component {
344 + state = {step: 1};
345 + componentDidMount() {
346 + ReactTestRenderer.unstable_yield(
347 + `Mount [${this.props.text}:${this.state.step}]`,
348 + );
349 + }
350 + componentDidUpdate() {
351 + ReactTestRenderer.unstable_yield(
352 + `Update [${this.props.text}:${this.state.step}]`,
353 + );
354 + }
355 + componentWillUnmount() {
356 + ReactTestRenderer.unstable_yield(
357 + `Unmount [${this.props.text}:${this.state.step}]`,
358 + );
359 + }
360 + render() {
361 + instance = this;
362 + const text = `${this.props.text}:${this.state.step}`;
363 + const ms = this.props.ms;
364 + try {
365 + TextResource.read(cache, [text, ms]);
366 + ReactTestRenderer.unstable_yield(text);
367 + return text;
368 + } catch (promise) {
369 + if (typeof promise.then === 'function') {
370 + ReactTestRenderer.unstable_yield(`Suspend! [${text}]`);
371 + } else {
372 + ReactTestRenderer.unstable_yield(`Error! [${text}]`);
373 + }
374 + throw promise;
375 + }
376 + }
377 + }
378 +
379 + function App() {
380 + return (
381 + <Suspense
382 + maxDuration={1000}
383 + fallback={<TextWithLifecycle text="Loading..." />}>
384 + <TextWithLifecycle text="A" />
385 + <AsyncTextWithLifecycle ms={100} text="B" ref={instance} />
386 + <TextWithLifecycle text="C" />
387 + </Suspense>
388 + );
389 + }
390 +
391 + const root = ReactTestRenderer.create(<App />);
392 +
393 + expect(ReactTestRenderer).toHaveYielded([
394 + 'A',
395 + 'Suspend! [B:1]',
396 + 'C',
397 +
398 + 'Mount [A]',
399 + // B's lifecycle should not fire because it suspended
400 + // 'Mount [B]',
401 + 'Mount [C]',
402 +
403 + // In a subsequent commit, render a placeholder
404 + 'Loading...',
405 + 'Mount [Loading...]',
406 + ]);
407 + expect(root).toMatchRenderedOutput('Loading...');
408 +
409 + jest.advanceTimersByTime(100);
410 + expect(ReactTestRenderer).toHaveYielded([
411 + 'Promise resolved [B:1]',
412 + 'B:1',
413 + 'Unmount [Loading...]',
414 + // Should be a mount, not an update
415 + 'Mount [B:1]',
416 + ]);
417 +
418 + expect(root).toMatchRenderedOutput('AB:1C');
419 +
420 + instance.setState({step: 2});
421 + expect(ReactTestRenderer).toHaveYielded([
422 + 'Suspend! [B:2]',
423 + 'Loading...',
424 + 'Mount [Loading...]',
425 + ]);
426 + expect(root).toMatchRenderedOutput('Loading...');
427 +
428 + jest.advanceTimersByTime(100);
429 +
430 + expect(ReactTestRenderer).toHaveYielded([
431 + 'Promise resolved [B:2]',
432 + 'B:2',
433 + 'Unmount [Loading...]',
434 + 'Update [B:2]',
435 + ]);
436 + expect(root).toMatchRenderedOutput('AB:2C');
437 + });
438 + });
439 });