Skip to content

Commit

Permalink
Don't invoke legacy lifecycles if getSnapshotBeforeUpdate() is define…
Browse files Browse the repository at this point in the history 
…d. DEV warn about this.
  • Loading branch information
@bvaughn
bvaughn committed Mar 22, 2018
1 parent 3b1d23c commit 55e75da
Show file tree
Hide file tree
Showing 5 changed files with 276 additions and 82 deletions.
128 changes: 118 additions & 10 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -673,10 +673,38 @@ describe('ReactComponentLifeCycle', () => {

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

it('should not invoke deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => {
class Component extends React.Component {
state = {};
getSnapshotBeforeUpdate() {
return null;
}
componentWillMount() {
throw Error('unexpected');
}
componentWillReceiveProps() {
throw Error('unexpected');
}
componentWillUpdate() {
throw Error('unexpected');
}
componentDidUpdate() {}
render() {
return null;
}
}

const container = document.createElement('div');
expect(() => ReactDOM.render(<Component value={1} />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
);
ReactDOM.render(<Component value={2} />, container);
});

it('should not invoke new unsafe lifecycles (cWM/cWRP/cWU) if static gDSFP is present', () => {
class Component extends React.Component {
state = {};
Expand All @@ -698,9 +726,10 @@ describe('ReactComponentLifeCycle', () => {
}

const container = document.createElement('div');
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API.',
expect(() => ReactDOM.render(<Component value={1} />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.',
);
ReactDOM.render(<Component value={2} />, container);
});

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

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

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

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

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

it('should warn about deprecated lifecycles (cWM/cWRP/cWU) if new getSnapshotBeforeUpdate is present', () => {
const container = document.createElement('div');

class AllLegacyLifecycles extends React.Component {
state = {};
getSnapshotBeforeUpdate() {}
componentWillMount() {}
UNSAFE_componentWillReceiveProps() {}
componentWillUpdate() {}
componentDidUpdate() {}
render() {
return null;
}
}

expect(() => ReactDOM.render(<AllLegacyLifecycles />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
'AllLegacyLifecycles uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
' componentWillMount\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',
);

class WillMount extends React.Component {
state = {};
getSnapshotBeforeUpdate() {}
UNSAFE_componentWillMount() {}
componentDidUpdate() {}
render() {
return null;
}
}

expect(() => ReactDOM.render(<WillMount />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
'WillMount uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\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',
);

class WillMountAndUpdate extends React.Component {
state = {};
getSnapshotBeforeUpdate() {}
componentWillMount() {}
UNSAFE_componentWillUpdate() {}
componentDidUpdate() {}
render() {
return null;
}
}

expect(() => ReactDOM.render(<WillMountAndUpdate />, container)).toWarnDev(
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
'WillMountAndUpdate uses getSnapshotBeforeUpdate() but also contains the following legacy lifecycles:\n' +
' componentWillMount\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',
);

class WillReceiveProps extends React.Component {
state = {};
getSnapshotBeforeUpdate() {}
componentWillReceiveProps() {}
componentDidUpdate() {}
render() {
return null;
}
}

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

it('calls effects on module-pattern component', function() {
const log = [];

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

ReactDOM.render(<div />, div);
expect(log).toEqual([]);

// De-duped
ReactDOM.render(<MyComponent />, div);
});

it('should warn if getSnapshotBeforeUpdate returns undefined', () => {
Expand Down
84 changes: 49 additions & 35 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ export default function(
if (
typeof instance.getSnapshotBeforeUpdate === 'function' &&
typeof instance.componentDidUpdate !== 'function' &&
typeof instance.UNSAFE_componentDidUpdate !== 'function' &&
!didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.has(type)
) {
didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.add(type);
Expand Down Expand Up @@ -452,24 +453,30 @@ export default function(
adoptClassInstance(workInProgress, instance);

if (__DEV__) {
if (typeof ctor.getDerivedStateFromProps === 'function') {
if (state === null) {
const componentName = getComponentName(workInProgress) || 'Component';
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' &&
state === null
) {
const componentName = getComponentName(workInProgress) || 'Component';
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.
// If new component APIs are 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.
if (
typeof ctor.getDerivedStateFromProps === 'function' ||
typeof instance.getSnapshotBeforeUpdate === 'function'
) {
let foundWillMountName = null;
let foundWillReceivePropsName = null;
let foundWillUpdateName = null;
Expand Down Expand Up @@ -503,16 +510,18 @@ export default function(
foundWillUpdateName !== null
) {
const componentName = getComponentName(workInProgress) || 'Component';
const newApiName = ctor.getDerivedStateFromProps
? 'getDerivedStateFromProps()'
: 'getSnapshotBeforeUpdate()';
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\n\n' +
'Unsafe legacy lifecycles will not be called for components using new component APIs.\n\n' +
'%s uses %s but also contains the following legacy lifecycles:%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,
newApiName,
foundWillMountName !== null ? `\n ${foundWillMountName}` : '',
foundWillReceivePropsName !== null
? `\n ${foundWillReceivePropsName}`
Expand Down Expand Up @@ -696,11 +705,12 @@ export default function(
}

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

// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof ctor.getDerivedStateFromProps !== 'function' &&
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
typeof instance.componentWillReceiveProps === 'function') &&
typeof ctor.getDerivedStateFromProps !== 'function'
typeof instance.componentWillReceiveProps === 'function')
) {
if (oldProps !== newProps || oldContext !== newContext) {
callComponentWillReceiveProps(
Expand Down Expand Up @@ -852,11 +863,12 @@ export default function(

if (shouldUpdate) {
// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof ctor.getDerivedStateFromProps !== 'function' &&
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
(typeof instance.UNSAFE_componentWillMount === 'function' ||
typeof instance.componentWillMount === 'function') &&
typeof ctor.getDerivedStateFromProps !== 'function'
typeof instance.componentWillMount === 'function')
) {
startPhaseTimer(workInProgress, 'componentWillMount');
if (typeof instance.componentWillMount === 'function') {
Expand Down Expand Up @@ -913,11 +925,12 @@ export default function(
// during componentDidUpdate we pass the "current" props.

// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof ctor.getDerivedStateFromProps !== 'function' &&
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
typeof instance.componentWillReceiveProps === 'function') &&
typeof ctor.getDerivedStateFromProps !== 'function'
typeof instance.componentWillReceiveProps === 'function')
) {
if (oldProps !== newProps || oldContext !== newContext) {
callComponentWillReceiveProps(
Expand Down Expand Up @@ -1038,11 +1051,12 @@ export default function(

if (shouldUpdate) {
// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for any component with the new gDSFP.
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof ctor.getDerivedStateFromProps !== 'function' &&
typeof instance.getSnapshotBeforeUpdate !== 'function' &&
(typeof instance.UNSAFE_componentWillUpdate === 'function' ||
typeof instance.componentWillUpdate === 'function') &&
typeof ctor.getDerivedStateFromProps !== 'function'
typeof instance.componentWillUpdate === 'function')
) {
startPhaseTimer(workInProgress, 'componentWillUpdate');
if (typeof instance.componentWillUpdate === 'function') {
Expand Down

0 comments on commit 55e75da

Please sign in to comment.