Remove false positive warning and add TODOs about `current` being non-null (#14821)
* Failing test for false positive warning * Add tests for forwardRef too * Remove the warning and add TODOs
Dan Abramov committed
Feb 13, 2019 at 00:00 UTC
c6bee765ba865298c69acdea70e1ec2d79f69efe
3 files changed
+101
-11
packages/react-reconciler/src/ReactFiberBeginWork.js
+8
@@ -220,6 +220,10 @@ function updateForwardRef(
220
nextProps: any,
221
renderExpirationTime: ExpirationTime,
222
) {
223
+ // TODO: current can be non-null here even if the component
224
+ // hasn't yet mounted. This happens after the first render suspends.
225
+ // We'll need to figure out if this is fine or can cause issues.
226
+
227
if (__DEV__) {
228
if (workInProgress.type !== workInProgress.elementType) {
229
// Lazy component props can't be validated in createElement
@@ -415,6 +419,10 @@ function updateSimpleMemoComponent(
419
updateExpirationTime,
420
renderExpirationTime: ExpirationTime,
421
): null | Fiber {
422
+ // TODO: current can be non-null here even if the component
423
+ // hasn't yet mounted. This happens when the inner render suspends.
424
+ // We'll need to figure out if this is fine or can cause issues.
425
+
426
if (__DEV__) {
427
if (workInProgress.type !== workInProgress.elementType) {
428
// Lazy component props can't be validated in createElement
packages/react-reconciler/src/ReactFiberHooks.js
-11
@@ -449,17 +449,6 @@ function mountWorkInProgressHook(): Hook {
449
450
if (__DEV__) {
451
(hook: any)._debugType = (currentHookNameInDev: any);
452
- if (
453
- currentlyRenderingFiber !== null &&
454
- currentlyRenderingFiber.alternate !== null
455
- ) {
456
- warning(
457
- false,
458
- '%s: Rendered more hooks than during the previous render. This is ' +
459
- 'not currently supported and may lead to unexpected behavior.',
460
- getComponentName(currentlyRenderingFiber.type),
461
- );
462
- }
452
}
453
if (workInProgressHook === null) {
454
// This is the first hook in the list
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+93
@@ -1366,4 +1366,97 @@ describe('ReactHooks', () => {
1366
),
1367
).toThrow('Hello');
1368
});
1369
+
1370
+ // Regression test for https://github.com/facebook/react/issues/14790
1371
+ it('does not fire a false positive warning when suspending memo', async () => {
1372
+ const {Suspense, useState} = React;
1373
+
1374
+ let wasSuspended = false;
1375
+ function trySuspend() {
1376
+ if (!wasSuspended) {
1377
+ throw new Promise(resolve => {
1378
+ wasSuspended = true;
1379
+ resolve();
1380
+ });
1381
+ }
1382
+ }
1383
+
1384
+ function Child() {
1385
+ useState();
1386
+ trySuspend();
1387
+ return 'hello';
1388
+ }
1389
+
1390
+ const Wrapper = React.memo(Child);
1391
+ const root = ReactTestRenderer.create(
1392
+ <Suspense fallback="loading">
1393
+ <Wrapper />
1394
+ </Suspense>,
1395
+ );
1396
+ expect(root).toMatchRenderedOutput('loading');
1397
+ await Promise.resolve();
1398
+ expect(root).toMatchRenderedOutput('hello');
1399
+ });
1400
+
1401
+ // Regression test for https://github.com/facebook/react/issues/14790
1402
+ it('does not fire a false positive warning when suspending forwardRef', async () => {
1403
+ const {Suspense, useState} = React;
1404
+
1405
+ let wasSuspended = false;
1406
+ function trySuspend() {
1407
+ if (!wasSuspended) {
1408
+ throw new Promise(resolve => {
1409
+ wasSuspended = true;
1410
+ resolve();
1411
+ });
1412
+ }
1413
+ }
1414
+
1415
+ function render(props, ref) {
1416
+ useState();
1417
+ trySuspend();
1418
+ return 'hello';
1419
+ }
1420
+
1421
+ const Wrapper = React.forwardRef(render);
1422
+ const root = ReactTestRenderer.create(
1423
+ <Suspense fallback="loading">
1424
+ <Wrapper />
1425
+ </Suspense>,
1426
+ );
1427
+ expect(root).toMatchRenderedOutput('loading');
1428
+ await Promise.resolve();
1429
+ expect(root).toMatchRenderedOutput('hello');
1430
+ });
1431
+
1432
+ // Regression test for https://github.com/facebook/react/issues/14790
1433
+ it('does not fire a false positive warning when suspending memo(forwardRef)', async () => {
1434
+ const {Suspense, useState} = React;
1435
+
1436
+ let wasSuspended = false;
1437
+ function trySuspend() {
1438
+ if (!wasSuspended) {
1439
+ throw new Promise(resolve => {
1440
+ wasSuspended = true;
1441
+ resolve();
1442
+ });
1443
+ }
1444
+ }
1445
+
1446
+ function render(props, ref) {
1447
+ useState();
1448
+ trySuspend();
1449
+ return 'hello';
1450
+ }
1451
+
1452
+ const Wrapper = React.memo(React.forwardRef(render));
1453
+ const root = ReactTestRenderer.create(
1454
+ <Suspense fallback="loading">
1455
+ <Wrapper />
1456
+ </Suspense>,
1457
+ );
1458
+ expect(root).toMatchRenderedOutput('loading');
1459
+ await Promise.resolve();
1460
+ expect(root).toMatchRenderedOutput('hello');
1461
+ });
1462
});