Suspending inside a constructor outside of strict mode (#13200)
* Suspending inside a constructor outside of strict mode Outside of strict mode, suspended components commit in an incomplete state, then are synchronously deleted in a subsequent commit. If a component suspends inside the constructor, it mounts without an instance. This breaks at least one invariant: during deletion, we assume that every mounted component has an instance, and check the instance for the existence of `componentWillUnmount`. Rather than add a redundant check to the deletion of every class component, components that suspend inside their constructor and outside of strict mode are turned into empty functional components before they are mounted. This is a bit weird, but it's an edge case, and the empty component will be synchronously unmounted regardless. * Do not fire lifecycles of a suspended component In non-strict mode, suspended components commit, but their lifecycles should not fire.
Andrew Clark committed
Jul 13, 2018 at 11:24 UTC
43ffae2d1750ee32409e140e41ea562ff46d2516
3 files changed
+140
packages/react-reconciler/src/ReactFiberUnwindWork.js
+21
@@ -30,6 +30,7 @@ import {
30
NoEffect,
31
ShouldCapture,
32
Update as UpdateEffect,
33
+ LifecycleEffectMask,
34
} from 'shared/ReactTypeOfSideEffect';
35
import {
36
enableGetDerivedStateFromCatch,
@@ -71,6 +72,10 @@ import {
72
import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority';
73
import {reconcileChildrenAtExpirationTime} from './ReactFiberBeginWork';
74
75
+function NoopComponent() {
76
+ return null;
77
+}
78
+
79
function createRootErrorUpdate(
80
fiber: Fiber,
81
errorInfo: CapturedValue<mixed>,
@@ -246,6 +251,22 @@ function throwException(
251
sourceFiber.tag = FunctionalComponent;
252
}
253
254
+ if (sourceFiber.tag === ClassComponent) {
255
+ // We're going to commit this fiber even though it didn't
256
+ // complete. But we shouldn't call any lifecycle methods or
257
+ // callbacks. Remove all lifecycle effect tags.
258
+ sourceFiber.effectTag &= ~LifecycleEffectMask;
259
+ if (sourceFiber.alternate === null) {
260
+ // We're about to mount a class component that doesn't have an
261
+ // instance. Turn this into a dummy functional component instead,
262
+ // to prevent type errors. This is a bit weird but it's an edge
263
+ // case and we're about to synchronously delete this
264
+ // component, anyway.
265
+ sourceFiber.tag = FunctionalComponent;
266
+ sourceFiber.type = NoopComponent;
267
+ }
268
+ }
269
+
270
// Exit without suspending.
271
return;
272
}
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+116
@@ -1282,6 +1282,122 @@ describe('ReactSuspense', () => {
1282
span('C'),
1283
]);
1284
});
1285
+
1286
+ it('suspends inside constructor', async () => {
1287
+ class AsyncTextInConstructor extends React.Component {
1288
+ constructor(props) {
1289
+ super(props);
1290
+ const text = props.text;
1291
+ try {
1292
+ TextResource.read(cache, [props.text, props.ms]);
1293
+ this.state = {text};
1294
+ } catch (promise) {
1295
+ if (typeof promise.then === 'function') {
1296
+ ReactNoop.yield(`Suspend! [${text}]`);
1297
+ } else {
1298
+ ReactNoop.yield(`Error! [${text}]`);
1299
+ }
1300
+ throw promise;
1301
+ }
1302
+ }
1303
+ render() {
1304
+ ReactNoop.yield(this.state.text);
1305
+ return <span prop={this.state.text} />;
1306
+ }
1307
+ }
1308
+
1309
+ ReactNoop.renderLegacySyncRoot(
1310
+ <Placeholder fallback={<Text text="Loading..." />}>
1311
+ <AsyncTextInConstructor ms={100} text="Hi" />
1312
+ </Placeholder>,
1313
+ );
1314
+
1315
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1316
+ });
1317
+ });
1318
+
1319
+ it('does not call lifecycles of a suspended component', async () => {
1320
+ class TextWithLifecycle extends React.Component {
1321
+ componentDidMount() {
1322
+ ReactNoop.yield(`Mount [${this.props.text}]`);
1323
+ }
1324
+ componentDidUpdate() {
1325
+ ReactNoop.yield(`Update [${this.props.text}]`);
1326
+ }
1327
+ componentWillUnmount() {
1328
+ ReactNoop.yield(`Unmount [${this.props.text}]`);
1329
+ }
1330
+ render() {
1331
+ return <Text {...this.props} />;
1332
+ }
1333
+ }
1334
+
1335
+ class AsyncTextWithLifecycle extends React.Component {
1336
+ componentDidMount() {
1337
+ ReactNoop.yield(`Mount [${this.props.text}]`);
1338
+ }
1339
+ componentDidUpdate() {
1340
+ ReactNoop.yield(`Update [${this.props.text}]`);
1341
+ }
1342
+ componentWillUnmount() {
1343
+ ReactNoop.yield(`Unmount [${this.props.text}]`);
1344
+ }
1345
+ render() {
1346
+ const text = this.props.text;
1347
+ const ms = this.props.ms;
1348
+ try {
1349
+ TextResource.read(cache, [text, ms]);
1350
+ ReactNoop.yield(text);
1351
+ return <span prop={text} />;
1352
+ } catch (promise) {
1353
+ if (typeof promise.then === 'function') {
1354
+ ReactNoop.yield(`Suspend! [${text}]`);
1355
+ } else {
1356
+ ReactNoop.yield(`Error! [${text}]`);
1357
+ }
1358
+ throw promise;
1359
+ }
1360
+ }
1361
+ }
1362
+
1363
+ function App() {
1364
+ return (
1365
+ <Placeholder
1366
+ delayMs={1000}
1367
+ fallback={<TextWithLifecycle text="Loading..." />}>
1368
+ <TextWithLifecycle text="A" />
1369
+ <AsyncTextWithLifecycle ms={100} text="B" />
1370
+ <TextWithLifecycle text="C" />
1371
+ </Placeholder>
1372
+ );
1373
+ }
1374
+
1375
+ ReactNoop.renderLegacySyncRoot(<App />, () =>
1376
+ ReactNoop.yield('Commit root'),
1377
+ );
1378
+ expect(ReactNoop.clearYields()).toEqual([
1379
+ 'A',
1380
+ 'Suspend! [B]',
1381
+ 'C',
1382
+
1383
+ 'Mount [A]',
1384
+ // B's lifecycle should not fire because it suspended
1385
+ // 'Mount [B]',
1386
+ 'Mount [C]',
1387
+ 'Commit root',
1388
+
1389
+ // In a subsequent commit, render a placeholder
1390
+ 'Loading...',
1391
+
1392
+ // A, B, and C are unmounted, but we skip calling B's componentWillUnmount
1393
+ 'Unmount [A]',
1394
+ 'Unmount [C]',
1395
+
1396
+ // Force delete all the existing children when switching to the
1397
+ // placeholder. This should be a mount, not an update.
1398
+ 'Mount [Loading...]',
1399
+ ]);
1400
+ expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1401
});
1402
});
1403
packages/shared/ReactTypeOfSideEffect.js
+3
@@ -24,6 +24,9 @@ export const DidCapture = /* */ 0b00001000000;
24
export const Ref = /* */ 0b00010000000;
25
export const Snapshot = /* */ 0b00100000000;
26
27
+// Update & Callback & Ref & Snapshot
28
+export const LifecycleEffectMask = /* */ 0b00110100100;
29
+
30
// Union of all host effects
31
export const HostEffectMask = /* */ 0b00111111111;
32