Skip to content

Commit 55e75da

Browse files
bvaughnbvaughn
committed
Don't invoke legacy lifecycles if getSnapshotBeforeUpdate() is defined. DEV warn about this.
1 parent 3b1d23c commit 55e75da

File tree

5 files changed

+276
-82
lines changed

5 files changed

+276
-82
lines changed

‎packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js‎

Lines changed: 118 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -673,10 +673,38 @@ describe('ReactComponentLifeCycle', () => {
673673

674674
const container = document.createElement('div');
675675
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
676-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.',
676+
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
677677
);
678678
});
679679

680+
it('should not invoke deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => {
681+
class Component extends React.Component {
682+
state = {};
683+
getSnapshotBeforeUpdate() {
684+
return null;
685+
}
686+
componentWillMount() {
687+
throw Error('unexpected');
688+
}
689+
componentWillReceiveProps() {
690+
throw Error('unexpected');
691+
}
692+
componentWillUpdate() {
693+
throw Error('unexpected');
694+
}
695+
componentDidUpdate() {}
696+
render() {
697+
return null;
698+
}
699+
}
700+
701+
const container = document.createElement('div');
702+
expect(() => ReactDOM.render(<Component value={1} />, container)).toWarnDev(
703+
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
704+
);
705+
ReactDOM.render(<Component value={2} />, container);
706+
});
707+
680708
it('should not invoke new unsafe lifecycles (cWM/cWRP/cWU) if static gDSFP is present', () => {
681709
class Component extends React.Component {
682710
state = {};
@@ -698,9 +726,10 @@ describe('ReactComponentLifeCycle', () => {
698726
}
699727

700728
const container = document.createElement('div');
701-
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
702-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.',
729+
expect(() => ReactDOM.render(<Component value={1} />, container)).toWarnDev(
730+
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
703731
);
732+
ReactDOM.render(<Component value={2} />, container);
704733
});
705734

706735
it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new static gDSFP is present', () => {
@@ -720,7 +749,7 @@ describe('ReactComponentLifeCycle', () => {
720749
}
721750

722751
expect(() => ReactDOM.render(<AllLegacyLifecycles />, container)).toWarnDev(
723-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' +
752+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
724753
'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
725754
' componentWillMount\n' +
726755
' UNSAFE_componentWillReceiveProps\n' +
@@ -741,7 +770,7 @@ describe('ReactComponentLifeCycle', () => {
741770
}
742771

743772
expect(() => ReactDOM.render(<WillMount />, container)).toWarnDev(
744-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' +
773+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
745774
'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
746775
' UNSAFE_componentWillMount\n\n' +
747776
'The above lifecycles should be removed. Learn more about this warning here:\n' +
@@ -761,7 +790,7 @@ describe('ReactComponentLifeCycle', () => {
761790
}
762791

763792
expect(() => ReactDOM.render(<WillMountAndUpdate />, container)).toWarnDev(
764-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' +
793+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
765794
'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
766795
' componentWillMount\n' +
767796
' UNSAFE_componentWillUpdate\n\n' +
@@ -781,14 +810,96 @@ describe('ReactComponentLifeCycle', () => {
781810
}
782811

783812
expect(() => ReactDOM.render(<WillReceiveProps />, container)).toWarnDev(
784-
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.\n\n' +
813+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
785814
'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
786815
' componentWillReceiveProps\n\n' +
787816
'The above lifecycles should be removed. Learn more about this warning here:\n' +
788817
'https://fb.me/react-async-component-lifecycle-hooks',
789818
);
790819
});
791820

821+
it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => {
822+
const container = document.createElement('div');
823+
824+
class AllLegacyLifecycles extends React.Component {
825+
state = {};
826+
getSnapshotBeforeUpdate() {}
827+
componentWillMount() {}
828+
UNSAFE_componentWillReceiveProps() {}
829+
componentWillUpdate() {}
830+
componentDidUpdate() {}
831+
render() {
832+
return null;
833+
}
834+
}
835+
836+
expect(() => ReactDOM.render(<AllLegacyLifecycles />, container)).toWarnDev(
837+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
838+
'AllLegacyLifecycles uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
839+
' componentWillMount\n' +
840+
' UNSAFE_componentWillReceiveProps\n' +
841+
' componentWillUpdate\n\n' +
842+
'The above lifecycles should be removed. Learn more about this warning here:\n' +
843+
'https://fb.me/react-async-component-lifecycle-hooks',
844+
);
845+
846+
class WillMount extends React.Component {
847+
state = {};
848+
getSnapshotBeforeUpdate() {}
849+
UNSAFE_componentWillMount() {}
850+
componentDidUpdate() {}
851+
render() {
852+
return null;
853+
}
854+
}
855+
856+
expect(() => ReactDOM.render(<WillMount />, container)).toWarnDev(
857+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
858+
'WillMount uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
859+
' UNSAFE_componentWillMount\n\n' +
860+
'The above lifecycles should be removed. Learn more about this warning here:\n' +
861+
'https://fb.me/react-async-component-lifecycle-hooks',
862+
);
863+
864+
class WillMountAndUpdate extends React.Component {
865+
state = {};
866+
getSnapshotBeforeUpdate() {}
867+
componentWillMount() {}
868+
UNSAFE_componentWillUpdate() {}
869+
componentDidUpdate() {}
870+
render() {
871+
return null;
872+
}
873+
}
874+
875+
expect(() => ReactDOM.render(<WillMountAndUpdate />, container)).toWarnDev(
876+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
877+
'WillMountAndUpdate uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
878+
' componentWillMount\n' +
879+
' UNSAFE_componentWillUpdate\n\n' +
880+
'The above lifecycles should be removed. Learn more about this warning here:\n' +
881+
'https://fb.me/react-async-component-lifecycle-hooks',
882+
);
883+
884+
class WillReceiveProps extends React.Component {
885+
state = {};
886+
getSnapshotBeforeUpdate() {}
887+
componentWillReceiveProps() {}
888+
componentDidUpdate() {}
889+
render() {
890+
return null;
891+
}
892+
}
893+
894+
expect(() => ReactDOM.render(<WillReceiveProps />, container)).toWarnDev(
895+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
896+
'WillReceiveProps uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
897+
' componentWillReceiveProps\n\n' +
898+
'The above lifecycles should be removed. Learn more about this warning here:\n' +
899+
'https://fb.me/react-async-component-lifecycle-hooks',
900+
);
901+
});
902+
792903
it('calls effects on module-pattern component', function() {
793904
const log = [];
794905

@@ -1037,9 +1148,6 @@ describe('ReactComponentLifeCycle', () => {
10371148

10381149
ReactDOM.render(<div />, div);
10391150
expect(log).toEqual([]);
1040-
1041-
// De-duped
1042-
ReactDOM.render(<MyComponent />, div);
10431151
});
10441152

10451153
it('should warn if getSnapshotBeforeUpdate returns undefined', () => {

‎packages/react-reconciler/src/ReactFiberClassComponent.js‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ export default function(
370370
if (
371371
typeof instance.getSnapshotBeforeUpdate === 'function' &&
372372
typeof instance.componentDidUpdate !== 'function' &&
373+
typeof instance.UNSAFE_componentDidUpdate !== 'function' &&
373374
!didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.has(type)
374375
) {
375376
didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.add(type);
@@ -452,24 +453,30 @@ export default function(
452453
adoptClassInstance(workInProgress, instance);
453454

454455
if (__DEV__) {
455-
if (typeof ctor.getDerivedStateFromProps === 'function') {
456-
if (state === null) {
457-
const componentName = getComponentName(workInProgress) || 'Component';
458-
if (!didWarnAboutUninitializedState[componentName]) {
459-
warning(
460-
false,
461-
'%s: Did not properly initialize state during construction. ' +
462-
'Expected state to be an object, but it was %s.',
463-
componentName,
464-
instance.state === null ? 'null' : 'undefined',
465-
);
466-
didWarnAboutUninitializedState[componentName] = true;
467-
}
456+
if (
457+
typeof ctor.getDerivedStateFromProps === 'function' &&
458+
state === null
459+
) {
460+
const componentName = getComponentName(workInProgress) || 'Component';
461+
if (!didWarnAboutUninitializedState[componentName]) {
462+
warning(
463+
false,
464+
'%s: Did not properly initialize state during construction. ' +
465+
'Expected state to be an object, but it was %s.',
466+
componentName,
467+
instance.state === null ? 'null' : 'undefined',
468+
);
469+
didWarnAboutUninitializedState[componentName] = true;
468470
}
471+
}
469472

470-
// If getDerivedStateFromProps() is defined, "unsafe" lifecycles won't be called.
471-
// Warn about these lifecycles if they are present.
472-
// Don't warn about react-lifecycles-compat polyfilled methods though.
473+
// If new component APIs are defined, "unsafe" lifecycles won't be called.
474+
// Warn about these lifecycles if they are present.
475+
// Don't warn about react-lifecycles-compat polyfilled methods though.
476+
if (
477+
typeof ctor.getDerivedStateFromProps === 'function' ||
478+
typeof instance.getSnapshotBeforeUpdate === 'function'
479+
) {
473480
let foundWillMountName = null;
474481
let foundWillReceivePropsName = null;
475482
let foundWillUpdateName = null;
@@ -503,16 +510,18 @@ export default function(
503510
foundWillUpdateName !== null
504511
) {
505512
const componentName = getComponentName(workInProgress) || 'Component';
513+
const newApiName = ctor.getDerivedStateFromProps
514+
? 'getDerivedStateFromProps()'
515+
: 'getSnapshotBeforeUpdate()';
506516
if (!didWarnAboutLegacyLifecyclesAndDerivedState[componentName]) {
507517
warning(
508518
false,
509-
'Unsafe legacy lifecycles will not be called for components using ' +
510-
'the new getDerivedStateFromProps() API.\n\n' +
511-
'%s uses getDerivedStateFromProps() but also contains the following legacy lifecycles:' +
512-
'%s%s%s\n\n' +
519+
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
520+
'%s uses %s but also contains the following legacy lifecycles:%s%s%s\n\n' +
513521
'The above lifecycles should be removed. Learn more about this warning here:\n' +
514522
'https://fb.me/react-async-component-lifecycle-hooks',
515523
componentName,
524+
newApiName,
516525
foundWillMountName !== null ? `\n ${foundWillMountName}` : '',
517526
foundWillReceivePropsName !== null
518527
? `\n ${foundWillReceivePropsName}`
@@ -696,11 +705,12 @@ export default function(
696705
}
697706

698707
// In order to support react-lifecycles-compat polyfilled components,
699-
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
708+
// Unsafe lifecycles should not be invoked for components using the new APIs.
700709
if (
710+
typeof ctor.getDerivedStateFromProps !== 'function' &&
711+
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
701712
(typeof instance.UNSAFE_componentWillMount === 'function' ||
702-
typeof instance.componentWillMount === 'function') &&
703-
typeof ctor.getDerivedStateFromProps !== 'function'
713+
typeof instance.componentWillMount === 'function')
704714
) {
705715
callComponentWillMount(workInProgress, instance);
706716
// If we had additional state updates during this life-cycle, let's
@@ -741,11 +751,12 @@ export default function(
741751
// during componentDidUpdate we pass the "current" props.
742752

743753
// In order to support react-lifecycles-compat polyfilled components,
744-
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
754+
// Unsafe lifecycles should not be invoked for components using the new APIs.
745755
if (
756+
typeof ctor.getDerivedStateFromProps !== 'function' &&
757+
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
746758
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
747-
typeof instance.componentWillReceiveProps === 'function') &&
748-
typeof ctor.getDerivedStateFromProps !== 'function'
759+
typeof instance.componentWillReceiveProps === 'function')
749760
) {
750761
if (oldProps !== newProps || oldContext !== newContext) {
751762
callComponentWillReceiveProps(
@@ -852,11 +863,12 @@ export default function(
852863

853864
if (shouldUpdate) {
854865
// In order to support react-lifecycles-compat polyfilled components,
855-
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
866+
// Unsafe lifecycles should not be invoked for components using the new APIs.
856867
if (
868+
typeof ctor.getDerivedStateFromProps !== 'function' &&
869+
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
857870
(typeof instance.UNSAFE_componentWillMount === 'function' ||
858-
typeof instance.componentWillMount === 'function') &&
859-
typeof ctor.getDerivedStateFromProps !== 'function'
871+
typeof instance.componentWillMount === 'function')
860872
) {
861873
startPhaseTimer(workInProgress, 'componentWillMount');
862874
if (typeof instance.componentWillMount === 'function') {
@@ -913,11 +925,12 @@ export default function(
913925
// during componentDidUpdate we pass the "current" props.
914926

915927
// In order to support react-lifecycles-compat polyfilled components,
916-
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
928+
// Unsafe lifecycles should not be invoked for components using the new APIs.
917929
if (
930+
typeof ctor.getDerivedStateFromProps !== 'function' &&
931+
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
918932
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
919-
typeof instance.componentWillReceiveProps === 'function') &&
920-
typeof ctor.getDerivedStateFromProps !== 'function'
933+
typeof instance.componentWillReceiveProps === 'function')
921934
) {
922935
if (oldProps !== newProps || oldContext !== newContext) {
923936
callComponentWillReceiveProps(
@@ -1038,11 +1051,12 @@ export default function(
10381051

10391052
if (shouldUpdate) {
10401053
// In order to support react-lifecycles-compat polyfilled components,
1041-
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
1054+
// Unsafe lifecycles should not be invoked for components using the new APIs.
10421055
if (
1056+
typeof ctor.getDerivedStateFromProps !== 'function' &&
1057+
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
10431058
(typeof instance.UNSAFE_componentWillUpdate === 'function' ||
1044-
typeof instance.componentWillUpdate === 'function') &&
1045-
typeof ctor.getDerivedStateFromProps !== 'function'
1059+
typeof instance.componentWillUpdate === 'function')
10461060
) {
10471061
startPhaseTimer(workInProgress, 'componentWillUpdate');
10481062
if (typeof instance.componentWillUpdate === 'function') {

0 commit comments

Comments
 (0)