@samitouri / QOS-React / commits / 4eed18dd72

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

Brian Vaughn committed Feb 1, 2018 at 11:15 UTC 4eed18dd72cb811de11bd34b8ff86e4d193c7d4e
7 files changed +178 -17
packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
+42
@@ -788,4 +788,46 @@ describe('ReactComponentLifeCycle', () => {
788 // De-duped
789 ReactDOM.render(<MyComponent />, div);
790 });
791 +
792 + it('should invoke both deprecated and new lifecycles if both are present', () => {
793 + const log = [];
794 +
795 + class MyComponent extends React.Component {
796 + componentWillMount() {
797 + log.push('componentWillMount');
798 + }
799 + componentWillReceiveProps() {
800 + log.push('componentWillReceiveProps');
801 + }
802 + componentWillUpdate() {
803 + log.push('componentWillUpdate');
804 + }
805 + UNSAFE_componentWillMount() {
806 + log.push('UNSAFE_componentWillMount');
807 + }
808 + UNSAFE_componentWillReceiveProps() {
809 + log.push('UNSAFE_componentWillReceiveProps');
810 + }
811 + UNSAFE_componentWillUpdate() {
812 + log.push('UNSAFE_componentWillUpdate');
813 + }
814 + render() {
815 + return null;
816 + }
817 + }
818 +
819 + const div = document.createElement('div');
820 + ReactDOM.render(<MyComponent foo="bar" />, div);
821 + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
822 +
823 + log.length = 0;
824 +
825 + ReactDOM.render(<MyComponent foo="baz" />, div);
826 + expect(log).toEqual([
827 + 'componentWillReceiveProps',
828 + 'UNSAFE_componentWillReceiveProps',
829 + 'componentWillUpdate',
830 + 'UNSAFE_componentWillUpdate',
831 + ]);
832 + });
833 });
packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js
+19
@@ -207,4 +207,23 @@ describe('ReactDOMServerLifecycles', () => {
207 // De-duped
208 ReactDOMServer.renderToString(<Component />);
209 });
210 +
211 + it('should invoke both deprecated and new lifecycles if both are present', () => {
212 + const log = [];
213 +
214 + class Component extends React.Component {
215 + componentWillMount() {
216 + log.push('componentWillMount');
217 + }
218 + UNSAFE_componentWillMount() {
219 + log.push('UNSAFE_componentWillMount');
220 + }
221 + render() {
222 + return null;
223 + }
224 + }
225 +
226 + ReactDOMServer.renderToString(<Component />);
227 + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
228 + });
229 });
packages/react-dom/src/server/ReactPartialRenderer.js
+10 -3
@@ -511,8 +511,11 @@ function resolve(
511 if (initialState === undefined) {
512 inst.state = initialState = null;
513 }
514 - if (inst.UNSAFE_componentWillMount || inst.componentWillMount) {
515 - if (inst.componentWillMount) {
514 + if (
515 + typeof inst.UNSAFE_componentWillMount === 'function' ||
516 + typeof inst.componentWillMount === 'function'
517 + ) {
518 + if (typeof inst.componentWillMount === 'function') {
519 if (__DEV__) {
520 if (
521 warnAboutDeprecatedLifecycles &&
@@ -542,7 +545,11 @@ function resolve(
545 if (typeof Component.getDerivedStateFromProps !== 'function') {
546 inst.componentWillMount();
547 }
545 - } else if (typeof Component.getDerivedStateFromProps !== 'function') {
548 + }
549 + if (
550 + typeof inst.UNSAFE_componentWillMount === 'function' &&
551 + typeof Component.getDerivedStateFromProps !== 'function'
552 + ) {
553 // In order to support react-lifecycles-compat polyfilled components,
554 // Unsafe lifecycles should not be invoked for any component with the new gDSFP.
555 inst.UNSAFE_componentWillMount();
packages/react-reconciler/src/ReactFiberClassComponent.js
+10 -11
@@ -459,7 +459,8 @@ export default function(
459
460 if (typeof instance.componentWillMount === 'function') {
461 instance.componentWillMount();
462 - } else {
462 + }
463 + if (typeof instance.UNSAFE_componentWillMount === 'function') {
464 instance.UNSAFE_componentWillMount();
465 }
466
@@ -486,15 +487,14 @@ export default function(
487 newContext,
488 ) {
489 const oldState = instance.state;
490 + startPhaseTimer(workInProgress, 'componentWillReceiveProps');
491 if (typeof instance.componentWillReceiveProps === 'function') {
490 - startPhaseTimer(workInProgress, 'componentWillReceiveProps');
492 instance.componentWillReceiveProps(newProps, newContext);
492 - stopPhaseTimer();
493 - } else {
494 - startPhaseTimer(workInProgress, 'componentWillReceiveProps');
493 + }
494 + if (typeof instance.UNSAFE_componentWillReceiveProps === 'function') {
495 instance.UNSAFE_componentWillReceiveProps(newProps, newContext);
496 - stopPhaseTimer();
496 }
497 + stopPhaseTimer();
498
499 if (instance.state !== oldState) {
500 if (__DEV__) {
@@ -861,15 +861,14 @@ export default function(
861 typeof instance.componentWillUpdate === 'function') &&
862 typeof workInProgress.type.getDerivedStateFromProps !== 'function'
863 ) {
864 + startPhaseTimer(workInProgress, 'componentWillUpdate');
865 if (typeof instance.componentWillUpdate === 'function') {
865 - startPhaseTimer(workInProgress, 'componentWillUpdate');
866 instance.componentWillUpdate(newProps, newState, newContext);
867 - stopPhaseTimer();
868 - } else {
869 - startPhaseTimer(workInProgress, 'componentWillUpdate');
867 + }
868 + if (typeof instance.UNSAFE_componentWillUpdate === 'function') {
869 instance.UNSAFE_componentWillUpdate(newProps, newState, newContext);
871 - stopPhaseTimer();
870 }
871 + stopPhaseTimer();
872 }
873 if (typeof instance.componentDidUpdate === 'function') {
874 workInProgress.effectTag |= Update;
packages/react-test-renderer/src/ReactShallowRenderer.js
+9 -3
@@ -209,7 +209,11 @@ class ReactShallowRenderer {
209 if (typeof element.type.getDerivedStateFromProps !== 'function') {
210 this._instance.componentWillMount();
211 }
212 - } else if (typeof element.type.getDerivedStateFromProps !== 'function') {
212 + }
213 + if (
214 + typeof this._instance.UNSAFE_componentWillMount === 'function' &&
215 + typeof element.type.getDerivedStateFromProps !== 'function'
216 + ) {
217 // In order to support react-lifecycles-compat polyfilled components,
218 // Unsafe lifecycles should not be invoked for any component with the new gDSFP.
219 this._instance.UNSAFE_componentWillMount();
@@ -259,7 +263,8 @@ class ReactShallowRenderer {
263 if (typeof element.type.getDerivedStateFromProps !== 'function') {
264 this._instance.componentWillReceiveProps(props, context);
265 }
262 - } else if (
266 + }
267 + if (
268 typeof this._instance.UNSAFE_componentWillReceiveProps === 'function' &&
269 typeof element.type.getDerivedStateFromProps !== 'function'
270 ) {
@@ -316,7 +321,8 @@ class ReactShallowRenderer {
321 if (typeof type.getDerivedStateFromProps !== 'function') {
322 this._instance.componentWillUpdate(props, state, context);
323 }
319 - } else if (
324 + }
325 + if (
326 typeof this._instance.UNSAFE_componentWillUpdate === 'function' &&
327 typeof type.getDerivedStateFromProps !== 'function'
328 ) {
packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js
+42
@@ -1235,4 +1235,46 @@ describe('ReactShallowRenderer', () => {
1235 // De-duped
1236 shallowRenderer.render(<Component />);
1237 });
1238 +
1239 + it('should invoke both deprecated and new lifecycles if both are present', () => {
1240 + const log = [];
1241 +
1242 + class Component extends React.Component {
1243 + componentWillMount() {
1244 + log.push('componentWillMount');
1245 + }
1246 + componentWillReceiveProps() {
1247 + log.push('componentWillReceiveProps');
1248 + }
1249 + componentWillUpdate() {
1250 + log.push('componentWillUpdate');
1251 + }
1252 + UNSAFE_componentWillMount() {
1253 + log.push('UNSAFE_componentWillMount');
1254 + }
1255 + UNSAFE_componentWillReceiveProps() {
1256 + log.push('UNSAFE_componentWillReceiveProps');
1257 + }
1258 + UNSAFE_componentWillUpdate() {
1259 + log.push('UNSAFE_componentWillUpdate');
1260 + }
1261 + render() {
1262 + return null;
1263 + }
1264 + }
1265 +
1266 + const shallowRenderer = createRenderer();
1267 + shallowRenderer.render(<Component foo="bar" />);
1268 + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
1269 +
1270 + log.length = 0;
1271 +
1272 + shallowRenderer.render(<Component foo="baz" />);
1273 + expect(log).toEqual([
1274 + 'componentWillReceiveProps',
1275 + 'UNSAFE_componentWillReceiveProps',
1276 + 'componentWillUpdate',
1277 + 'UNSAFE_componentWillUpdate',
1278 + ]);
1279 + });
1280 });
packages/react/src/__tests__/createReactClassIntegration-test.js
+46
@@ -478,4 +478,50 @@ describe('create-react-class-integration', () => {
478 }).toWarnDev('Defines both componentWillReceiveProps');
479 ReactDOM.render(<Component foo={1} />, document.createElement('div'));
480 });
481 +
482 + it('should invoke both deprecated and new lifecycles if both are present', () => {
483 + const log = [];
484 +
485 + const Component = createReactClass({
486 + mixins: [
487 + {
488 + componentWillMount: function() {
489 + log.push('componentWillMount');
490 + },
491 + componentWillReceiveProps: function() {
492 + log.push('componentWillReceiveProps');
493 + },
494 + componentWillUpdate: function() {
495 + log.push('componentWillUpdate');
496 + },
497 + },
498 + ],
499 + UNSAFE_componentWillMount: function() {
500 + log.push('UNSAFE_componentWillMount');
501 + },
502 + UNSAFE_componentWillReceiveProps: function() {
503 + log.push('UNSAFE_componentWillReceiveProps');
504 + },
505 + UNSAFE_componentWillUpdate: function() {
506 + log.push('UNSAFE_componentWillUpdate');
507 + },
508 + render: function() {
509 + return null;
510 + },
511 + });
512 +
513 + const div = document.createElement('div');
514 + ReactDOM.render(<Component foo="bar" />, div);
515 + expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
516 +
517 + log.length = 0;
518 +
519 + ReactDOM.render(<Component foo="baz" />, div);
520 + expect(log).toEqual([
521 + 'componentWillReceiveProps',
522 + 'UNSAFE_componentWillReceiveProps',
523 + 'componentWillUpdate',
524 + 'UNSAFE_componentWillUpdate',
525 + ]);
526 + });
527 });