From 6aa9b2be2c79da0f3d93f390d5eb9e9bcd96174e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 21 Mar 2018 11:47:40 -0700 Subject: [PATCH 1/5] Expanded DEV-only warnings for gDSFP and legacy lifecycles --- .../__tests__/ReactComponentLifeCycle-test.js | 82 ++++++++++++++++- .../src/ReactFiberClassComponent.js | 88 ++++++++++-------- .../src/ReactShallowRenderer.js | 47 ++++++---- .../__tests__/ReactShallowRenderer-test.js | 90 +++++++++++++++++-- .../createReactClassIntegration-test.js | 8 +- 5 files changed, 254 insertions(+), 61 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index aa81546bd21..ee7253f5081 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -669,7 +669,7 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); expect(() => ReactDOM.render(, container)).toWarnDev( - 'Defines both componentWillReceiveProps', + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.', ); }); @@ -695,7 +695,85 @@ describe('ReactComponentLifeCycle', () => { const container = document.createElement('div'); expect(() => ReactDOM.render(, container)).toWarnDev( - 'Defines both componentWillReceiveProps', + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.', + ); + }); + + it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new static gDSFP is present', () => { + const container = document.createElement('div'); + + class AllLegacyLifecycles extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate', + ); + + class WillMount extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount', + ); + + class WillMountAndUpdate extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + componentWillUpdate() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillUpdate', + ); + + class WillReceiveProps extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillReceiveProps() {} + render() { + return null; + } + } + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillReceiveProps', ); }); diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 7f0514f6e41..e1d6cb0b0d6 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -41,14 +41,14 @@ const isArray = Array.isArray; let didWarnAboutStateAssignmentForComponent; let didWarnAboutUndefinedDerivedState; let didWarnAboutUninitializedState; -let didWarnAboutWillReceivePropsAndDerivedState; +let didWarnAboutLegacyLifecyclesAndDerivedState; let warnOnInvalidCallback; if (__DEV__) { didWarnAboutStateAssignmentForComponent = {}; didWarnAboutUndefinedDerivedState = {}; didWarnAboutUninitializedState = {}; - didWarnAboutWillReceivePropsAndDerivedState = {}; + didWarnAboutLegacyLifecyclesAndDerivedState = {}; const didWarnOnInvalidCallback = {}; @@ -424,20 +424,54 @@ export default function( adoptClassInstance(workInProgress, instance); if (__DEV__) { - if ( - typeof ctor.getDerivedStateFromProps === 'function' && - state === null - ) { - const componentName = getComponentName(workInProgress) || 'Unknown'; - if (!didWarnAboutUninitializedState[componentName]) { - warning( - false, - '%s: Did not properly initialize state during construction. ' + - 'Expected state to be an object, but it was %s.', - componentName, - instance.state === null ? 'null' : 'undefined', - ); - didWarnAboutUninitializedState[componentName] = true; + if (typeof ctor.getDerivedStateFromProps === 'function') { + if (state === null) { + const componentName = getComponentName(workInProgress) || 'Unknown'; + if (!didWarnAboutUninitializedState[componentName]) { + warning( + false, + '%s: Did not properly initialize state during construction. ' + + 'Expected state to be an object, but it was %s.', + componentName, + instance.state === null ? 'null' : 'undefined', + ); + didWarnAboutUninitializedState[componentName] = true; + } + } + + // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. + // Warn about these lifecycles if they are present. + // Don't warn about react-lifecycles-compat polyfilled methods though. + const definesWillMount = + (typeof instance.componentWillMount === 'function' && + instance.componentWillMount.__suppressDeprecationWarning !== + true) || + typeof instance.UNSAFE_componentWillMount === 'function'; + const definesWillReceiveProps = + (typeof instance.componentWillReceiveProps === 'function' && + instance.componentWillReceiveProps.__suppressDeprecationWarning !== + true) || + typeof instance.UNSAFE_componentWillReceiveProps === 'function'; + const definesWillUpdate = + typeof instance.componentWillUpdate === 'function' || + typeof instance.UNSAFE_componentWillUpdate === 'function'; + + if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { + const componentName = getComponentName(workInProgress) || 'Unknown'; + if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { + warning( + false, + 'Unsafe legacy lifecycles will not be called for components using ' + + 'the new getDerivedStateFromProps() API.\n\n' + + '%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' + + '%s%s%s', + componentName, + definesWillMount ? '\n componentWillMount' : '', + definesWillReceiveProps ? '\n componentWillReceiveProps' : '', + definesWillUpdate ? '\n componentWillUpdate' : '', + ); + didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; + } } } } @@ -539,28 +573,6 @@ export default function( const {type} = workInProgress; if (typeof type.getDerivedStateFromProps === 'function') { - if (__DEV__) { - // Don't warn about react-lifecycles-compat polyfilled components - if ( - (typeof instance.componentWillReceiveProps === 'function' && - instance.componentWillReceiveProps.__suppressDeprecationWarning !== - true) || - typeof instance.UNSAFE_componentWillReceiveProps === 'function' - ) { - const componentName = getComponentName(workInProgress) || 'Unknown'; - if (!didWarnAboutWillReceivePropsAndDerivedState[componentName]) { - warning( - false, - '%s: Defines both componentWillReceiveProps() and static ' + - 'getDerivedStateFromProps() methods. We recommend using ' + - 'only getDerivedStateFromProps().', - componentName, - ); - didWarnAboutWillReceivePropsAndDerivedState[componentName] = true; - } - } - } - if ( debugRenderPhaseSideEffects || (debugRenderPhaseSideEffectsForStrictMode && diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index b08732e0fd3..6cd713b6275 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -23,7 +23,7 @@ let didWarnAboutLegacyWillReceiveProps; let didWarnAboutLegacyWillUpdate; let didWarnAboutUndefinedDerivedState; let didWarnAboutUninitializedState; -let didWarnAboutWillReceivePropsAndDerivedState; +let didWarnAboutLegacyLifecyclesAndDerivedState; if (__DEV__) { if (warnAboutDeprecatedLifecycles) { @@ -33,7 +33,7 @@ if (__DEV__) { } didWarnAboutUndefinedDerivedState = {}; didWarnAboutUninitializedState = {}; - didWarnAboutWillReceivePropsAndDerivedState = {}; + didWarnAboutLegacyLifecyclesAndDerivedState = {}; } class ReactShallowRenderer { @@ -352,23 +352,40 @@ class ReactShallowRenderer { if (typeof type.getDerivedStateFromProps === 'function') { if (__DEV__) { - // Don't warn about react-lifecycles-compat polyfilled components - if ( - (typeof this._instance.componentWillReceiveProps === 'function' && - this._instance.componentWillReceiveProps - .__suppressDeprecationWarning !== true) || - typeof this._instance.UNSAFE_componentWillReceiveProps === 'function' - ) { - const componentName = getName(type, this._instance); - if (!didWarnAboutWillReceivePropsAndDerivedState[componentName]) { + const instance = this._instance; + + // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. + // Warn about these lifecycles if they are present. + // Don't warn about react-lifecycles-compat polyfilled methods though. + const definesWillMount = + (typeof instance.componentWillMount === 'function' && + instance.componentWillMount.__suppressDeprecationWarning !== + true) || + typeof instance.UNSAFE_componentWillMount === 'function'; + const definesWillReceiveProps = + (typeof instance.componentWillReceiveProps === 'function' && + instance.componentWillReceiveProps.__suppressDeprecationWarning !== + true) || + typeof instance.UNSAFE_componentWillReceiveProps === 'function'; + const definesWillUpdate = + typeof instance.componentWillUpdate === 'function' || + typeof instance.UNSAFE_componentWillUpdate === 'function'; + + if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { + const componentName = getName(type, instance); + if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { warning( false, - '%s: Defines both componentWillReceiveProps() and static ' + - 'getDerivedStateFromProps() methods. We recommend using ' + - 'only getDerivedStateFromProps().', + 'Unsafe legacy lifecycles will not be called for components using ' + + 'the new getDerivedStateFromProps() API.\n\n' + + '%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' + + '%s%s%s', componentName, + definesWillMount ? '\n componentWillMount' : '', + definesWillReceiveProps ? '\n componentWillReceiveProps' : '', + definesWillUpdate ? '\n componentWillUpdate' : '', ); - didWarnAboutWillReceivePropsAndDerivedState[componentName] = true; + didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; } } } diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 2bf95824c32..855a0d7214c 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -125,8 +125,90 @@ describe('ReactShallowRenderer', () => { } const shallowRenderer = createRenderer(); - expect(() => shallowRenderer.render()).toWarnDev( - 'Defines both componentWillReceiveProps() and static getDerivedStateFromProps()', + expect(() => shallowRenderer.render()).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.', + ); + }); + + it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new static gDSFP is present', () => { + let shallowRenderer; + + class AllLegacyLifecycles extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} + render() { + return null; + } + } + + shallowRenderer = createRenderer(); + expect(() => shallowRenderer.render()).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate', + ); + + class WillMount extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + render() { + return null; + } + } + + shallowRenderer = createRenderer(); + expect(() => shallowRenderer.render()).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount', + ); + + class WillMountAndUpdate extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillMount() {} + componentWillUpdate() {} + render() { + return null; + } + } + + shallowRenderer = createRenderer(); + expect(() => shallowRenderer.render()).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillUpdate', + ); + + class WillReceiveProps extends React.Component { + state = {}; + static getDerivedStateFromProps() { + return null; + } + componentWillReceiveProps() {} + render() { + return null; + } + } + + shallowRenderer = createRenderer(); + expect(() => shallowRenderer.render()).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillReceiveProps', ); }); @@ -1238,9 +1320,7 @@ describe('ReactShallowRenderer', () => { const shallowRenderer = createRenderer(); expect(() => shallowRenderer.render()).toWarnDev( - 'ComponentWithWarnings: Defines both componentWillReceiveProps() and static ' + - 'getDerivedStateFromProps() methods. We recommend using ' + - 'only getDerivedStateFromProps().', + 'ComponentWithWarnings uses getDerivedStateFromProps() but also contains the following legacy lifecycles', ); // Should not log duplicate warning diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index c567b4fd853..76dbe49c4fb 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -475,7 +475,13 @@ describe('create-react-class-integration', () => { expect(() => { ReactDOM.render(, document.createElement('div')); - }).toWarnDev('Defines both componentWillReceiveProps'); + }).toWarnDev( + 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + + 'Unknown uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + ' componentWillMount\n' + + ' componentWillReceiveProps\n' + + ' componentWillUpdate', + ); ReactDOM.render(, document.createElement('div')); }); From 1844e385cb3f07b6840d5ce9f99a8446b92cb50e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 22 Mar 2018 10:02:34 -0700 Subject: [PATCH 2/5] Expanded warning message to add fb.me link for more info --- .../__tests__/ReactComponentLifeCycle-test.js | 16 ++++++++++++---- .../src/ReactFiberClassComponent.js | 4 +++- .../src/ReactShallowRenderer.js | 4 +++- .../src/__tests__/ReactShallowRenderer-test.js | 16 ++++++++++++---- .../createReactClassIntegration-test.js | 4 +++- 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index ee7253f5081..d52940d2b50 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -720,7 +720,9 @@ describe('ReactComponentLifeCycle', () => { 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' componentWillReceiveProps\n' + - ' componentWillUpdate', + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillMount extends React.Component { @@ -737,7 +739,9 @@ describe('ReactComponentLifeCycle', () => { expect(() => ReactDOM.render(, container)).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount', + ' componentWillMount\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillMountAndUpdate extends React.Component { @@ -756,7 +760,9 @@ describe('ReactComponentLifeCycle', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillUpdate', + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillReceiveProps extends React.Component { @@ -773,7 +779,9 @@ describe('ReactComponentLifeCycle', () => { expect(() => ReactDOM.render(, container)).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillReceiveProps', + ' componentWillReceiveProps\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); }); diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index e1d6cb0b0d6..c43b8b01381 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -464,7 +464,9 @@ export default function( 'Unsafe legacy lifecycles will not be called for components using ' + 'the new getDerivedStateFromProps() API.\n\n' + '%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' + - '%s%s%s', + '%s%s%s\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, definesWillMount ? '\n componentWillMount' : '', definesWillReceiveProps ? '\n componentWillReceiveProps' : '', diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 6cd713b6275..945ed7c3d07 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -379,7 +379,9 @@ class ReactShallowRenderer { 'Unsafe legacy lifecycles will not be called for components using ' + 'the new getDerivedStateFromProps() API.\n\n' + '%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' + - '%s%s%s', + '%s%s%s\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, definesWillMount ? '\n componentWillMount' : '', definesWillReceiveProps ? '\n componentWillReceiveProps' : '', diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 855a0d7214c..e161291727d 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -152,7 +152,9 @@ describe('ReactShallowRenderer', () => { 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' componentWillReceiveProps\n' + - ' componentWillUpdate', + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillMount extends React.Component { @@ -170,7 +172,9 @@ describe('ReactShallowRenderer', () => { expect(() => shallowRenderer.render()).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount', + ' componentWillMount\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillMountAndUpdate extends React.Component { @@ -190,7 +194,9 @@ describe('ReactShallowRenderer', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillUpdate', + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); class WillReceiveProps extends React.Component { @@ -208,7 +214,9 @@ describe('ReactShallowRenderer', () => { expect(() => shallowRenderer.render()).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillReceiveProps', + ' componentWillReceiveProps\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); }); diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index 76dbe49c4fb..5ef8248177c 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -480,7 +480,9 @@ describe('create-react-class-integration', () => { 'Unknown uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' componentWillReceiveProps\n' + - ' componentWillUpdate', + ' componentWillUpdate\n\n' + + 'The above lifecycles should be removed. Learn more about this warning here:\n' + + 'https://fb.me/react-async-component-lifecycle-hooks', ); ReactDOM.render(, document.createElement('div')); }); From 04d2b35f148185caf7368f9424e902f7476063b4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 22 Mar 2018 10:21:14 -0700 Subject: [PATCH 3/5] Changed default warning component name from 'Unknown' to 'Component' --- packages/react-reconciler/src/ReactFiberClassComponent.js | 2 +- packages/react-test-renderer/src/ReactShallowRenderer.js | 2 +- .../react/src/__tests__/createReactClassIntegration-test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index c43b8b01381..9fbecc3da6c 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -457,7 +457,7 @@ export default function( typeof instance.UNSAFE_componentWillUpdate === 'function'; if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { - const componentName = getComponentName(workInProgress) || 'Unknown'; + const componentName = getComponentName(workInProgress) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { warning( false, diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 945ed7c3d07..c6d8711d378 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -372,7 +372,7 @@ class ReactShallowRenderer { typeof instance.UNSAFE_componentWillUpdate === 'function'; if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { - const componentName = getName(type, instance); + const componentName = getName(type, instance) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { warning( false, diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index 5ef8248177c..3f15707f786 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -477,7 +477,7 @@ describe('create-react-class-integration', () => { ReactDOM.render(, document.createElement('div')); }).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + - 'Unknown uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + + 'Component uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + ' componentWillReceiveProps\n' + ' componentWillUpdate\n\n' + From 611ddca18ba6eb3e8662d88f047c53158b6d76c7 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 22 Mar 2018 10:59:21 -0700 Subject: [PATCH 4/5] Imrpove gDSFP dev warning to show specific lifecycle name (e.g. cWM or UNSAFE_cWM) --- .../__tests__/ReactComponentLifeCycle-test.js | 12 ++--- .../src/ReactFiberClassComponent.js | 53 ++++++++++++------- .../src/ReactShallowRenderer.js | 53 ++++++++++++------- .../__tests__/ReactShallowRenderer-test.js | 12 ++--- 4 files changed, 82 insertions(+), 48 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 88b2f888271..0788c871157 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -708,7 +708,7 @@ describe('ReactComponentLifeCycle', () => { return null; } componentWillMount() {} - componentWillReceiveProps() {} + UNSAFE_componentWillReceiveProps() {} componentWillUpdate() {} render() { return null; @@ -719,7 +719,7 @@ describe('ReactComponentLifeCycle', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillReceiveProps\n' + + ' UNSAFE_componentWillReceiveProps\n' + ' componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', @@ -730,7 +730,7 @@ describe('ReactComponentLifeCycle', () => { static getDerivedStateFromProps() { return null; } - componentWillMount() {} + UNSAFE_componentWillMount() {} render() { return null; } @@ -739,7 +739,7 @@ describe('ReactComponentLifeCycle', () => { expect(() => ReactDOM.render(, container)).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n\n' + + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', ); @@ -750,7 +750,7 @@ describe('ReactComponentLifeCycle', () => { return null; } componentWillMount() {} - componentWillUpdate() {} + UNSAFE_componentWillUpdate() {} render() { return null; } @@ -760,7 +760,7 @@ describe('ReactComponentLifeCycle', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillUpdate\n\n' + + ' UNSAFE_componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', ); diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 72bcd02893f..5240f16b736 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -437,21 +437,38 @@ export default function( // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. // Warn about these lifecycles if they are present. // Don't warn about react-lifecycles-compat polyfilled methods though. - const definesWillMount = - (typeof instance.componentWillMount === 'function' && - instance.componentWillMount.__suppressDeprecationWarning !== - true) || - typeof instance.UNSAFE_componentWillMount === 'function'; - const definesWillReceiveProps = - (typeof instance.componentWillReceiveProps === 'function' && - instance.componentWillReceiveProps.__suppressDeprecationWarning !== - true) || - typeof instance.UNSAFE_componentWillReceiveProps === 'function'; - const definesWillUpdate = - typeof instance.componentWillUpdate === 'function' || - typeof instance.UNSAFE_componentWillUpdate === 'function'; - - if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { + let willMount = null; + let willReceiveProps = null; + let willUpdate = null; + if ( + typeof instance.componentWillMount === 'function' && + instance.componentWillMount.__suppressDeprecationWarning !== true + ) { + willMount = 'componentWillMount'; + } else if (typeof instance.UNSAFE_componentWillMount === 'function') { + willMount = 'UNSAFE_componentWillMount'; + } + if ( + typeof instance.componentWillReceiveProps === 'function' && + instance.componentWillReceiveProps.__suppressDeprecationWarning !== + true + ) { + willReceiveProps = 'componentWillReceiveProps'; + } else if ( + typeof instance.UNSAFE_componentWillReceiveProps === 'function' + ) { + willReceiveProps = 'UNSAFE_componentWillReceiveProps'; + } + if (typeof instance.componentWillUpdate === 'function') { + willUpdate = 'componentWillUpdate'; + } else if (typeof instance.UNSAFE_componentWillUpdate === 'function') { + willUpdate = 'UNSAFE_componentWillUpdate'; + } + if ( + willMount !== null || + willReceiveProps !== null || + willUpdate !== null + ) { const componentName = getComponentName(workInProgress) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { warning( @@ -463,9 +480,9 @@ export default function( 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, - definesWillMount ? '\n componentWillMount' : '', - definesWillReceiveProps ? '\n componentWillReceiveProps' : '', - definesWillUpdate ? '\n componentWillUpdate' : '', + willMount !== null ? `\n ${willMount}` : '', + willReceiveProps !== null ? `\n ${willReceiveProps}` : '', + willUpdate !== null ? `\n ${willUpdate}` : '', ); didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; } diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index c6d8711d378..9dc7b5d2cbd 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -357,21 +357,38 @@ class ReactShallowRenderer { // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. // Warn about these lifecycles if they are present. // Don't warn about react-lifecycles-compat polyfilled methods though. - const definesWillMount = - (typeof instance.componentWillMount === 'function' && - instance.componentWillMount.__suppressDeprecationWarning !== - true) || - typeof instance.UNSAFE_componentWillMount === 'function'; - const definesWillReceiveProps = - (typeof instance.componentWillReceiveProps === 'function' && - instance.componentWillReceiveProps.__suppressDeprecationWarning !== - true) || - typeof instance.UNSAFE_componentWillReceiveProps === 'function'; - const definesWillUpdate = - typeof instance.componentWillUpdate === 'function' || - typeof instance.UNSAFE_componentWillUpdate === 'function'; - - if (definesWillMount || definesWillReceiveProps || definesWillUpdate) { + let willMount = null; + let willReceiveProps = null; + let willUpdate = null; + if ( + typeof instance.componentWillMount === 'function' && + instance.componentWillMount.__suppressDeprecationWarning !== true + ) { + willMount = 'componentWillMount'; + } else if (typeof instance.UNSAFE_componentWillMount === 'function') { + willMount = 'UNSAFE_componentWillMount'; + } + if ( + typeof instance.componentWillReceiveProps === 'function' && + instance.componentWillReceiveProps.__suppressDeprecationWarning !== + true + ) { + willReceiveProps = 'componentWillReceiveProps'; + } else if ( + typeof instance.UNSAFE_componentWillReceiveProps === 'function' + ) { + willReceiveProps = 'UNSAFE_componentWillReceiveProps'; + } + if (typeof instance.componentWillUpdate === 'function') { + willUpdate = 'componentWillUpdate'; + } else if (typeof instance.UNSAFE_componentWillUpdate === 'function') { + willUpdate = 'UNSAFE_componentWillUpdate'; + } + if ( + willMount !== null || + willReceiveProps !== null || + willUpdate !== null + ) { const componentName = getName(type, instance) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { warning( @@ -383,9 +400,9 @@ class ReactShallowRenderer { 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, - definesWillMount ? '\n componentWillMount' : '', - definesWillReceiveProps ? '\n componentWillReceiveProps' : '', - definesWillUpdate ? '\n componentWillUpdate' : '', + willMount !== null ? `\n ${willMount}` : '', + willReceiveProps !== null ? `\n ${willReceiveProps}` : '', + willUpdate !== null ? `\n ${willUpdate}` : '', ); didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; } diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index e161291727d..287394290b3 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -139,7 +139,7 @@ describe('ReactShallowRenderer', () => { return null; } componentWillMount() {} - componentWillReceiveProps() {} + UNSAFE_componentWillReceiveProps() {} componentWillUpdate() {} render() { return null; @@ -151,7 +151,7 @@ describe('ReactShallowRenderer', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillReceiveProps\n' + + ' UNSAFE_componentWillReceiveProps\n' + ' componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', @@ -162,7 +162,7 @@ describe('ReactShallowRenderer', () => { static getDerivedStateFromProps() { return null; } - componentWillMount() {} + UNSAFE_componentWillMount() {} render() { return null; } @@ -172,7 +172,7 @@ describe('ReactShallowRenderer', () => { expect(() => shallowRenderer.render()).toWarnDev( 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + - ' componentWillMount\n\n' + + ' UNSAFE_componentWillMount\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', ); @@ -183,7 +183,7 @@ describe('ReactShallowRenderer', () => { return null; } componentWillMount() {} - componentWillUpdate() {} + UNSAFE_componentWillUpdate() {} render() { return null; } @@ -194,7 +194,7 @@ describe('ReactShallowRenderer', () => { 'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' + 'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' + ' componentWillMount\n' + - ' componentWillUpdate\n\n' + + ' UNSAFE_componentWillUpdate\n\n' + 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', ); From 9a41ac40c6b0716f731d8e14b1b8cbd8c14edf41 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 22 Mar 2018 11:03:57 -0700 Subject: [PATCH 5/5] Variable naming nit --- .../src/ReactFiberClassComponent.js | 32 ++++++++++--------- .../src/ReactShallowRenderer.js | 32 ++++++++++--------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 5240f16b736..8cf9c376769 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -437,37 +437,37 @@ export default function( // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. // Warn about these lifecycles if they are present. // Don't warn about react-lifecycles-compat polyfilled methods though. - let willMount = null; - let willReceiveProps = null; - let willUpdate = null; + let foundWillMountName = null; + let foundWillReceivePropsName = null; + let foundWillUpdateName = null; if ( typeof instance.componentWillMount === 'function' && instance.componentWillMount.__suppressDeprecationWarning !== true ) { - willMount = 'componentWillMount'; + foundWillMountName = 'componentWillMount'; } else if (typeof instance.UNSAFE_componentWillMount === 'function') { - willMount = 'UNSAFE_componentWillMount'; + foundWillMountName = 'UNSAFE_componentWillMount'; } if ( typeof instance.componentWillReceiveProps === 'function' && instance.componentWillReceiveProps.__suppressDeprecationWarning !== true ) { - willReceiveProps = 'componentWillReceiveProps'; + foundWillReceivePropsName = 'componentWillReceiveProps'; } else if ( typeof instance.UNSAFE_componentWillReceiveProps === 'function' ) { - willReceiveProps = 'UNSAFE_componentWillReceiveProps'; + foundWillReceivePropsName = 'UNSAFE_componentWillReceiveProps'; } if (typeof instance.componentWillUpdate === 'function') { - willUpdate = 'componentWillUpdate'; + foundWillUpdateName = 'componentWillUpdate'; } else if (typeof instance.UNSAFE_componentWillUpdate === 'function') { - willUpdate = 'UNSAFE_componentWillUpdate'; + foundWillUpdateName = 'UNSAFE_componentWillUpdate'; } if ( - willMount !== null || - willReceiveProps !== null || - willUpdate !== null + foundWillMountName !== null || + foundWillReceivePropsName !== null || + foundWillUpdateName !== null ) { const componentName = getComponentName(workInProgress) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { @@ -480,9 +480,11 @@ export default function( 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, - willMount !== null ? `\n ${willMount}` : '', - willReceiveProps !== null ? `\n ${willReceiveProps}` : '', - willUpdate !== null ? `\n ${willUpdate}` : '', + foundWillMountName !== null ? `\n ${foundWillMountName}` : '', + foundWillReceivePropsName !== null + ? `\n ${foundWillReceivePropsName}` + : '', + foundWillUpdateName !== null ? `\n ${foundWillUpdateName}` : '', ); didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; } diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 9dc7b5d2cbd..215f5b94d60 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -357,37 +357,37 @@ class ReactShallowRenderer { // If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called. // Warn about these lifecycles if they are present. // Don't warn about react-lifecycles-compat polyfilled methods though. - let willMount = null; - let willReceiveProps = null; - let willUpdate = null; + let foundWillMountName = null; + let foundWillReceivePropsName = null; + let foundWillUpdateName = null; if ( typeof instance.componentWillMount === 'function' && instance.componentWillMount.__suppressDeprecationWarning !== true ) { - willMount = 'componentWillMount'; + foundWillMountName = 'componentWillMount'; } else if (typeof instance.UNSAFE_componentWillMount === 'function') { - willMount = 'UNSAFE_componentWillMount'; + foundWillMountName = 'UNSAFE_componentWillMount'; } if ( typeof instance.componentWillReceiveProps === 'function' && instance.componentWillReceiveProps.__suppressDeprecationWarning !== true ) { - willReceiveProps = 'componentWillReceiveProps'; + foundWillReceivePropsName = 'componentWillReceiveProps'; } else if ( typeof instance.UNSAFE_componentWillReceiveProps === 'function' ) { - willReceiveProps = 'UNSAFE_componentWillReceiveProps'; + foundWillReceivePropsName = 'UNSAFE_componentWillReceiveProps'; } if (typeof instance.componentWillUpdate === 'function') { - willUpdate = 'componentWillUpdate'; + foundWillUpdateName = 'componentWillUpdate'; } else if (typeof instance.UNSAFE_componentWillUpdate === 'function') { - willUpdate = 'UNSAFE_componentWillUpdate'; + foundWillUpdateName = 'UNSAFE_componentWillUpdate'; } if ( - willMount !== null || - willReceiveProps !== null || - willUpdate !== null + foundWillMountName !== null || + foundWillReceivePropsName !== null || + foundWillUpdateName !== null ) { const componentName = getName(type, instance) || 'Component'; if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) { @@ -400,9 +400,11 @@ class ReactShallowRenderer { 'The above lifecycles should be removed. Learn more about this warning here:\n' + 'https://fb.me/react-async-component-lifecycle-hooks', componentName, - willMount !== null ? `\n ${willMount}` : '', - willReceiveProps !== null ? `\n ${willReceiveProps}` : '', - willUpdate !== null ? `\n ${willUpdate}` : '', + foundWillMountName !== null ? `\n ${foundWillMountName}` : '', + foundWillReceivePropsName !== null + ? `\n ${foundWillReceivePropsName}` + : '', + foundWillUpdateName !== null ? `\n ${foundWillUpdateName}` : '', ); didWarnAboutLegacyLifecyclesAndDerivedState[componentName] = true; }