@samitouri / QOS-React / commits / cbbc2b6c4d

[Synchronous Suspense] Suspending a class outside concurrent mode (#13926)

* [Synchronous Suspense] Suspending a class outside concurrent mode When a class component suspends during mount outside concurrent mode, change the tag so it's not mistaken for a completed component. For example, we should not call componentWillUnmount if it is deleted. * PR nits

Andrew Clark committed Oct 23, 2018 at 11:36 UTC cbbc2b6c4d0d8519145560bd8183ecde55168b12

7 files changed +148 -6

packages/react-reconciler/src/ReactFiberBeginWork.js

+75 -1

@@ -33,6 +33,7 @@ import {

33 MemoComponent,

34 SimpleMemoComponent,

35 LazyComponent,

36 + IncompleteClassComponent,

37 } from 'shared/ReactWorkTags';

38 import {

39 NoEffect,

@@ -762,7 +763,7 @@ function mountLazyComponent(

763 renderExpirationTime,

764 ) {

765 if (_current !== null) {

765 - // An indeterminate component only mounts if it suspended inside a non-

766 + // An lazy component only mounts if it suspended inside a non-

767 // concurrent tree, in an inconsistent state. We want to tree it like

768 // a new mount, even though an empty version of it already committed.

769 // Disconnect the alternate pointers.

@@ -840,6 +841,64 @@ function mountLazyComponent(

841 return child;

842 }

843

844 +function mountIncompleteClassComponent(

845 + _current,

846 + workInProgress,

847 + Component,

848 + nextProps,

849 + renderExpirationTime,

850 +) {

851 + if (_current !== null) {

852 + // An incomplete component only mounts if it suspended inside a non-

853 + // concurrent tree, in an inconsistent state. We want to tree it like

854 + // a new mount, even though an empty version of it already committed.

855 + // Disconnect the alternate pointers.

856 + _current.alternate = null;

857 + workInProgress.alternate = null;

858 + // Since this is conceptually a new fiber, schedule a Placement effect

859 + workInProgress.effectTag |= Placement;

860 + }

861 +

862 + // Promote the fiber to a class and try rendering again.

863 + workInProgress.tag = ClassComponent;

864 +

865 + // The rest of this function is a fork of `updateClassComponent`

866 +

867 + // Push context providers early to prevent context stack mismatches.

868 + // During mounting we don't know the child context yet as the instance doesn't exist.

869 + // We will invalidate the child context in finishClassComponent() right after rendering.

870 + let hasContext;

871 + if (isLegacyContextProvider(Component)) {

872 + hasContext = true;

873 + pushLegacyContextProvider(workInProgress);

874 + } else {

875 + hasContext = false;

876 + }

877 + prepareToReadContext(workInProgress, renderExpirationTime);

878 +

879 + constructClassInstance(

880 + workInProgress,

881 + Component,

882 + nextProps,

883 + renderExpirationTime,

884 + );

885 + mountClassInstance(

886 + workInProgress,

887 + Component,

888 + nextProps,

889 + renderExpirationTime,

890 + );

891 +

892 + return finishClassComponent(

893 + null,

894 + workInProgress,

895 + Component,

896 + true,

897 + hasContext,

898 + renderExpirationTime,

899 + );

900 +}

901 +

902 function mountIndeterminateComponent(

903 _current,

904 workInProgress,

@@ -1654,6 +1713,21 @@ function beginWork(

1713 renderExpirationTime,

1714 );

1715 }

1716 + case IncompleteClassComponent: {

1717 + const Component = workInProgress.type;

1718 + const unresolvedProps = workInProgress.pendingProps;

1719 + const resolvedProps =

1720 + workInProgress.elementType === Component

1721 + ? unresolvedProps

1722 + : resolveDefaultProps(Component, unresolvedProps);

1723 + return mountIncompleteClassComponent(

1724 + current,

1725 + workInProgress,

1726 + Component,

1727 + resolvedProps,

1728 + renderExpirationTime,

1729 + );

1730 + }

1731 default:

1732 invariant(

1733 false,

packages/react-reconciler/src/ReactFiberCommitWork.js

+14 -1

@@ -32,6 +32,7 @@ import {

32 HostPortal,

33 Profiler,

34 SuspenseComponent,

35 + IncompleteClassComponent,

36 } from 'shared/ReactWorkTags';

37 import {

38 invokeGuardedCallback,

@@ -221,6 +222,7 @@ function commitBeforeMutationLifeCycles(

222 case HostComponent:

223 case HostText:

224 case HostPortal:

225 + case IncompleteClassComponent:

226 // Nothing to do for these component types

227 return;

228 default: {

@@ -392,6 +394,8 @@ function commitLifeCycles(

394 }

395 return;

396 }

397 + case IncompleteClassComponent:

398 + break;

399 default: {

400 invariant(

401 false,

@@ -495,7 +499,13 @@ function commitUnmount(current: Fiber): void {

499 case ClassComponent: {

500 safelyDetachRef(current);

501 const instance = current.stateNode;

498 - if (typeof instance.componentWillUnmount === 'function') {

502 + if (

503 + // Typically, a component that mounted will have an instance. However,

504 + // outside of concurrent mode, a suspended component may commit without

505 + // an instance, so we need to check whether it exists.

506 + instance !== null &&

507 + typeof instance.componentWillUnmount === 'function'

508 + ) {

509 safelyCallComponentWillUnmount(current, instance);

510 }

511 return;

@@ -924,6 +934,9 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {

934 case SuspenseComponent: {

935 return;

936 }

937 + case IncompleteClassComponent: {

938 + return;

939 + }

940 default: {

941 invariant(

942 false,

packages/react-reconciler/src/ReactFiberCompleteWork.js

+10

@@ -37,6 +37,7 @@ import {

37 MemoComponent,

38 SimpleMemoComponent,

39 LazyComponent,

40 + IncompleteClassComponent,

41 } from 'shared/ReactWorkTags';

42 import {Placement, Ref, Update} from 'shared/ReactSideEffectTags';

43 import invariant from 'shared/invariant';

@@ -717,6 +718,15 @@ function completeWork(

718 break;

719 case MemoComponent:

720 break;

721 + case IncompleteClassComponent: {

722 + // Same as class component case. I put it down here so that the tags are

723 + // sequential to ensure this switch is compiled to a jump table.

724 + const Component = workInProgress.type;

725 + if (isLegacyContextProvider(Component)) {

726 + popLegacyContext(workInProgress);

727 + }

728 + break;

729 + }

730 default:

731 invariant(

732 false,

packages/react-reconciler/src/ReactFiberUnwindWork.js

+7 -4

@@ -25,6 +25,7 @@ import {

25 HostPortal,

26 ContextProvider,

27 SuspenseComponent,

28 + IncompleteClassComponent,

29 } from 'shared/ReactWorkTags';

30 import {

31 DidCapture,

@@ -252,10 +253,12 @@ function throwException(

253 // But we shouldn't call any lifecycle methods or callbacks. Remove

254 // all lifecycle effect tags.

255 sourceFiber.effectTag &= ~LifecycleEffectMask;

255 - if (sourceFiber.alternate === null) {

256 - // Set the instance back to null. We use this as a heuristic to

257 - // detect that the fiber mounted in an inconsistent state.

258 - sourceFiber.stateNode = null;

256 + const current = sourceFiber.alternate;

257 + if (current === null) {

258 + // This is a new mount. Change the tag so it's not mistaken for a

259 + // completed component. For example, we should not call

260 + // componentWillUnmount if it is deleted.

261 + sourceFiber.tag = IncompleteClassComponent;

262 }

263 }

264

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

+39

@@ -492,6 +492,45 @@ describe('ReactSuspense', () => {

492 expect(root).toMatchRenderedOutput('Stateful: 2B');

493 });

494

495 + it('suspends in a class that has componentWillUnmount and is then deleted', () => {

496 + class AsyncTextWithUnmount extends React.Component {

497 + componentWillUnmount() {

498 + ReactTestRenderer.unstable_yield('will unmount');

499 + }

500 + render() {

501 + const text = this.props.text;

502 + const ms = this.props.ms;

503 + try {

504 + TextResource.read(cache, [text, ms]);

505 + ReactTestRenderer.unstable_yield(text);

506 + return text;

507 + } catch (promise) {

508 + if (typeof promise.then === 'function') {

509 + ReactTestRenderer.unstable_yield(`Suspend! [${text}]`);

510 + } else {

511 + ReactTestRenderer.unstable_yield(`Error! [${text}]`);

512 + }

513 + throw promise;

514 + }

515 + }

516 + }

517 +

518 + function App({text}) {

519 + return (

520 + <Suspense fallback={<Text text="Loading..." />}>

521 + <AsyncTextWithUnmount text={text} ms={100} />

522 + </Suspense>

523 + );

524 + }

525 +

526 + const root = ReactTestRenderer.create(<App text="A" />);

527 + expect(ReactTestRenderer).toHaveYielded(['Suspend! [A]', 'Loading...']);

528 + root.update(<Text text="B" />);

529 + // Should not fire componentWillUnmount

530 + expect(ReactTestRenderer).toHaveYielded(['B']);

531 + expect(root).toMatchRenderedOutput('B');

532 + });

533 +

534 it('retries when an update is scheduled on a timed out tree', () => {

535 let instance;

536 class Stateful extends React.Component {

packages/react-test-renderer/src/ReactTestRenderer.js

@@ -29,6 +29,7 @@ import {

29 Profiler,

30 MemoComponent,

31 SimpleMemoComponent,

32 + IncompleteClassComponent,

33 } from 'shared/ReactWorkTags';

34 import invariant from 'shared/invariant';

35 import ReactVersion from 'shared/ReactVersion';

@@ -193,6 +194,7 @@ function toTree(node: ?Fiber) {

194 case Profiler:

195 case ForwardRef:

196 case MemoComponent:

197 + case IncompleteClassComponent:

198 return childrenToTree(node.child);

199 default:

200 invariant(

packages/shared/ReactWorkTags.js

@@ -45,3 +45,4 @@ export const SuspenseComponent = 13;

45 export const MemoComponent = 14;

46 export const SimpleMemoComponent = 15;

47 export const LazyComponent = 16;

48 +export const IncompleteClassComponent = 17;