Skip to content

Commit

Permalink
Invoke both legacy and UNSAFE_ lifecycles when both are present (#12134)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@bvaughn
bvaughn authored Feb 1, 2018
1 parent aeba3c4 commit 4eed18d
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 17 deletions.
42 changes: 42 additions & 0 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,4 +788,46 @@ describe('ReactComponentLifeCycle', () => {
// De-duped
ReactDOM.render(<MyComponent />, 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(<MyComponent foo="bar" />, div);
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);

log.length = 0;

ReactDOM.render(<MyComponent foo="baz" />, div);
expect(log).toEqual([
'componentWillReceiveProps',
'UNSAFE_componentWillReceiveProps',
'componentWillUpdate',
'UNSAFE_componentWillUpdate',
]);
});
});
19 changes: 19 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,4 +207,23 @@ describe('ReactDOMServerLifecycles', () => {
// De-duped
ReactDOMServer.renderToString(<Component />);
});

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(<Component />);
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
});
});
13 changes: 10 additions & 3 deletions packages/react-dom/src/server/ReactPartialRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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();
Expand Down
21 changes: 10 additions & 11 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,8 @@ export default function(

if (typeof instance.componentWillMount === 'function') {
instance.componentWillMount();
} else {
}
if (typeof instance.UNSAFE_componentWillMount === 'function') {
instance.UNSAFE_componentWillMount();
}

Expand All @@ -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');
}
if (typeof instance.UNSAFE_componentWillReceiveProps === 'function') {
instance.UNSAFE_componentWillReceiveProps(newProps, newContext);
stopPhaseTimer();
}
stopPhaseTimer();

if (instance.state !== oldState) {
if (__DEV__) {
Expand Down Expand Up @@ -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');
}
if (typeof instance.UNSAFE_componentWillUpdate === 'function') {
instance.UNSAFE_componentWillUpdate(newProps, newState, newContext);
stopPhaseTimer();
}
stopPhaseTimer();
}
if (typeof instance.componentDidUpdate === 'function') {
workInProgress.effectTag |= Update;
Expand Down
12 changes: 9 additions & 3 deletions packages/react-test-renderer/src/ReactShallowRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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'
) {
Expand Down Expand Up @@ -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'
) {
Expand Down
42 changes: 42 additions & 0 deletions packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1235,4 +1235,46 @@ describe('ReactShallowRenderer', () => {
// De-duped
shallowRenderer.render(<Component />);
});

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(<Component foo="bar" />);
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);

log.length = 0;

shallowRenderer.render(<Component foo="baz" />);
expect(log).toEqual([
'componentWillReceiveProps',
'UNSAFE_componentWillReceiveProps',
'componentWillUpdate',
'UNSAFE_componentWillUpdate',
]);
});
});
46 changes: 46 additions & 0 deletions packages/react/src/__tests__/createReactClassIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,4 +478,50 @@ describe('create-react-class-integration', () => {
}).toWarnDev('Defines both componentWillReceiveProps');
ReactDOM.render(<Component foo={1} />, 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(<Component foo="bar" />, div);
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);

log.length = 0;

ReactDOM.render(<Component foo="baz" />, div);
expect(log).toEqual([
'componentWillReceiveProps',
'UNSAFE_componentWillReceiveProps',
'componentWillUpdate',
'UNSAFE_componentWillUpdate',
]);
});
});

0 comments on commit 4eed18d

Please sign in to comment.