diff --git a/packages/react-call-return/src/__tests__/ReactCallReturn-test.js b/packages/react-call-return/src/__tests__/ReactCallReturn-test.js index 8f783945aa24c..2726557f62ba3 100644 --- a/packages/react-call-return/src/__tests__/ReactCallReturn-test.js +++ b/packages/react-call-return/src/__tests__/ReactCallReturn-test.js @@ -290,7 +290,10 @@ describe('ReactCallReturn', () => { , ); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Return', + ); expect(ops).toEqual([ 'Mount Return 1', diff --git a/packages/react-reconciler/src/ReactDebugAsyncWarnings.js b/packages/react-reconciler/src/ReactDebugAsyncWarnings.js new file mode 100644 index 0000000000000..ec2acd96aafac --- /dev/null +++ b/packages/react-reconciler/src/ReactDebugAsyncWarnings.js @@ -0,0 +1,166 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {Fiber} from './ReactFiber'; + +import getComponentName from 'shared/getComponentName'; +import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; +import {AsyncUpdates} from './ReactTypeOfInternalContext'; +import warning from 'fbjs/lib/warning'; + +type LIFECYCLE = + | 'UNSAFE_componentWillMount' + | 'UNSAFE_componentWillReceiveProps' + | 'UNSAFE_componentWillUpdate'; +type LifecycleToComponentsMap = {[lifecycle: LIFECYCLE]: Array}; +type FiberToLifecycleMap = Map; + +const ReactDebugAsyncWarnings = { + discardPendingWarnings(): void {}, + flushPendingAsyncWarnings(): void {}, + recordLifecycleWarnings(fiber: Fiber, instance: any): void {}, +}; + +if (__DEV__) { + const LIFECYCLE_SUGGESTIONS = { + UNSAFE_componentWillMount: 'componentDidMount', + UNSAFE_componentWillReceiveProps: 'static getDerivedStateFromProps', + UNSAFE_componentWillUpdate: 'componentDidUpdate', + }; + + let pendingWarningsMap: FiberToLifecycleMap = new Map(); + + // Tracks components we have already warned about. + const didWarnSet = new Set(); + + ReactDebugAsyncWarnings.discardPendingWarnings = () => { + pendingWarningsMap = new Map(); + }; + + ReactDebugAsyncWarnings.flushPendingAsyncWarnings = () => { + ((pendingWarningsMap: any): FiberToLifecycleMap).forEach( + (lifecycleWarningsMap, asyncRoot) => { + const lifecyclesWarningMesages = []; + + Object.keys(lifecycleWarningsMap).forEach(lifecycle => { + const lifecycleWarnings = lifecycleWarningsMap[lifecycle]; + if (lifecycleWarnings.length > 0) { + const componentNames = new Set(); + lifecycleWarnings.forEach(fiber => { + componentNames.add(getComponentName(fiber) || 'Component'); + didWarnSet.add(fiber.type); + }); + + const formatted = lifecycle.replace('UNSAFE_', ''); + const suggestion = LIFECYCLE_SUGGESTIONS[lifecycle]; + const sortedComponentNames = Array.from(componentNames) + .sort() + .join(', '); + + lifecyclesWarningMesages.push( + `${formatted}: Please update the following components to use ` + + `${suggestion} instead: ${sortedComponentNames}`, + ); + } + }); + + if (lifecyclesWarningMesages.length > 0) { + const asyncRootComponentStack = getStackAddendumByWorkInProgressFiber( + asyncRoot, + ); + + warning( + false, + 'Unsafe lifecycle methods were found within the following async tree:%s' + + '\n\n%s' + + '\n\nLearn more about this warning here:' + + '\nhttps://fb.me/react-async-component-lifecycle-hooks', + asyncRootComponentStack, + lifecyclesWarningMesages.join('\n\n'), + ); + } + }, + ); + + pendingWarningsMap = new Map(); + }; + + const getAsyncRoot = (fiber: Fiber): Fiber => { + let maybeAsyncRoot = null; + + while (fiber !== null) { + if (fiber.internalContextTag & AsyncUpdates) { + maybeAsyncRoot = fiber; + } + + fiber = fiber.return; + } + + return maybeAsyncRoot; + }; + + ReactDebugAsyncWarnings.recordLifecycleWarnings = ( + fiber: Fiber, + instance: any, + ) => { + const asyncRoot = getAsyncRoot(fiber); + + // Dedup strategy: Warn once per component. + // This is difficult to track any other way since component names + // are often vague and are likely to collide between 3rd party libraries. + // An expand property is probably okay to use here since it's DEV-only, + // and will only be set in the event of serious warnings. + if (didWarnSet.has(fiber.type)) { + return; + } + + let warningsForRoot; + if (!pendingWarningsMap.has(asyncRoot)) { + warningsForRoot = { + UNSAFE_componentWillMount: [], + UNSAFE_componentWillReceiveProps: [], + UNSAFE_componentWillUpdate: [], + }; + + pendingWarningsMap.set(asyncRoot, warningsForRoot); + } else { + warningsForRoot = pendingWarningsMap.get(asyncRoot); + } + + const unsafeLifecycles = []; + if ( + typeof instance.componentWillMount === 'function' || + typeof instance.UNSAFE_componentWillMount === 'function' + ) { + unsafeLifecycles.push('UNSAFE_componentWillMount'); + } + if ( + typeof instance.componentWillReceiveProps === 'function' || + typeof instance.UNSAFE_componentWillReceiveProps === 'function' + ) { + unsafeLifecycles.push('UNSAFE_componentWillReceiveProps'); + } + if ( + typeof instance.componentWillUpdate === 'function' || + typeof instance.UNSAFE_componentWillUpdate === 'function' + ) { + unsafeLifecycles.push('UNSAFE_componentWillUpdate'); + } + + if (unsafeLifecycles.length > 0) { + unsafeLifecycles.forEach(lifecycle => { + ((warningsForRoot: any): LifecycleToComponentsMap)[lifecycle].push( + fiber, + ); + }); + } + }; +} + +export default ReactDebugAsyncWarnings; diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index dd406c86fe7b3..4eed41271806d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -16,6 +16,7 @@ import { enableAsyncSubtreeAPI, warnAboutDeprecatedLifecycles, } from 'shared/ReactFeatureFlags'; +import ReactDebugAsyncWarnings from './ReactDebugAsyncWarnings'; import {isMounted} from 'react-reconciler/reflection'; import * as ReactInstanceMap from 'shared/ReactInstanceMap'; import emptyObject from 'fbjs/lib/emptyObject'; @@ -642,6 +643,17 @@ export default function( workInProgress.internalContextTag |= AsyncUpdates; } + if (__DEV__) { + // If we're inside of an async sub-tree, + // Warn about any unsafe lifecycles on this class component. + if (workInProgress.internalContextTag & AsyncUpdates) { + ReactDebugAsyncWarnings.recordLifecycleWarnings( + workInProgress, + instance, + ); + } + } + if ( typeof instance.UNSAFE_componentWillMount === 'function' || typeof instance.componentWillMount === 'function' diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 9259468e8fc24..f6c8445232a2b 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -16,6 +16,7 @@ import type {ExpirationTime} from './ReactFiberExpirationTime'; import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook'; import ReactErrorUtils from 'shared/ReactErrorUtils'; import {ReactCurrentOwner} from 'shared/ReactGlobalSharedState'; +import ReactDebugAsyncWarnings from './ReactDebugAsyncWarnings'; import { PerformedWork, Placement, @@ -310,6 +311,10 @@ export default function( } function commitAllLifeCycles() { + if (__DEV__) { + ReactDebugAsyncWarnings.flushPendingAsyncWarnings(); + } + while (nextEffect !== null) { const effectTag = nextEffect.effectTag; @@ -651,6 +656,10 @@ export default function( } 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 diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js index 2400f1b500cb5..6f5c640a2afc0 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.js @@ -1430,8 +1430,8 @@ describe('ReactIncremental', () => { changeState() { this.setState({foo: 'bar'}); } - componentWillUpdate() { - ops.push('componentWillUpdate'); + componentDidUpdate() { + ops.push('componentDidUpdate'); } render() { ops.push('render'); @@ -1451,7 +1451,7 @@ describe('ReactIncremental', () => { instance.changeState(); ReactNoop.flush(); - expect(ops).toEqual(['componentWillUpdate', 'render']); + expect(ops).toEqual(['render', 'componentDidUpdate']); expect(instance.state).toEqual({foo: 'bar'}); }); @@ -2335,7 +2335,10 @@ describe('ReactIncremental', () => { } ReactNoop.render(); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: MyComponent', + ); expect(ops).toEqual([ 'render', diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js index 1db7a44cf4423..503ecaa6f81ab 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js @@ -254,7 +254,12 @@ describe('ReactDebugFiberPerf', () => { , ); addComment('Should not print a warning'); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev([ + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: NotCascading' + + '\n\ncomponentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: NotCascading', + ]); ReactNoop.render( @@ -288,7 +293,14 @@ describe('ReactDebugFiberPerf', () => { } ReactNoop.render(); addComment('Mount'); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: AllLifecycles' + + '\n\ncomponentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: AllLifecycles' + + '\n\ncomponentWillUpdate: Please update the following components ' + + 'to use componentDidUpdate instead: AllLifecycles', + ); ReactNoop.render(); addComment('Update'); ReactNoop.flush(); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js index c1414e077b19d..e9ef0336a96e8 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalReflection-test.js @@ -59,7 +59,10 @@ describe('ReactIncrementalReflection', () => { ops = []; // Render the rest and commit the updates. - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Component', + ); expect(ops).toEqual(['componentDidMount', true]); @@ -97,7 +100,10 @@ describe('ReactIncrementalReflection', () => { } ReactNoop.render(); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Component', + ); expect(ops).toEqual(['Component']); ops = []; @@ -184,7 +190,12 @@ describe('ReactIncrementalReflection', () => { // not find any host nodes in it. expect(ReactNoop.findInstance(classInstance)).toBe(null); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Component' + + '\n\ncomponentWillUpdate: Please update the following components ' + + 'to use componentDidUpdate instead: Component', + ); const hostSpan = classInstance.span; expect(hostSpan).toBeDefined(); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js index 557475e7269e5..5f46ef4db24f6 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js @@ -346,7 +346,10 @@ describe('ReactIncrementalScheduling', () => { } ReactNoop.render(); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: Foo', + ); ReactNoop.render(); expect(ReactNoop.flush()).toEqual([ diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js index c293a070bf18e..ff4a35611f705 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js @@ -307,12 +307,16 @@ describe('ReactIncrementalUpdates', () => { } } ReactNoop.render(); - ReactNoop.flush(); + expect(ReactNoop.flush).toWarnDev( + 'componentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: Foo', + ); ops = []; ReactNoop.flushSync(() => { instance.setState({a: 'a'}); + ReactNoop.render(); // Trigger componentWillReceiveProps }); diff --git a/packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js b/packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js index 224cb7670bcc2..a559d401001ea 100644 --- a/packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js +++ b/packages/react/src/__tests__/ReactAsyncClassComponent-test.internal.js @@ -126,4 +126,298 @@ describe('ReactAsyncClassComponent', () => { expect(instance.state.count).toBe(2); }); }); + + describe('async subtree', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactTestRenderer = require('react-test-renderer'); + }); + + it('should warn about unsafe legacy lifecycle methods within the tree', () => { + class SyncRoot extends React.Component { + UNSAFE_componentWillMount() {} + UNSAFE_componentWillUpdate() {} + UNSAFE_componentWillReceiveProps() {} + render() { + return ; + } + } + class AsyncRoot extends React.unstable_AsyncComponent { + UNSAFE_componentWillMount() {} + UNSAFE_componentWillUpdate() {} + render() { + return ( +
+ + + +
+ + +
+
+ ); + } + } + function Wrapper({children}) { + return
{children}
; + } + class Foo extends React.Component { + UNSAFE_componentWillReceiveProps() {} + render() { + return null; + } + } + class Bar extends React.Component { + UNSAFE_componentWillReceiveProps() {} + render() { + return null; + } + } + + let rendered; + expect(() => { + rendered = ReactTestRenderer.create(); + }).toWarnDev( + 'Unsafe lifecycle methods were found within the following async tree:' + + '\n in AsyncRoot (at **)' + + '\n in SyncRoot (at **)' + + '\n\ncomponentWillMount: Please update the following components ' + + 'to use componentDidMount instead: AsyncRoot' + + '\n\ncomponentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: Bar, Foo' + + '\n\ncomponentWillUpdate: Please update the following components ' + + 'to use componentDidUpdate instead: AsyncRoot' + + '\n\nLearn more about this warning here:' + + '\nhttps://fb.me/react-async-component-lifecycle-hooks', + ); + + // Dedupe + rendered = ReactTestRenderer.create(); + rendered.update(); + }); + + it('should coalesce warnings by lifecycle name', () => { + class SyncRoot extends React.Component { + UNSAFE_componentWillMount() {} + UNSAFE_componentWillUpdate() {} + UNSAFE_componentWillReceiveProps() {} + render() { + return ; + } + } + class AsyncRoot extends React.unstable_AsyncComponent { + UNSAFE_componentWillMount() {} + UNSAFE_componentWillUpdate() {} + render() { + return ; + } + } + class Parent extends React.Component { + componentWillMount() {} + componentWillUpdate() {} + componentWillReceiveProps() {} + render() { + return ; + } + } + class Child extends React.Component { + UNSAFE_componentWillReceiveProps() {} + render() { + return null; + } + } + + let rendered; + + expect( + () => (rendered = ReactTestRenderer.create()), + ).toWarnDev( + 'Unsafe lifecycle methods were found within the following async tree:' + + '\n in AsyncRoot (at **)' + + '\n in SyncRoot (at **)' + + '\n\ncomponentWillMount: Please update the following components ' + + 'to use componentDidMount instead: AsyncRoot, Parent' + + '\n\ncomponentWillReceiveProps: Please update the following components ' + + 'to use static getDerivedStateFromProps instead: Child, Parent' + + '\n\ncomponentWillUpdate: Please update the following components ' + + 'to use componentDidUpdate instead: AsyncRoot, Parent' + + '\n\nLearn more about this warning here:' + + '\nhttps://fb.me/react-async-component-lifecycle-hooks', + ); + + // Dedupe + rendered = ReactTestRenderer.create(); + rendered.update(); + }); + + it('should group warnings by async root', () => { + class SyncRoot extends React.Component { + UNSAFE_componentWillMount() {} + UNSAFE_componentWillUpdate() {} + UNSAFE_componentWillReceiveProps() {} + render() { + return ( +
+ + +
+ ); + } + } + class AsyncRootOne extends React.unstable_AsyncComponent { + render() { + return ( + + + + ); + } + } + class AsyncRootTwo extends React.unstable_AsyncComponent { + render() { + return ( + + + + ); + } + } + class Foo extends React.Component { + componentWillMount() {} + render() { + return this.props.children; + } + } + class Bar extends React.Component { + componentWillMount() {} + render() { + return null; + } + } + class Baz extends React.Component { + componentWillMount() {} + render() { + return null; + } + } + + let rendered; + + expect( + () => (rendered = ReactTestRenderer.create()), + ).toWarnDev([ + 'Unsafe lifecycle methods were found within the following async tree:' + + '\n in AsyncRootOne (at **)' + + '\n in div (at **)' + + '\n in SyncRoot (at **)' + + '\n\ncomponentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Bar, Foo', + 'Unsafe lifecycle methods were found within the following async tree:' + + '\n in AsyncRootTwo (at **)' + + '\n in div (at **)' + + '\n in SyncRoot (at **)' + + '\n\ncomponentWillMount: Please update the following components ' + + 'to use componentDidMount instead: Baz', + ]); + + // Dedupe + rendered = ReactTestRenderer.create(); + rendered.update(); + }); + + it('should warn about components not present during the initial render', () => { + class AsyncRoot extends React.unstable_AsyncComponent { + render() { + return this.props.foo ? : ; + } + } + class Foo extends React.Component { + UNSAFE_componentWillMount() {} + render() { + return null; + } + } + class Bar extends React.Component { + UNSAFE_componentWillMount() {} + render() { + return null; + } + } + + let rendered; + expect(() => { + rendered = ReactTestRenderer.create(); + }).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', + ); + + expect(() => rendered.update()).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', + ); + + // Dedupe + rendered.update(); + rendered.update(); + }); + + it('should not warn about uncommitted lifecycles in the event of an error', () => { + let caughtError; + + class AsyncRoot extends React.unstable_AsyncComponent { + render() { + return ; + } + } + class ErrorBoundary extends React.Component { + state = { + error: null, + }; + componentDidCatch(error) { + caughtError = error; + this.setState({error}); + } + render() { + return this.state.error ? : ; + } + } + class Foo extends React.Component { + UNSAFE_componentWillMount() {} + render() { + throw Error('whoops'); + } + } + class Bar extends React.Component { + UNSAFE_componentWillMount() {} + render() { + return null; + } + } + + expect(() => { + ReactTestRenderer.create(); + }).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); + }); + }); });