Don't fire the render phase update warning for class lifecycles (#18330 )

* Change the warning to not say "function body" This warning is more generic and may happen with class components too. * Dedupe by the rendering component * Don't warn outside of render
improve error message for cross-functional component updates (#18316 )
2020-03-19 11:36:20 -07:00 · 2020-03-19 11:34:11 -07:00 · 2020-03-19 11:34:07 -07:00 · 2020-03-19 10:38:39 -07:00
13 changed files with 260 additions and 205 deletions
--- a/packages/react-dom/src/tests/ReactCompositeComponent-test.js
+++ b/packages/react-dom/src/tests/ReactCompositeComponent-test.js
@@ -493,43 +493,6 @@ describe('ReactCompositeComponent', () => {
    ReactDOM.render(<Component prop={123} />, container);
  });

-  it('should warn about `setState` in getChildContext', () => {
-    const container = document.createElement('div');
-
-    let renderPasses = 0;
-
-    class Component extends React.Component {
-      state = {value: 0};
-
-      getChildContext() {
-        if (this.state.value === 0) {
-          this.setState({value: 1});
-        }
-      }
-
-      render() {
-        renderPasses++;
-        return <div />;
-      }
-    }
-    Component.childContextTypes = {};
-
-    let instance;
-
-    expect(() => {
-      instance = ReactDOM.render(<Component />, container);
-    }).toErrorDev(
-      'Warning: setState(...): Cannot call setState() inside getChildContext()',
-    );
-
-    expect(renderPasses).toBe(2);
-    expect(instance.state.value).toBe(1);
-
-    // Test deduplication; (no additional warnings are expected).
-    ReactDOM.unmountComponentAtNode(container);
-    ReactDOM.render(<Component />, container);
-  });
-
  it('should cleanup even if render() fatals', () => {
    class BadComponent extends React.Component {
      render() {
@@ -1786,4 +1749,114 @@ describe('ReactCompositeComponent', () => {
    ReactDOM.render(<Shadow />, container);
    expect(container.firstChild.tagName).toBe('DIV');
  });
+
+  it('should not warn on updating function component from componentWillMount', () => {
+    let _setState;
+    function A() {
+      _setState = React.useState()[1];
+      return null;
+    }
+    class B extends React.Component {
+      UNSAFE_componentWillMount() {
+        _setState({});
+      }
+      render() {
+        return null;
+      }
+    }
+    function Parent() {
+      return (
+        <div>
+          <A />
+          <B />
+        </div>
+      );
+    }
+    const container = document.createElement('div');
+    ReactDOM.render(<Parent />, container);
+  });
+
+  it('should not warn on updating function component from componentWillUpdate', () => {
+    let _setState;
+    function A() {
+      _setState = React.useState()[1];
+      return null;
+    }
+    class B extends React.Component {
+      UNSAFE_componentWillUpdate() {
+        _setState({});
+      }
+      render() {
+        return null;
+      }
+    }
+    function Parent() {
+      return (
+        <div>
+          <A />
+          <B />
+        </div>
+      );
+    }
+    const container = document.createElement('div');
+    ReactDOM.render(<Parent />, container);
+    ReactDOM.render(<Parent />, container);
+  });
+
+  it('should not warn on updating function component from componentWillReceiveProps', () => {
+    let _setState;
+    function A() {
+      _setState = React.useState()[1];
+      return null;
+    }
+    class B extends React.Component {
+      UNSAFE_componentWillReceiveProps() {
+        _setState({});
+      }
+      render() {
+        return null;
+      }
+    }
+    function Parent() {
+      return (
+        <div>
+          <A />
+          <B />
+        </div>
+      );
+    }
+    const container = document.createElement('div');
+    ReactDOM.render(<Parent />, container);
+    ReactDOM.render(<Parent />, container);
+  });
+
+  it('should warn on updating function component from render', () => {
+    let _setState;
+    function A() {
+      _setState = React.useState()[1];
+      return null;
+    }
+    class B extends React.Component {
+      render() {
+        _setState({});
+        return null;
+      }
+    }
+    function Parent() {
+      return (
+        <div>
+          <A />
+          <B />
+        </div>
+      );
+    }
+    const container = document.createElement('div');
+    expect(() => {
+      ReactDOM.render(<Parent />, container);
+    }).toErrorDev(
+      'Cannot update a component (`A`) while rendering a different component (`B`)',
+    );
+    // Dedupe.
+    ReactDOM.render(<Parent />, container);
+  });
 });
--- a/packages/react-dom/src/tests/ReactDOMComponent-test.js
+++ b/packages/react-dom/src/tests/ReactDOMComponent-test.js
@@ -1242,53 +1242,6 @@ describe('ReactDOMComponent', () => {
      );
    });

-    it('should emit a warning once for a named custom component using shady DOM', () => {
-      const defaultCreateElement = document.createElement.bind(document);
-
-      try {
-        document.createElement = element => {
-          const container = defaultCreateElement(element);
-          container.shadyRoot = {};
-          return container;
-        };
-        class ShadyComponent extends React.Component {
-          render() {
-            return <polymer-component />;
-          }
-        }
-        const node = document.createElement('div');
-        expect(() => ReactDOM.render(<ShadyComponent />, node)).toErrorDev(
-          'ShadyComponent is using shady DOM. Using shady DOM with React can ' +
-            'cause things to break subtly.',
-        );
-        mountComponent({is: 'custom-shady-div2'});
-      } finally {
-        document.createElement = defaultCreateElement;
-      }
-    });
-
-    it('should emit a warning once for an unnamed custom component using shady DOM', () => {
-      const defaultCreateElement = document.createElement.bind(document);
-
-      try {
-        document.createElement = element => {
-          const container = defaultCreateElement(element);
-          container.shadyRoot = {};
-          return container;
-        };
-
-        expect(() => mountComponent({is: 'custom-shady-div'})).toErrorDev(
-          'A component is using shady DOM. Using shady DOM with React can ' +
-            'cause things to break subtly.',
-        );
-
-        // No additional warnings are expected
-        mountComponent({is: 'custom-shady-div2'});
-      } finally {
-        document.createElement = defaultCreateElement;
-      }
-    });
-
    it('should treat menuitem as a void element but still create the closing tag', () => {
      // menuitem is not implemented in jsdom, so this triggers the unknown warning error
      const container = document.createElement('div');
--- a/packages/react-dom/src/client/ReactDOMComponent.js
+++ b/packages/react-dom/src/client/ReactDOMComponent.js
@@ -7,8 +7,6 @@
 * @flow
 */

-// TODO: direct imports like some-package/src/* are bad. Fix me.
-import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCurrentFiber';
 import {registrationNameModules} from 'legacy-events/EventPluginRegistry';
 import {canUseDOM} from 'shared/ExecutionEnvironment';
 import endsWith from 'shared/endsWith';
@@ -90,7 +88,6 @@ import {
 import {legacyListenToEvent} from '../events/DOMLegacyEventPluginSystem';

 let didWarnInvalidHydration = false;
-let didWarnShadyDOM = false;
 let didWarnScriptTags = false;

 const DANGEROUSLY_SET_INNER_HTML = 'dangerouslySetInnerHTML';
@@ -509,18 +506,6 @@ export function setInitialProperties(
  const isCustomComponentTag = isCustomComponent(tag, rawProps);
  if (__DEV__) {
    validatePropertiesInDevelopment(tag, rawProps);
-    if (
-      isCustomComponentTag &&
-      !didWarnShadyDOM &&
-      (domElement: any).shadyRoot
-    ) {
-      console.error(
-        '%s is using shady DOM. Using shady DOM with React can ' +
-          'cause things to break subtly.',
-        getCurrentFiberOwnerNameInDevOrNull() || 'A component',
-      );
-      didWarnShadyDOM = true;
-    }
  }

  // TODO: Make sure that we check isMounted before firing any of these events.
@@ -906,18 +891,6 @@ export function diffHydratedProperties(
    suppressHydrationWarning = rawProps[SUPPRESS_HYDRATION_WARNING] === true;
    isCustomComponentTag = isCustomComponent(tag, rawProps);
    validatePropertiesInDevelopment(tag, rawProps);
-    if (
-      isCustomComponentTag &&
-      !didWarnShadyDOM &&
-      (domElement: any).shadyRoot
-    ) {
-      console.error(
-        '%s is using shady DOM. Using shady DOM with React can ' +
-          'cause things to break subtly.',
-        getCurrentFiberOwnerNameInDevOrNull() || 'A component',
-      );
-      didWarnShadyDOM = true;
-    }
  }

  // TODO: Make sure that we check isMounted before firing any of these events.
--- a/packages/react-noop-renderer/src/createReactNoop.js
+++ b/packages/react-noop-renderer/src/createReactNoop.js
@@ -785,6 +785,32 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
      };
    },

+    createLegacyRoot() {
+      const container = {
+        rootID: '' + idCounter++,
+        pendingChildren: [],
+        children: [],
+      };
+      const fiberRoot = NoopRenderer.createContainer(
+        container,
+        LegacyRoot,
+        false,
+        null,
+      );
+      return {
+        _Scheduler: Scheduler,
+        render(children: ReactNodeList) {
+          NoopRenderer.updateContainer(children, fiberRoot, null, null);
+        },
+        getChildren() {
+          return getChildren(container);
+        },
+        getChildrenAsJSX() {
+          return getChildrenAsJSX(container);
+        },
+      };
+    },
+
    getChildrenAsJSX(rootID: string = DEFAULT_ROOT_ID) {
      const container = rootContainers.get(rootID);
      return getChildrenAsJSX(container);
--- a/packages/react-reconciler/src/ReactCurrentFiber.js
+++ b/packages/react-reconciler/src/ReactCurrentFiber.js
@@ -23,8 +23,6 @@ import getComponentName from 'shared/getComponentName';

 const ReactDebugCurrentFrame = ReactSharedInternals.ReactDebugCurrentFrame;

-type LifeCyclePhase = 'render' | 'getChildContext';
-
 function describeFiber(fiber: Fiber): string {
  switch (fiber.tag) {
    case HostRoot:
@@ -57,7 +55,7 @@ export function getStackByFiberInDevAndProd(workInProgress: Fiber): string {
 }

 export let current: Fiber | null = null;
-export let phase: LifeCyclePhase | null = null;
+export let isRendering: boolean = false;

 export function getCurrentFiberOwnerNameInDevOrNull(): string | null {
  if (__DEV__) {
@@ -88,7 +86,7 @@ export function resetCurrentFiber() {
  if (__DEV__) {
    ReactDebugCurrentFrame.getCurrentStack = null;
    current = null;
-    phase = null;
+    isRendering = false;
  }
 }

@@ -96,12 +94,12 @@ export function setCurrentFiber(fiber: Fiber) {
  if (__DEV__) {
    ReactDebugCurrentFrame.getCurrentStack = getCurrentFiberStackInDev;
    current = fiber;
-    phase = null;
+    isRendering = false;
  }
 }

-export function setCurrentPhase(lifeCyclePhase: LifeCyclePhase | null) {
+export function setIsRendering(rendering: boolean) {
  if (__DEV__) {
-    phase = lifeCyclePhase;
+    isRendering = rendering;
  }
 }
--- a/packages/react-reconciler/src/ReactFiberBeginWork.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -74,9 +74,9 @@ import ReactStrictModeWarnings from './ReactStrictModeWarnings';
 import {refineResolvedLazyComponent} from 'shared/ReactLazyComponent';
 import {REACT_LAZY_TYPE, getIteratorFn} from 'shared/ReactSymbols';
 import {
-  setCurrentPhase,
  getCurrentFiberOwnerNameInDevOrNull,
  getCurrentFiberStackInDev,
+  setIsRendering,
 } from './ReactCurrentFiber';
 import {startWorkTimer, cancelWorkTimer} from './ReactDebugFiberPerf';
 import {
@@ -193,7 +193,6 @@ let didWarnAboutContextTypeOnFunctionComponent;
 let didWarnAboutGetDerivedStateOnFunctionComponent;
 let didWarnAboutFunctionRefs;
 export let didWarnAboutReassigningProps;
-let didWarnAboutMaxDuration;
 let didWarnAboutRevealOrder;
 let didWarnAboutTailOptions;
 let didWarnAboutDefaultPropsOnFunctionComponent;
@@ -205,7 +204,6 @@ if (__DEV__) {
  didWarnAboutGetDerivedStateOnFunctionComponent = {};
  didWarnAboutFunctionRefs = {};
  didWarnAboutReassigningProps = false;
-  didWarnAboutMaxDuration = false;
  didWarnAboutRevealOrder = {};
  didWarnAboutTailOptions = {};
  didWarnAboutDefaultPropsOnFunctionComponent = {};
@@ -312,7 +310,7 @@ function updateForwardRef(
  prepareToReadContext(workInProgress, renderExpirationTime);
  if (__DEV__) {
    ReactCurrentOwner.current = workInProgress;
-    setCurrentPhase('render');
+    setIsRendering(true);
    nextChildren = renderWithHooks(
      current,
      workInProgress,
@@ -337,7 +335,7 @@ function updateForwardRef(
        );
      }
    }
-    setCurrentPhase(null);
+    setIsRendering(false);
  } else {
    nextChildren = renderWithHooks(
      current,
@@ -644,7 +642,7 @@ function updateFunctionComponent(
  prepareToReadContext(workInProgress, renderExpirationTime);
  if (__DEV__) {
    ReactCurrentOwner.current = workInProgress;
-    setCurrentPhase('render');
+    setIsRendering(true);
    nextChildren = renderWithHooks(
      current,
      workInProgress,
@@ -669,7 +667,7 @@ function updateFunctionComponent(
        );
      }
    }
-    setCurrentPhase(null);
+    setIsRendering(false);
  } else {
    nextChildren = renderWithHooks(
      current,
@@ -720,7 +718,7 @@ function updateBlock(
  prepareToReadContext(workInProgress, renderExpirationTime);
  if (__DEV__) {
    ReactCurrentOwner.current = workInProgress;
-    setCurrentPhase('render');
+    setIsRendering(true);
    nextChildren = renderWithHooks(
      current,
      workInProgress,
@@ -745,7 +743,7 @@ function updateBlock(
        );
      }
    }
-    setCurrentPhase(null);
+    setIsRendering(false);
  } else {
    nextChildren = renderWithHooks(
      current,
@@ -923,7 +921,7 @@ function finishClassComponent(
    }
  } else {
    if (__DEV__) {
-      setCurrentPhase('render');
+      setIsRendering(true);
      nextChildren = instance.render();
      if (
        debugRenderPhaseSideEffectsForStrictMode &&
@@ -931,7 +929,7 @@ function finishClassComponent(
      ) {
        instance.render();
      }
-      setCurrentPhase(null);
+      setIsRendering(false);
    } else {
      nextChildren = instance.render();
    }
@@ -1358,6 +1356,7 @@ function mountIndeterminateComponent(
      ReactStrictModeWarnings.recordLegacyContextWarning(workInProgress, null);
    }

+    setIsRendering(true);
    ReactCurrentOwner.current = workInProgress;
    value = renderWithHooks(
      null,
@@ -1367,6 +1366,7 @@ function mountIndeterminateComponent(
      context,
      renderExpirationTime,
    );
+    setIsRendering(false);
  } else {
    value = renderWithHooks(
      null,
@@ -1637,18 +1637,6 @@ function updateSuspenseComponent(

  pushSuspenseContext(workInProgress, suspenseContext);

-  if (__DEV__) {
-    if ('maxDuration' in nextProps) {
-      if (!didWarnAboutMaxDuration) {
-        didWarnAboutMaxDuration = true;
-        console.error(
-          'maxDuration has been removed from React. ' +
-            'Remove the maxDuration prop.',
-        );
-      }
-    }
-  }
-
  // This next part is a bit confusing. If the children timeout, we switch to
  // showing the fallback children in place of the "primary" children.
  // However, we don't want to delete the primary children because then their
@@ -2732,9 +2720,9 @@ function updateContextConsumer(
  let newChildren;
  if (__DEV__) {
    ReactCurrentOwner.current = workInProgress;
-    setCurrentPhase('render');
+    setIsRendering(true);
    newChildren = render(newValue);
-    setCurrentPhase(null);
+    setIsRendering(false);
  } else {
    newChildren = render(newValue);
  }
--- a/packages/react-reconciler/src/ReactFiberContext.js
+++ b/packages/react-reconciler/src/ReactFiberContext.js
@@ -17,7 +17,7 @@ import getComponentName from 'shared/getComponentName';
 import invariant from 'shared/invariant';
 import checkPropTypes from 'prop-types/checkPropTypes';

-import {setCurrentPhase, getCurrentFiberStackInDev} from './ReactCurrentFiber';
+import {getCurrentFiberStackInDev} from './ReactCurrentFiber';
 import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
 import {createCursor, push, pop} from './ReactFiberStack';

@@ -210,15 +210,9 @@ function processChildContext(
    }

    let childContext;
-    if (__DEV__) {
-      setCurrentPhase('getChildContext');
-    }
    startPhaseTimer(fiber, 'getChildContext');
    childContext = instance.getChildContext();
    stopPhaseTimer();
-    if (__DEV__) {
-      setCurrentPhase(null);
-    }
    for (let contextKey in childContext) {
      invariant(
        contextKey in childContextTypes,
--- a/packages/react-reconciler/src/ReactFiberReconciler.js
+++ b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -70,7 +70,7 @@ import {
 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
 import {
  getStackByFiberInDevAndProd,
-  phase as ReactCurrentFiberPhase,
+  isRendering as ReactCurrentFiberIsRendering,
  current as ReactCurrentFiberCurrent,
 } from './ReactCurrentFiber';
 import {StrictMode} from './ReactTypeOfMode';
@@ -259,7 +259,7 @@ export function updateContainer(

  if (__DEV__) {
    if (
-      ReactCurrentFiberPhase === 'render' &&
+      ReactCurrentFiberIsRendering &&
      ReactCurrentFiberCurrent !== null &&
      !didWarnAboutNestedUpdates
    ) {
--- a/packages/react-reconciler/src/ReactFiberThrow.js
+++ b/packages/react-reconciler/src/ReactFiberThrow.js
@@ -199,9 +199,11 @@ function throwException(
      // to render it.
      let currentSource = sourceFiber.alternate;
      if (currentSource) {
+        sourceFiber.updateQueue = currentSource.updateQueue;
        sourceFiber.memoizedState = currentSource.memoizedState;
        sourceFiber.expirationTime = currentSource.expirationTime;
      } else {
+        sourceFiber.updateQueue = null;
        sourceFiber.memoizedState = null;
      }
    }
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -156,7 +156,7 @@ import {
 import getComponentName from 'shared/getComponentName';
 import ReactStrictModeWarnings from './ReactStrictModeWarnings';
 import {
-  phase as ReactCurrentDebugFiberPhaseInDEV,
+  isRendering as ReactCurrentDebugFiberIsRenderingInDEV,
  resetCurrentFiber as resetCurrentDebugFiberInDEV,
  setCurrentFiber as setCurrentDebugFiberInDEV,
  getStackByFiberInDevAndProd,
@@ -2793,42 +2793,49 @@ 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) {
+    if (
+      ReactCurrentDebugFiberIsRenderingInDEV &&
+      (executionContext & RenderContext) !== NoContext
+    ) {
      switch (fiber.tag) {
        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';
+          // Dedupe by the rendering component because it's the one that needs to be fixed.
+          const dedupeKey = renderingComponentName;
+          if (!didWarnAboutUpdateInRenderForAnotherComponent.has(dedupeKey)) {
+            didWarnAboutUpdateInRenderForAnotherComponent.add(dedupeKey);
+            const setStateComponentName =
+              getComponentName(fiber.type) || 'Unknown';
+            console.error(
+              'Cannot update a component (`%s`) while rendering 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: {
-          switch (ReactCurrentDebugFiberPhaseInDEV) {
-            case 'getChildContext':
-              if (didWarnAboutUpdateInGetChildContext) {
-                return;
-              }
-              console.error(
-                'setState(...): Cannot call setState() inside getChildContext()',
-              );
-              didWarnAboutUpdateInGetChildContext = true;
-              break;
-            case 'render':
-              if (didWarnAboutUpdateInRender) {
-                return;
-              }
-              console.error(
-                '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;
+          if (!didWarnAboutUpdateInRender) {
+            console.error(
+              '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;
        }
--- 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`) while rendering 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`) while rendering ' +
+          'a different component (`%s`).',
      );
    }
  });
--- 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`) while rendering 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', () => {
--- a/packages/react-reconciler/src/tests/ReactSuspenseWithNoopRenderer-test.internal.js
+++ b/packages/react-reconciler/src/tests/ReactSuspenseWithNoopRenderer-test.internal.js
@@ -110,25 +110,6 @@ function loadModules({
        }
      }

-      it('warns if the deprecated maxDuration option is used', () => {
-        function Foo() {
-          return (
-            <Suspense maxDuration={100} fallback="Loading...">
-              <div />;
-            </Suspense>
-          );
-        }
-
-        ReactNoop.render(<Foo />);
-
-        expect(() => Scheduler.unstable_flushAll()).toErrorDev([
-          'Warning: maxDuration has been removed from React. ' +
-            'Remove the maxDuration prop.' +
-            '\n    in Suspense (at **)' +
-            '\n    in Foo (at **)',
-        ]);
-      });
-
      it('does not restart rendering for initial render', async () => {
        function Bar(props) {
          Scheduler.unstable_yieldValue('Bar');
@@ -1467,6 +1448,55 @@ function loadModules({
            'Caught an error: Error in host config.',
          );
        });
+
+        it('does not drop mounted effects', async () => {
+          let never = {then() {}};
+
+          let setShouldSuspend;
+          function App() {
+            const [shouldSuspend, _setShouldSuspend] = React.useState(0);
+            setShouldSuspend = _setShouldSuspend;
+            return (
+              <Suspense fallback="Loading...">
+                <Child shouldSuspend={shouldSuspend} />
+              </Suspense>
+            );
+          }
+
+          function Child({shouldSuspend}) {
+            if (shouldSuspend) {
+              throw never;
+            }
+
+            React.useEffect(() => {
+              Scheduler.unstable_yieldValue('Mount');
+              return () => {
+                Scheduler.unstable_yieldValue('Unmount');
+              };
+            }, []);
+
+            return 'Child';
+          }
+
+          const root = ReactNoop.createLegacyRoot(null);
+          await ReactNoop.act(async () => {
+            root.render(<App />);
+          });
+          expect(Scheduler).toHaveYielded(['Mount']);
+          expect(root).toMatchRenderedOutput('Child');
+
+          // Suspend the child. This puts it into an inconsistent state.
+          await ReactNoop.act(async () => {
+            setShouldSuspend(true);
+          });
+          expect(root).toMatchRenderedOutput('Loading...');
+
+          // Unmount everying
+          await ReactNoop.act(async () => {
+            root.render(null);
+          });
+          expect(Scheduler).toHaveYielded(['Unmount']);
+        });
      });

      it('does not call lifecycles of a suspended component', async () => {
Author	SHA1	Message	Date
Dan Abramov	da834083cc	Don't fire the render phase update warning for class lifecycles (#18330 ) * Change the warning to not say "function body" This warning is more generic and may happen with class components too. * Dedupe by the rendering component * Don't warn outside of render	2020-03-19 11:36:20 -07:00
Jovi De Croock	7554ab7672	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>	2020-03-19 11:34:11 -07:00
Dan Abramov	2186beb775	Remove unnecessary warnings (#18135 )	2020-03-19 11:34:07 -07:00
Andrew Clark	82cf50ab3a	Bugfix: Dropped effects in Legacy Mode Suspense (#18238 ) * Failing: Dropped effects in Legacy Mode Suspense * Transfer mounted effects on suspend in legacy mode In legacy mode, a component that suspends bails out and commit in its previous state. If the component previously had mounted effects, we must transfer those to the work-in-progress so they don't get dropped.	2020-03-19 10:38:39 -07:00