Improved flushing of pending warnings to avoid leaks

Author: bvaughn

packages/react-reconciler/src/ReactDebugAsyncWarnings.js

@@ -24,7 +24,8 @@ type FiberToLifecycleMap = Map<Fiber, LifecycleToComponentsMap>;
 const DID_WARN_KEY = '__didWarnAboutUnsafeAsyncLifecycles';
 
 const ReactDebugAsyncWarnings = {
-  flushPendingAsyncWarnings(maybeAsyncRoot: Fiber): void {},
+  discardPendingWarnings(): void {},
+  flushPendingAsyncWarnings(): void {},
   recordLifecycleWarnings(fiber: Fiber, instance: any): void {},
 };
 
@@ -37,15 +38,13 @@ if (__DEV__) {
 
   let pendingWarningsMap: FiberToLifecycleMap = new Map();
 
-  ReactDebugAsyncWarnings.flushPendingAsyncWarnings = (
-    maybeAsyncRoot: Fiber,
-  ) => {
+  ReactDebugAsyncWarnings.discardPendingWarnings = () => {
+    pendingWarningsMap = new Map();
+  };
+
+  ReactDebugAsyncWarnings.flushPendingAsyncWarnings = () => {
     ((pendingWarningsMap: any): FiberToLifecycleMap).forEach(
       (lifecycleWarningsMap, asyncRoot) => {
-        if (asyncRoot !== maybeAsyncRoot) {
-          return;
-        }
-
         const lifecyclesWarningMesages = [];
 
         Object.keys(lifecycleWarningsMap).forEach(lifecycle => {
@@ -85,10 +84,10 @@ if (__DEV__) {
             lifecyclesWarningMesages.join('\n\n'),
           );
         }
-
-        pendingWarningsMap.delete(asyncRoot);
       },
     );
+
+    pendingWarningsMap = new Map();
   };
 
   const getAsyncRoot = (fiber: Fiber): Fiber => {

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -311,6 +311,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
   }
 
   function commitAllLifeCycles() {
+    if (__DEV__) {
+      ReactDebugAsyncWarnings.flushPendingAsyncWarnings();
+    }
+
     while (nextEffect !== null) {
       const effectTag = nextEffect.effectTag;
 
@@ -544,7 +548,6 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
       );
       if (__DEV__) {
         ReactDebugCurrentFiber.resetCurrentFiber();
-        ReactDebugAsyncWarnings.flushPendingAsyncWarnings(workInProgress);
       }
 
       const returnFiber = workInProgress.return;
@@ -653,6 +656,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
   }
 
   function performFailedUnitOfWork(workInProgress: Fiber): Fiber | null {
+    if (__DEV__) {
+      ReactDebugAsyncWarnings.discardPendingWarnings();
+    }
+
     // The current, flushed, state of this fiber is the alternate.
     // Ideally nothing should rely on this, but relying on it here
     // means that we don't need an additional field on the work in

packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.internal.js

@@ -48,15 +48,13 @@ describe('ReactIncrementalErrorLogging', () => {
     );
 
     expect(() => {
-      expect(ReactNoop.flushDeferredPri).toWarnDev([
-        'componentWillMount: Please update the following components ' +
-          'to use componentDidMount instead: ErrorThrowingComponent',
+      expect(ReactNoop.flushDeferredPri).toWarnDev(
         'The above error occurred in the <ErrorThrowingComponent> component:\n' +
           '    in ErrorThrowingComponent (at **)\n' +
           '    in span (at **)\n' +
           '    in div (at **)\n\n' +
           'Consider adding an error boundary to your tree to customize error handling behavior.',
-      ]);
+      );
     }).toThrowError('componentWillMount error');
   });
 

packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js

@@ -373,7 +373,7 @@ describe('ReactAsyncClassComponent', () => {
       rendered.update(<AsyncRoot foo={false} />);
     });
 
-    it('should warn about lifecycles even in the event of an error', () => {
+    it('should not warn about uncommitted lifecycles in the event of an error', () => {
       let caughtError;
 
       class AsyncRoot extends React.unstable_AsyncComponent {
@@ -408,20 +408,14 @@ describe('ReactAsyncClassComponent', () => {
 
       expect(() => {
         ReactTestRenderer.create(<AsyncRoot foo={true} />);
-      }).toWarnDev([
-        'Unsafe lifecycle methods were found within the following async tree:' +
-          '\n    in AsyncRoot (at **)' +
-          '\n\ncomponentWillMount: Please update the following components ' +
-          'to use componentDidMount instead: Foo' +
-          '\n\nLearn more about this warning here:' +
-          '\nhttps://fb.me/react-async-component-lifecycle-hooks',
+      }).toWarnDev(
         'Unsafe lifecycle methods were found within the following async tree:' +
           '\n    in AsyncRoot (at **)' +
           '\n\ncomponentWillMount: Please update the following components ' +
           'to use componentDidMount instead: Bar' +
           '\n\nLearn more about this warning here:' +
           '\nhttps://fb.me/react-async-component-lifecycle-hooks',
-      ]);
+      );
 
       expect(caughtError).not.toBe(null);
     });