From 4eed18dd72cb811de11bd34b8ff86e4d193c7d4e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 1 Feb 2018 11:15:57 -0800 Subject: [PATCH] Invoke both legacy and UNSAFE_ lifecycles when both are present (#12134) * Invoke both legacy and UNSAFE_ lifecycles when both are present This is to support edge cases with eg create-react-class where a mixin defines a legacy lifecycle but the component being created defines an UNSAFE one (or vice versa). I did not warn about this case because the warning would be a bit redundant with the deprecation warning which we will soon be enabling. I could be convinced to change my stance here though. * Added explicit function-type check to SS ReactPartialRenderer --- .../__tests__/ReactComponentLifeCycle-test.js | 42 +++++++++++++++++ .../ReactDOMServerLifecycles-test.js | 19 ++++++++ .../src/server/ReactPartialRenderer.js | 13 ++++-- .../src/ReactFiberClassComponent.js | 25 +++++----- .../src/ReactShallowRenderer.js | 12 +++-- .../__tests__/ReactShallowRenderer-test.js | 42 +++++++++++++++++ .../createReactClassIntegration-test.js | 46 +++++++++++++++++++ 7 files changed, 180 insertions(+), 19 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index b8f5300049..aa81546bd2 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -788,4 +788,46 @@ describe('ReactComponentLifeCycle', () => { // De-duped ReactDOM.render(, div); }); + + it('should invoke both deprecated and new lifecycles if both are present', () => { + const log = []; + + class MyComponent extends React.Component { + componentWillMount() { + log.push('componentWillMount'); + } + componentWillReceiveProps() { + log.push('componentWillReceiveProps'); + } + componentWillUpdate() { + log.push('componentWillUpdate'); + } + UNSAFE_componentWillMount() { + log.push('UNSAFE_componentWillMount'); + } + UNSAFE_componentWillReceiveProps() { + log.push('UNSAFE_componentWillReceiveProps'); + } + UNSAFE_componentWillUpdate() { + log.push('UNSAFE_componentWillUpdate'); + } + render() { + return null; + } + } + + const div = document.createElement('div'); + ReactDOM.render(, div); + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']); + + log.length = 0; + + ReactDOM.render(, div); + expect(log).toEqual([ + 'componentWillReceiveProps', + 'UNSAFE_componentWillReceiveProps', + 'componentWillUpdate', + 'UNSAFE_componentWillUpdate', + ]); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js index 10171a0e06..862bc27e53 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js @@ -207,4 +207,23 @@ describe('ReactDOMServerLifecycles', () => { // De-duped ReactDOMServer.renderToString(); }); + + it('should invoke both deprecated and new lifecycles if both are present', () => { + const log = []; + + class Component extends React.Component { + componentWillMount() { + log.push('componentWillMount'); + } + UNSAFE_componentWillMount() { + log.push('UNSAFE_componentWillMount'); + } + render() { + return null; + } + } + + ReactDOMServer.renderToString(); + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']); + }); }); diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index c5cdc2679d..656f20fc71 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -511,8 +511,11 @@ function resolve( if (initialState === undefined) { inst.state = initialState = null; } - if (inst.UNSAFE_componentWillMount || inst.componentWillMount) { - if (inst.componentWillMount) { + if ( + typeof inst.UNSAFE_componentWillMount === 'function' || + typeof inst.componentWillMount === 'function' + ) { + if (typeof inst.componentWillMount === 'function') { if (__DEV__) { if ( warnAboutDeprecatedLifecycles && @@ -542,7 +545,11 @@ function resolve( if (typeof Component.getDerivedStateFromProps !== 'function') { inst.componentWillMount(); } - } else if (typeof Component.getDerivedStateFromProps !== 'function') { + } + if ( + typeof inst.UNSAFE_componentWillMount === 'function' && + typeof Component.getDerivedStateFromProps !== 'function' + ) { // In order to support react-lifecycles-compat polyfilled components, // Unsafe lifecycles should not be invoked for any component with the new gDSFP. inst.UNSAFE_componentWillMount(); diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 3752b5c368..9e0dd98575 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -459,7 +459,8 @@ export default function( if (typeof instance.componentWillMount === 'function') { instance.componentWillMount(); - } else { + } + if (typeof instance.UNSAFE_componentWillMount === 'function') { instance.UNSAFE_componentWillMount(); } @@ -486,15 +487,14 @@ export default function( newContext, ) { const oldState = instance.state; + startPhaseTimer(workInProgress, 'componentWillReceiveProps'); if (typeof instance.componentWillReceiveProps === 'function') { - startPhaseTimer(workInProgress, 'componentWillReceiveProps'); instance.componentWillReceiveProps(newProps, newContext); - stopPhaseTimer(); - } else { - startPhaseTimer(workInProgress, 'componentWillReceiveProps'); - instance.UNSAFE_componentWillReceiveProps(newProps, newContext); - stopPhaseTimer(); } + if (typeof instance.UNSAFE_componentWillReceiveProps === 'function') { + instance.UNSAFE_componentWillReceiveProps(newProps, newContext); + } + stopPhaseTimer(); if (instance.state !== oldState) { if (__DEV__) { @@ -861,15 +861,14 @@ export default function( typeof instance.componentWillUpdate === 'function') && typeof workInProgress.type.getDerivedStateFromProps !== 'function' ) { + startPhaseTimer(workInProgress, 'componentWillUpdate'); if (typeof instance.componentWillUpdate === 'function') { - startPhaseTimer(workInProgress, 'componentWillUpdate'); instance.componentWillUpdate(newProps, newState, newContext); - stopPhaseTimer(); - } else { - startPhaseTimer(workInProgress, 'componentWillUpdate'); - instance.UNSAFE_componentWillUpdate(newProps, newState, newContext); - stopPhaseTimer(); } + if (typeof instance.UNSAFE_componentWillUpdate === 'function') { + instance.UNSAFE_componentWillUpdate(newProps, newState, newContext); + } + stopPhaseTimer(); } if (typeof instance.componentDidUpdate === 'function') { workInProgress.effectTag |= Update; diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 01fd15d28c..46802a0d0a 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -209,7 +209,11 @@ class ReactShallowRenderer { if (typeof element.type.getDerivedStateFromProps !== 'function') { this._instance.componentWillMount(); } - } else if (typeof element.type.getDerivedStateFromProps !== 'function') { + } + if ( + typeof this._instance.UNSAFE_componentWillMount === 'function' && + typeof element.type.getDerivedStateFromProps !== 'function' + ) { // In order to support react-lifecycles-compat polyfilled components, // Unsafe lifecycles should not be invoked for any component with the new gDSFP. this._instance.UNSAFE_componentWillMount(); @@ -259,7 +263,8 @@ class ReactShallowRenderer { if (typeof element.type.getDerivedStateFromProps !== 'function') { this._instance.componentWillReceiveProps(props, context); } - } else if ( + } + if ( typeof this._instance.UNSAFE_componentWillReceiveProps === 'function' && typeof element.type.getDerivedStateFromProps !== 'function' ) { @@ -316,7 +321,8 @@ class ReactShallowRenderer { if (typeof type.getDerivedStateFromProps !== 'function') { this._instance.componentWillUpdate(props, state, context); } - } else if ( + } + if ( typeof this._instance.UNSAFE_componentWillUpdate === 'function' && typeof type.getDerivedStateFromProps !== 'function' ) { diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 240129421e..377ba5f671 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -1235,4 +1235,46 @@ describe('ReactShallowRenderer', () => { // De-duped shallowRenderer.render(); }); + + it('should invoke both deprecated and new lifecycles if both are present', () => { + const log = []; + + class Component extends React.Component { + componentWillMount() { + log.push('componentWillMount'); + } + componentWillReceiveProps() { + log.push('componentWillReceiveProps'); + } + componentWillUpdate() { + log.push('componentWillUpdate'); + } + UNSAFE_componentWillMount() { + log.push('UNSAFE_componentWillMount'); + } + UNSAFE_componentWillReceiveProps() { + log.push('UNSAFE_componentWillReceiveProps'); + } + UNSAFE_componentWillUpdate() { + log.push('UNSAFE_componentWillUpdate'); + } + render() { + return null; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']); + + log.length = 0; + + shallowRenderer.render(); + expect(log).toEqual([ + 'componentWillReceiveProps', + 'UNSAFE_componentWillReceiveProps', + 'componentWillUpdate', + 'UNSAFE_componentWillUpdate', + ]); + }); }); diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index c8e3670afc..c567b4fd85 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -478,4 +478,50 @@ describe('create-react-class-integration', () => { }).toWarnDev('Defines both componentWillReceiveProps'); ReactDOM.render(, document.createElement('div')); }); + + it('should invoke both deprecated and new lifecycles if both are present', () => { + const log = []; + + const Component = createReactClass({ + mixins: [ + { + componentWillMount: function() { + log.push('componentWillMount'); + }, + componentWillReceiveProps: function() { + log.push('componentWillReceiveProps'); + }, + componentWillUpdate: function() { + log.push('componentWillUpdate'); + }, + }, + ], + UNSAFE_componentWillMount: function() { + log.push('UNSAFE_componentWillMount'); + }, + UNSAFE_componentWillReceiveProps: function() { + log.push('UNSAFE_componentWillReceiveProps'); + }, + UNSAFE_componentWillUpdate: function() { + log.push('UNSAFE_componentWillUpdate'); + }, + render: function() { + return null; + }, + }); + + const div = document.createElement('div'); + ReactDOM.render(, div); + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']); + + log.length = 0; + + ReactDOM.render(, div); + expect(log).toEqual([ + 'componentWillReceiveProps', + 'UNSAFE_componentWillReceiveProps', + 'componentWillUpdate', + 'UNSAFE_componentWillUpdate', + ]); + }); });