From 8205a0028d8fa43f964a74dc3620bf1f61dc5ff4 Mon Sep 17 00:00:00 2001
From: Jovi De Croock <decroockjovi@gmail.com>
Date: Thu, 19 Mar 2020 10:49:18 -0700
Subject: [PATCH] improve error message for cross-functional component updates
 (#18316)

* improve error message for cross-functional component updates

* correctly use %s by quoting it

* use workInProgress and lint

* add test assertion

* fix test

* Improve the error message

Co-authored-by: Dan Abramov <dan.abramov@me.com>
---
 .../src/ReactFiberWorkLoop.js                 | 27 ++++++++++++++++---
 .../src/__tests__/ReactHooks-test.internal.js |  6 ++---
 ...eactHooksWithNoopRenderer-test.internal.js | 17 +++++++++---
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
index 3c2427594a798..f65f1ba15a418 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -2794,6 +2794,11 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
 
 let didWarnAboutUpdateInRender = false;
 let didWarnAboutUpdateInGetChildContext = false;
+let didWarnAboutUpdateInRenderForAnotherComponent;
+if (__DEV__) {
+  didWarnAboutUpdateInRenderForAnotherComponent = new Set();
+}
+
 function warnAboutRenderPhaseUpdatesInDEV(fiber) {
   if (__DEV__) {
     if ((executionContext & RenderContext) !== NoContext) {
@@ -2801,10 +2806,24 @@ function warnAboutRenderPhaseUpdatesInDEV(fiber) {
         case FunctionComponent:
         case ForwardRef:
         case SimpleMemoComponent: {
-          console.error(
-            'Cannot update a component from inside the function body of a ' +
-              'different component.',
-          );
+          const renderingComponentName =
+            (workInProgress && getComponentName(workInProgress.type)) ||
+            'Unknown';
+          const setStateComponentName =
+            getComponentName(fiber.type) || 'Unknown';
+          const dedupeKey =
+            renderingComponentName + ' ' + setStateComponentName;
+          if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) {
+            didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey);
+            console.error(
+              'Cannot update a component (`%s`) from inside the function body of a ' +
+                'different component (`%s`). To locate the bad setState() call inside `%s`, ' +
+                'follow the stack trace as described in https://fb.me/setstate-in-render',
+              setStateComponentName,
+              renderingComponentName,
+              renderingComponentName,
+            );
+          }
           break;
         }
         case ClassComponent: {
diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
index f9073f1bf7018..6de3567163a3b 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
@@ -1087,7 +1087,7 @@ describe('ReactHooks', () => {
       ),
     ).toErrorDev([
       'Context can only be read while React is rendering',
-      'Cannot update a component from inside the function body of a different component.',
+      'Cannot update a component (`Fn`) from inside the function body of a different component (`Cls`).',
     ]);
   });
 
@@ -1783,8 +1783,8 @@ describe('ReactHooks', () => {
     if (__DEV__) {
       expect(console.error).toHaveBeenCalledTimes(2);
       expect(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Cannot update a component from inside the function body ' +
-          'of a different component.%s',
+        'Warning: Cannot update a component (`%s`) from inside the function body ' +
+          'of a different component (`%s`).',
       );
     }
   });
diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
index 81b774bd0a553..3739e655069c0 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
@@ -430,7 +430,7 @@ function loadModules({
 
           function Bar({triggerUpdate}) {
             if (triggerUpdate) {
-              setStep(1);
+              setStep(x => x + 1);
             }
             return <Text text="Bar" />;
           }
@@ -458,10 +458,21 @@ function loadModules({
             expect(() =>
               expect(Scheduler).toFlushAndYield(['Foo [0]', 'Bar', 'Foo [1]']),
             ).toErrorDev([
-              'Cannot update a component from inside the function body of a ' +
-                'different component.',
+              'Cannot update a component (`Foo`) from inside the function body of a ' +
+                'different component (`Bar`). To locate the bad setState() call inside `Bar`',
             ]);
           });
+
+          // It should not warn again (deduplication).
+          await ReactNoop.act(async () => {
+            root.render(
+              <>
+                <Foo />
+                <Bar triggerUpdate={true} />
+              </>,
+            );
+            expect(Scheduler).toFlushAndYield(['Foo [1]', 'Bar', 'Foo [2]']);
+          });
         });
 
         it('keeps restarting until there are no more new updates', () => {