Address more feedback

Address more feedback Address more feedback
facebook · Jan 23, 2020 · 12d95b6 · 12d95b6
1 parent 4cf7703
commit 12d95b6
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 61 deletions.
diff --git a/packages/react-reconciler/src/ReactStrictModeWarnings.js b/packages/react-reconciler/src/ReactStrictModeWarnings.js
@@ -286,6 +286,7 @@ if (__DEV__) {
   let pendingLegacyContextWarning: FiberToFiberComponentsMap = new Map();
 
   // Tracks components we have already warned about.
+  const legacyContextCounts = new Map();
   const didWarnAboutLegacyContext = new Set();
 
   ReactStrictModeWarnings.recordLegacyContextWarning = (
@@ -300,17 +301,23 @@ if (__DEV__) {
       );
       return;
     }
+    const type = fiber.type;
 
-    // Dedup strategy: Warn once per component.
-    if (didWarnAboutLegacyContext.has(fiber.type)) {
+    // Update legacy context counts
+    const warningCount = legacyContextCounts.get(type) || 0;
+    // Increase warning count by 1
+    legacyContextCounts.set(type, warningCount + 1);
+
+    // Dedup strategy: Warn once per component
+    if (didWarnAboutLegacyContext.has(type)) {
       return;
     }
 
     let warningsForRoot = pendingLegacyContextWarning.get(strictRoot);
 
     if (
-      fiber.type.contextTypes != null ||
-      fiber.type.childContextTypes != null ||
+      type.contextTypes != null ||
+      type.childContextTypes != null ||
       (instance !== null && typeof instance.getChildContext === 'function')
     ) {
       if (warningsForRoot === undefined) {
@@ -324,49 +331,41 @@ if (__DEV__) {
   ReactStrictModeWarnings.flushLegacyContextWarning = () => {
     ((pendingLegacyContextWarning: any): FiberToFiberComponentsMap).forEach(
       (fiberArray: FiberArray, strictRoot) => {
-        const componentStacks = new Map();
-
-        fiberArray.forEach(fiber => {
-          const componentName = getComponentName(fiber.type) || 'Component';
-          const componentStack = getStackByFiberInDevAndProd(fiber);
-          let count = 0;
-          if (componentStacks.has(componentStack)) {
-            ({count} = (componentStacks.get(componentStack): any));
-          }
-          // Increase count by 1
-          componentStacks.set(componentStack, {
-            count: count + 1,
-            name: componentName,
-            stack: componentStack,
+        const fibers = fiberArray.length;
+        let componentNames = '';
+
+        fiberArray
+          .sort((a, b) => {
+            const aCount = legacyContextCounts.get(a.type) || 0;
+            const bCount = legacyContextCounts.get(b.type) || 0;
+            return bCount - aCount;
+          })
+          .forEach((fiber, i) => {
+            const type = fiber.type;
+            if (didWarnAboutLegacyContext.has(type)) {
+              return;
+            }
+            didWarnAboutLegacyContext.add(type);
+            // Build up the string of comma separated component names
+            componentNames +=
+              (getComponentName(fiber.type) || 'Component') +
+              (i === fibers - 1 ? '' : ', ');
           });
-          didWarnAboutLegacyContext.add(fiber.type);
-        });
-
-        // Get the stacks from our componentStacks Map
-        const stacks = Array.from(componentStacks.values());
-
-        // Sort the stacks by their counts
-        stacks.sort((a, b) => b.count - a.count);
-
-        const mostFrequentStack = stacks[0];
-
-        if (mostFrequentStack) {
-          // We map to a Set to remove duplicate component names
-          const componentNames = Array.from(
-            new Set(stacks.map(stack => stack.name)),
-          ).join(', ');
-
-          console.error(
-            'Legacy context API has been detected within a strict-mode tree.' +
-              '\n\nThe old API will be supported in all 16.x releases, but applications ' +
-              'using it should migrate to the new version.' +
-              '\n\nPlease update the following components: %s' +
-              '\n\nLearn more about this warning here: https://fb.me/react-legacy-context' +
-              '%s',
-            componentNames,
-            mostFrequentStack.stack,
-          );
-        }
+        const mostFrequentFiber = fiberArray[0];
+        const stack = mostFrequentFiber
+          ? getStackByFiberInDevAndProd(mostFrequentFiber)
+          : '';
+
+        console.error(
+          'Legacy context API has been detected within a strict-mode tree.' +
+            '\n\nThe old API will be supported in all 16.x releases, but applications ' +
+            'using it should migrate to the new version.' +
+            '\n\nPlease update the following components: %s' +
+            '\n\nLearn more about this warning here: https://fb.me/react-legacy-context' +
+            '%s',
+          componentNames,
+          stack,
+        );
       },
     );
   };

diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js
@@ -2036,20 +2036,17 @@ describe('ReactIncremental', () => {
     };
 
     ReactNoop.render(<Recurse />);
-    expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
-      [
-        'Warning: The <Recurse /> component appears to be a function component that returns a class instance. ' +
-          'Change Recurse to a class that extends React.Component instead. ' +
-          "If you can't use a class try assigning the prototype on the function as a workaround. " +
-          '`Recurse.prototype = React.Component.prototype`. ' +
-          "Don't use an arrow function since it cannot be called with `new` by React.",
-        'Legacy context API has been detected within a strict-mode tree.\n\n' +
-          'The old API will be supported in all 16.x releases, but applications ' +
-          'using it should migrate to the new version.\n\n' +
-          'Please update the following components: Recurse',
-      ],
-      {withoutStack: 0},
-    );
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
+      'Warning: The <Recurse /> component appears to be a function component that returns a class instance. ' +
+        'Change Recurse to a class that extends React.Component instead. ' +
+        "If you can't use a class try assigning the prototype on the function as a workaround. " +
+        '`Recurse.prototype = React.Component.prototype`. ' +
+        "Don't use an arrow function since it cannot be called with `new` by React.",
+      'Legacy context API has been detected within a strict-mode tree.\n\n' +
+        'The old API will be supported in all 16.x releases, but applications ' +
+        'using it should migrate to the new version.\n\n' +
+        'Please update the following components: Recurse',
+    ]);
     expect(ops).toEqual([
       'Recurse {}',
       'Recurse {"n":2}',
@@ -2114,7 +2111,7 @@ describe('ReactIncremental', () => {
       'Legacy context API has been detected within a strict-mode tree.\n\n' +
         'The old API will be supported in all 16.x releases, but applications ' +
         'using it should migrate to the new version.\n\n' +
-        'Please update the following components: Intl, ShowLocale',
+        'Please update the following components: ShowLocale, Intl',
     );
   });