Skip to content

Commit

Permalink
Added getDerivedStateFromProps to ReactFiberClassComponent
Browse files Browse the repository at this point in the history 
Also updated class component and lifecyle tests to cover the added functionality.
  • Loading branch information
@bvaughn
bvaughn committed Jan 17, 2018
1 parent 8d0e001 commit b71ca93
Show file tree
Hide file tree
Showing 5 changed files with 340 additions and 33 deletions.
73 changes: 71 additions & 2 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,10 @@ describe('ReactComponentLifeCycle', () => {
};
};
class Outer extends React.Component {
static getDerivedStateFromProps(props, prevState) {
log.push('outer getDerivedStateFromProps');
return null;
}
UNSAFE_componentWillMount = logger('outer componentWillMount');
componentDidMount = logger('outer componentDidMount');
UNSAFE_componentWillReceiveProps = logger(
Expand All @@ -528,6 +532,10 @@ describe('ReactComponentLifeCycle', () => {
}

class Inner extends React.Component {
static getDerivedStateFromProps(props, prevState) {
log.push('inner getDerivedStateFromProps');
return null;
}
UNSAFE_componentWillMount = logger('inner componentWillMount');
componentDidMount = logger('inner componentDidMount');
UNSAFE_componentWillReceiveProps = logger(
Expand All @@ -544,21 +552,33 @@ describe('ReactComponentLifeCycle', () => {

const container = document.createElement('div');
log = [];
ReactDOM.render(<Outer x={17} />, container);
expect(() => ReactDOM.render(<Outer x={1} />, container)).toWarnDev([
'Warning: Outer: Defines both componentWillReceiveProps() and static ' +
'getDerivedStateFromProps() methods. ' +
'We recommend using only getDerivedStateFromProps().',
'Warning: Inner: Defines both componentWillReceiveProps() and static ' +
'getDerivedStateFromProps() methods. ' +
'We recommend using only getDerivedStateFromProps().',
]);
expect(log).toEqual([
'outer getDerivedStateFromProps',
'outer componentWillMount',
'inner getDerivedStateFromProps',
'inner componentWillMount',
'inner componentDidMount',
'outer componentDidMount',
]);

// Dedup warnings
log = [];
ReactDOM.render(<Outer x={42} />, container);
ReactDOM.render(<Outer x={2} />, container);
expect(log).toEqual([
'outer componentWillReceiveProps',
'outer getDerivedStateFromProps',
'outer shouldComponentUpdate',
'outer componentWillUpdate',
'inner componentWillReceiveProps',
'inner getDerivedStateFromProps',
'inner shouldComponentUpdate',
'inner componentWillUpdate',
'inner componentDidUpdate',
Expand All @@ -573,6 +593,37 @@ describe('ReactComponentLifeCycle', () => {
]);
});

it('warns about deprecated unsafe lifecycles', function() {
class MyComponent extends React.Component {
componentWillMount() {}
componentWillReceiveProps() {}
componentWillUpdate() {}
render() {
return null;
}
}

const container = document.createElement('div');
expect(() => ReactDOM.render(<MyComponent x={1} />, container)).toWarnDev([
'Warning: MyComponent: componentWillMount() is deprecated and will be ' +
'removed in the next major version. ' +
'Please use UNSAFE_componentWillMount() instead.',
]);

expect(() => ReactDOM.render(<MyComponent x={2} />, container)).toWarnDev([
'Warning: MyComponent: componentWillReceiveProps() is deprecated and ' +
'will be removed in the next major version. ' +
'Please use UNSAFE_componentWillReceiveProps() instead.',
'Warning: MyComponent: componentWillUpdate() is deprecated and will be ' +
'removed in the next major version. ' +
'Please use UNSAFE_componentWillUpdate() instead.',
]);

// Dedupe check (instantiate and update)
ReactDOM.render(<MyComponent key="new" x={1} />, container);
ReactDOM.render(<MyComponent key="new" x={2} />, container);
});

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

Expand Down Expand Up @@ -623,4 +674,22 @@ describe('ReactComponentLifeCycle', () => {
'ref',
]);
});

it('should warn if getDerivedStateFromProps returns undefined', () => {
class MyComponent extends React.Component {
static getDerivedStateFromProps() {}
render() {
return null;
}
}

const div = document.createElement('div');
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
'MyComponent.getDerivedStateFromProps(): A valid state object (or null) must ' +
'be returned. You may have returned undefined.',
);

// De-duped
ReactDOM.render(<MyComponent />, div);
});
});
143 changes: 112 additions & 31 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,23 @@ import {hasContextChanged} from './ReactFiberContext';
const fakeInternalInstance = {};
const isArray = Array.isArray;

let didWarnAboutLegacyWillMount;
let didWarnAboutLegacyWillReceiveProps;
let didWarnAboutLegacyWillUpdate;
let didWarnAboutStateAssignmentForComponent;
let didWarnAboutUndefinedDerivedState;
let didWarnAboutWillReceivePropsAndDerivedState;
let warnOnInvalidCallback;

if (__DEV__) {
const didWarnOnInvalidCallback = {};
didWarnAboutLegacyWillMount = {};
didWarnAboutLegacyWillReceiveProps = {};
didWarnAboutLegacyWillUpdate = {};
didWarnAboutStateAssignmentForComponent = {};
didWarnAboutUndefinedDerivedState = {};
didWarnAboutWillReceivePropsAndDerivedState = {};

const didWarnOnInvalidCallback = {};

warnOnInvalidCallback = function(callback: mixed, callerName: string) {
if (callback === null || typeof callback === 'function') {
Expand Down Expand Up @@ -380,8 +391,17 @@ export default function(
const instance = new ctor(props, context);
adoptClassInstance(workInProgress, instance);

// TODO (getDerivedStateFromProps) Call Component.getDerivedStateFromProps
// Merge the returned value into instance.state
const partialState = callGetDerivedStateFromProps(
workInProgress,
instance,
props,
);
if (partialState) {
// Render-phase updates (like this) should not be added to the update queue,
// So that multiple render passes do not enqueue multiple updates.
// Instead, just synchronously merge the returned state into the instance.
instance.state = Object.assign({}, instance.state, partialState);
}

// Cache unmasked context so we can avoid recreating masked context unless necessary.
// ReactFiberContext usually updates this cache but can't for newly-created instances.
Expand All @@ -398,12 +418,16 @@ export default function(

if (typeof instance.componentWillMount === 'function') {
if (__DEV__) {
warning(
false,
'%s: componentWillMount() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillMount() instead.',
getComponentName(workInProgress),
);
const componentName = getComponentName(workInProgress) || 'Unknown';
if (!didWarnAboutLegacyWillMount[componentName]) {
warning(
false,
'%s: componentWillMount() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillMount() instead.',
componentName,
);
didWarnAboutLegacyWillMount[componentName] = true;
}
}
instance.componentWillMount();
} else {
Expand Down Expand Up @@ -435,12 +459,16 @@ export default function(
const oldState = instance.state;
if (typeof instance.componentWillReceiveProps === 'function') {
if (__DEV__) {
warning(
false,
'%s: componentWillReceiveProps() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillReceiveProps() instead.',
getComponentName(workInProgress),
);
const componentName = getComponentName(workInProgress) || 'Unknown';
if (!didWarnAboutLegacyWillReceiveProps[componentName]) {
warning(
false,
'%s: componentWillReceiveProps() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillReceiveProps() instead.',
componentName,
);
didWarnAboutLegacyWillReceiveProps[componentName] = true;
}
}

startPhaseTimer(workInProgress, 'componentWillReceiveProps');
Expand All @@ -457,18 +485,9 @@ export default function(
}
}

// TODO (getDerivedStateFromProps) If both cWRP and static gDSFP methods exist, warn.
// Call cWRP first then static gDSFP; don't bother trying to sync apply setState() changes.

// TODO (getDerivedStateFromProps) Call Component.getDerivedStateFromProps

// TODO (getDerivedStateFromProps) Returned value should not be added to update queue.
// Just synchronously Object.assign it into instance.state
// This should be covered in a test too.

if (instance.state !== oldState) {
if (__DEV__) {
const componentName = getComponentName(workInProgress) || 'Component';
const componentName = getComponentName(workInProgress) || 'Unknown';
if (!didWarnAboutStateAssignmentForComponent[componentName]) {
warning(
false,
Expand All @@ -484,6 +503,50 @@ export default function(
}
}

function callGetDerivedStateFromProps(workInProgress, instance, props) {
const {type} = workInProgress;

if (typeof type.getDerivedStateFromProps === 'function') {
if (__DEV__) {
if (
typeof instance.componentWillReceiveProps === 'function' ||
typeof instance.UNSAFE_componentWillReceiveProps === 'function'
) {
const componentName = getComponentName(workInProgress) || 'Unknown';
if (!didWarnAboutWillReceivePropsAndDerivedState[componentName]) {
warning(
false,
'%s: Defines both componentWillReceiveProps() and static ' +
'getDerivedStateFromProps() methods. We recommend using ' +
'only getDerivedStateFromProps().',
componentName,
);
didWarnAboutWillReceivePropsAndDerivedState[componentName] = true;
}
}
}

const partialState = type.getDerivedStateFromProps(props, instance.state);

if (__DEV__) {
if (partialState === undefined) {
const componentName = getComponentName(workInProgress) || 'Unknown';
if (!didWarnAboutUndefinedDerivedState[componentName]) {
warning(
false,
'%s.getDerivedStateFromProps(): A valid state object (or null) must be returned. ' +
'You may have returned undefined.',
componentName,
);
didWarnAboutUndefinedDerivedState[componentName] = componentName;
}
}
}

return partialState || null;
}
}

// Invokes the mount life-cycles on a previously never rendered instance.
function mountClassInstance(
workInProgress: Fiber,
Expand Down Expand Up @@ -675,6 +738,12 @@ export default function(
);
}

const partialState = callGetDerivedStateFromProps(
workInProgress,
instance,
newProps,
);

// Compute the next state using the memoized state and the update queue.
const oldState = workInProgress.memoizedState;
// TODO: Previous state can be null.
Expand All @@ -692,6 +761,13 @@ export default function(
newState = oldState;
}

if (partialState) {
// Render-phase updates (like this) should not be added to the update queue,
// So that multiple render passes do not enqueue multiple updates.
// Instead, just synchronously merge the returned state into the instance.
newState = Object.assign({}, newState, partialState);
}

if (
oldProps === newProps &&
oldState === newState &&
Expand Down Expand Up @@ -730,12 +806,17 @@ export default function(
) {
if (typeof instance.componentWillUpdate === 'function') {
if (__DEV__) {
warning(
false,
'%s: componentWillUpdate() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillUpdate() instead.',
getComponentName(workInProgress),
);
const componentName =
getComponentName(workInProgress) || 'Component';
if (!didWarnAboutLegacyWillUpdate[componentName]) {
warning(
false,
'%s: componentWillUpdate() is deprecated and will be removed in the ' +
'next major version. Please use UNSAFE_componentWillUpdate() instead.',
componentName,
);
didWarnAboutLegacyWillUpdate[componentName] = true;
}
}

startPhaseTimer(workInProgress, 'componentWillUpdate');
Expand Down
49 changes: 49 additions & 0 deletions packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,55 @@ describe 'ReactCoffeeScriptClass', ->
test React.createElement(Foo), 'SPAN', 'bar'
undefined

it 'sets initial state with value returned by static getDerivedStateFromProps', ->
class Foo extends React.Component
render: ->
div
className: "#{@state.foo} #{@state.bar}"
Foo.getDerivedStateFromProps = (nextProps, prevState) ->
{
foo: nextProps.foo
bar: 'bar'
}
test React.createElement(Foo, foo: 'foo'), 'DIV', 'foo bar'
undefined

it 'updates initial state with values returned by static getDerivedStateFromProps', ->
class Foo extends React.Component
constructor: (props, context) ->
super props, context
@state =
foo: 'foo'
bar: 'bar'
render: ->
div
className: "#{@state.foo} #{@state.bar}"
Foo.getDerivedStateFromProps = (nextProps, prevState) ->
{
foo: "not-#{prevState.foo}"
}
test React.createElement(Foo), 'DIV', 'not-foo bar'
undefined

it 'renders updated state with values returned by static getDerivedStateFromProps', ->
class Foo extends React.Component
constructor: (props, context) ->
super props, context
@state =
value: 'initial'
render: ->
div
className: @state.value
Foo.getDerivedStateFromProps = (nextProps, prevState) ->
if nextProps.update
return {
value: 'updated'
}
return null
test React.createElement(Foo, update: false), 'DIV', 'initial'
test React.createElement(Foo, update: true), 'DIV', 'updated'
undefined

it 'renders based on context in the constructor', ->
class Foo extends React.Component
@contextTypes:
Expand Down
Loading

0 comments on commit b71ca93

Please sign in to comment.