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
facebook · Feb 13, 2019 · c6bee76 · c6bee76
1 parent 3ae94e1
commit c6bee76
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 11 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -220,6 +220,10 @@ function updateForwardRef(
   nextProps: any,
   renderExpirationTime: ExpirationTime,
 ) {
+  // TODO: current can be non-null here even if the component
+  // hasn't yet mounted. This happens after the first render suspends.
+  // We'll need to figure out if this is fine or can cause issues.
+
   if (__DEV__) {
     if (workInProgress.type !== workInProgress.elementType) {
       // Lazy component props can't be validated in createElement
@@ -415,6 +419,10 @@ function updateSimpleMemoComponent(
   updateExpirationTime,
   renderExpirationTime: ExpirationTime,
 ): null | Fiber {
+  // TODO: current can be non-null here even if the component
+  // hasn't yet mounted. This happens when the inner render suspends.
+  // We'll need to figure out if this is fine or can cause issues.
+
   if (__DEV__) {
     if (workInProgress.type !== workInProgress.elementType) {
       // Lazy component props can't be validated in createElement

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -449,17 +449,6 @@ function mountWorkInProgressHook(): Hook {
 
   if (__DEV__) {
     (hook: any)._debugType = (currentHookNameInDev: any);
-    if (
-      currentlyRenderingFiber !== null &&
-      currentlyRenderingFiber.alternate !== null
-    ) {
-      warning(
-        false,
-        '%s: Rendered more hooks than during the previous render. This is ' +
-          'not currently supported and may lead to unexpected behavior.',
-        getComponentName(currentlyRenderingFiber.type),
-      );
-    }
   }
   if (workInProgressHook === null) {
     // This is the first hook in the list

diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -1366,4 +1366,97 @@ describe('ReactHooks', () => {
       ),
     ).toThrow('Hello');
   });
+
+  // Regression test for https://github.com/facebook/react/issues/14790
+  it('does not fire a false positive warning when suspending memo', async () => {
+    const {Suspense, useState} = React;
+
+    let wasSuspended = false;
+    function trySuspend() {
+      if (!wasSuspended) {
+        throw new Promise(resolve => {
+          wasSuspended = true;
+          resolve();
+        });
+      }
+    }
+
+    function Child() {
+      useState();
+      trySuspend();
+      return 'hello';
+    }
+
+    const Wrapper = React.memo(Child);
+    const root = ReactTestRenderer.create(
+      <Suspense fallback="loading">
+        <Wrapper />
+      </Suspense>,
+    );
+    expect(root).toMatchRenderedOutput('loading');
+    await Promise.resolve();
+    expect(root).toMatchRenderedOutput('hello');
+  });
+
+  // Regression test for https://github.com/facebook/react/issues/14790
+  it('does not fire a false positive warning when suspending forwardRef', async () => {
+    const {Suspense, useState} = React;
+
+    let wasSuspended = false;
+    function trySuspend() {
+      if (!wasSuspended) {
+        throw new Promise(resolve => {
+          wasSuspended = true;
+          resolve();
+        });
+      }
+    }
+
+    function render(props, ref) {
+      useState();
+      trySuspend();
+      return 'hello';
+    }
+
+    const Wrapper = React.forwardRef(render);
+    const root = ReactTestRenderer.create(
+      <Suspense fallback="loading">
+        <Wrapper />
+      </Suspense>,
+    );
+    expect(root).toMatchRenderedOutput('loading');
+    await Promise.resolve();
+    expect(root).toMatchRenderedOutput('hello');
+  });
+
+  // Regression test for https://github.com/facebook/react/issues/14790
+  it('does not fire a false positive warning when suspending memo(forwardRef)', async () => {
+    const {Suspense, useState} = React;
+
+    let wasSuspended = false;
+    function trySuspend() {
+      if (!wasSuspended) {
+        throw new Promise(resolve => {
+          wasSuspended = true;
+          resolve();
+        });
+      }
+    }
+
+    function render(props, ref) {
+      useState();
+      trySuspend();
+      return 'hello';
+    }
+
+    const Wrapper = React.memo(React.forwardRef(render));
+    const root = ReactTestRenderer.create(
+      <Suspense fallback="loading">
+        <Wrapper />
+      </Suspense>,
+    );
+    expect(root).toMatchRenderedOutput('loading');
+    await Promise.resolve();
+    expect(root).toMatchRenderedOutput('hello');
+  });
 });