From 6b0ffdf99846d60eedf9931eb38b1d6dac95133c Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Tue, 15 Oct 2019 14:15:18 -0700
Subject: [PATCH] Warn for update on different component in render

This warning already exists for class components, but not for functions.

It does not apply to render phase updates to the same component, which
have special semantics that we do support.
---
 .../src/ReactFiberWorkLoop.js                 | 67 +++++++++++--------
 ...eactHooksWithNoopRenderer-test.internal.js | 44 ++++++++++++
 2 files changed, 84 insertions(+), 27 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
index 0ff8ebe047699..488a911cdc020 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -372,7 +372,7 @@ export function scheduleUpdateOnFiber(
   expirationTime: ExpirationTime,
 ) {
   checkForNestedUpdates();
-  warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber);
+  warnAboutRenderPhaseUpdatesInDEV(fiber);
 
   const root = markUpdateTimeFromFiberToRoot(fiber, expirationTime);
   if (root === null) {
@@ -2663,32 +2663,45 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
 
 let didWarnAboutUpdateInRender = false;
 let didWarnAboutUpdateInGetChildContext = false;
-function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
-  if (__DEV__) {
-    if (fiber.tag === ClassComponent) {
-      switch (ReactCurrentDebugFiberPhaseInDEV) {
-        case 'getChildContext':
-          if (didWarnAboutUpdateInGetChildContext) {
-            return;
-          }
-          warningWithoutStack(
-            false,
-            'setState(...): Cannot call setState() inside getChildContext()',
-          );
-          didWarnAboutUpdateInGetChildContext = true;
-          break;
-        case 'render':
-          if (didWarnAboutUpdateInRender) {
-            return;
-          }
-          warningWithoutStack(
-            false,
-            'Cannot update during an existing state transition (such as ' +
-              'within `render`). Render methods should be a pure function of ' +
-              'props and state.',
-          );
-          didWarnAboutUpdateInRender = true;
-          break;
+function warnAboutRenderPhaseUpdatesInDEV(fiber) {
+  if (__DEV__ && (executionContext & RenderContext) !== NoContext) {
+    switch (fiber.tag) {
+      case FunctionComponent:
+      case ForwardRef:
+      case SimpleMemoComponent: {
+        warning(
+          false,
+          'Cannot update a component from inside the function body of a ' +
+            'different component.',
+        );
+        break;
+      }
+      case ClassComponent: {
+        switch (ReactCurrentDebugFiberPhaseInDEV) {
+          case 'getChildContext':
+            if (didWarnAboutUpdateInGetChildContext) {
+              return;
+            }
+            warningWithoutStack(
+              false,
+              'setState(...): Cannot call setState() inside getChildContext()',
+            );
+            didWarnAboutUpdateInGetChildContext = true;
+            break;
+          case 'render':
+            if (didWarnAboutUpdateInRender) {
+              return;
+            }
+            warningWithoutStack(
+              false,
+              'Cannot update during an existing state transition (such as ' +
+                'within `render`). Render methods should be a pure function of ' +
+                'props and state.',
+            );
+            didWarnAboutUpdateInRender = true;
+            break;
+        }
+        break;
       }
     }
   }
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
index 151688592507f..4ec0c077f12bc 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
@@ -491,6 +491,50 @@ describe('ReactHooksWithNoopRenderer', () => {
       ]);
       expect(ReactNoop.getChildren()).toEqual([span(22)]);
     });
+
+    it('warns about render phase update on a different component', async () => {
+      let setStep;
+      function Foo() {
+        const [step, _setStep] = useState(0);
+        setStep = _setStep;
+        return <Text text={`Foo [${step}]`} />;
+      }
+
+      function Bar({triggerUpdate}) {
+        if (triggerUpdate) {
+          setStep(1);
+        }
+        return <Text text="Bar" />;
+      }
+
+      const root = ReactNoop.createRoot();
+
+      await ReactNoop.act(async () => {
+        root.render(
+          <>
+            <Foo />
+            <Bar />
+          </>,
+        );
+      });
+      expect(Scheduler).toHaveYielded(['Foo [0]', 'Bar']);
+
+      // Bar will update Foo during its render phase. React should warn.
+      await ReactNoop.act(async () => {
+        root.render(
+          <>
+            <Foo />
+            <Bar triggerUpdate={true} />
+          </>,
+        );
+        expect(() =>
+          expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']),
+        ).toWarnDev([
+          'Cannot update a component from inside the function body of a ' +
+            'different component.',
+        ]);
+      });
+    });
   });
 
   describe('useReducer', () => {