Skip to content

Commit

Permalink
New commit phase lifecycle: getSnapshotBeforeUpdate (facebook#12404)
Browse files Browse the repository at this point in the history
* Implemented new getSnapshotBeforeUpdate lifecycle
* Store snapshot value from Fiber to instance (__reactInternalSnapshotBeforeUpdate)
* Use commitAllHostEffects() traversal for getSnapshotBeforeUpdate()
* Added DEV warnings and tests for new lifecycle
* Don't invoke legacy lifecycles if getSnapshotBeforeUpdate() is defined. DEV warn about this.
* Converted did-warn objects to Sets in ReactFiberClassComponent
* Replaced redundant new lifecycle checks in a few methods
* Check for polyfill suppress flag on cWU as well before warning
* Added Snapshot bit to HostEffectMask
  • Loading branch information
bvaughn authored and rhagigi committed Apr 19, 2018
1 parent f273ac4 commit 299d017
Show file tree
Hide file tree
Showing 12 changed files with 597 additions and 149 deletions.
242 changes: 235 additions & 7 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ describe('ReactComponentLifeCycle', () => {
}
componentDidMount = logger('outer componentDidMount');
shouldComponentUpdate = logger('outer shouldComponentUpdate');
getSnapshotBeforeUpdate = logger('outer getSnapshotBeforeUpdate');
componentDidUpdate = logger('outer componentDidUpdate');
componentWillUnmount = logger('outer componentWillUnmount');
render() {
Expand All @@ -610,6 +611,7 @@ describe('ReactComponentLifeCycle', () => {
}
componentDidMount = logger('inner componentDidMount');
shouldComponentUpdate = logger('inner shouldComponentUpdate');
getSnapshotBeforeUpdate = logger('inner getSnapshotBeforeUpdate');
componentDidUpdate = logger('inner componentDidUpdate');
componentWillUnmount = logger('inner componentWillUnmount');
render() {
Expand All @@ -635,6 +637,8 @@ describe('ReactComponentLifeCycle', () => {
'outer shouldComponentUpdate',
'inner getDerivedStateFromProps',
'inner shouldComponentUpdate',
'inner getSnapshotBeforeUpdate',
'outer getSnapshotBeforeUpdate',
'inner componentDidUpdate',
'outer componentDidUpdate',
]);
Expand Down Expand Up @@ -669,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 @@ -694,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 @@ -716,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 @@ -737,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 @@ -757,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 @@ -777,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 @@ -961,4 +1076,117 @@ describe('ReactComponentLifeCycle', () => {
ReactTestUtils.Simulate.click(divRef.current);
expect(divRef.current.textContent).toBe('remote:2, local:2');
});

it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => {
const log = [];

class MyComponent extends React.Component {
state = {
value: 0,
};
static getDerivedStateFromProps(nextProps, prevState) {
return {
value: prevState.value + 1,
};
}
getSnapshotBeforeUpdate(prevProps, prevState) {
log.push(
`getSnapshotBeforeUpdate() prevProps:${prevProps.value} prevState:${
prevState.value
}`,
);
return 'abc';
}
componentDidUpdate(prevProps, prevState, snapshot) {
log.push(
`componentDidUpdate() prevProps:${prevProps.value} prevState:${
prevState.value
} snapshot:${snapshot}`,
);
}
render() {
log.push('render');
return null;
}
}

const div = document.createElement('div');
ReactDOM.render(
<div>
<MyComponent value="foo" />
</div>,
div,
);
expect(log).toEqual(['render']);
log.length = 0;

ReactDOM.render(
<div>
<MyComponent value="bar" />
</div>,
div,
);
expect(log).toEqual([
'render',
'getSnapshotBeforeUpdate() prevProps:foo prevState:1',
'componentDidUpdate() prevProps:foo prevState:1 snapshot:abc',
]);
log.length = 0;

ReactDOM.render(
<div>
<MyComponent value="baz" />
</div>,
div,
);
expect(log).toEqual([
'render',
'getSnapshotBeforeUpdate() prevProps:bar prevState:2',
'componentDidUpdate() prevProps:bar prevState:2 snapshot:abc',
]);
log.length = 0;

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

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

const div = document.createElement('div');
ReactDOM.render(<MyComponent value="foo" />, div);
expect(() => ReactDOM.render(<MyComponent value="bar" />, div)).toWarnDev(
'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' +
'be returned. You have returned undefined.',
);

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

it('should warn if getSnapshotBeforeUpdate is defined with no componentDidUpdate', () => {
class MyComponent extends React.Component {
getSnapshotBeforeUpdate() {
return null;
}
render() {
return null;
}
}

const div = document.createElement('div');
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
'This component defines getSnapshotBeforeUpdate() only.',
);

// De-duped
ReactDOM.render(<MyComponent />, div);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2081,4 +2081,44 @@ describe('ReactErrorBoundaries', () => {
});
}).toThrow('foo error');
});

it('handles errors that occur in before-mutation commit hook', () => {
const errors = [];
let caughtError;
class Parent extends React.Component {
getSnapshotBeforeUpdate() {
errors.push('parent sad');
throw new Error('parent sad');
}
componentDidUpdate() {}
render() {
return <Child {...this.props} />;
}
}
class Child extends React.Component {
getSnapshotBeforeUpdate() {
errors.push('child sad');
throw new Error('child sad');
}
componentDidUpdate() {}
render() {
return <div />;
}
}

const container = document.createElement('div');
ReactDOM.render(<Parent value={1} />, container);
try {
ReactDOM.render(<Parent value={2} />, container);
} catch (e) {
if (e.message !== 'parent sad' && e.message !== 'child sad') {
throw e;
}
caughtError = e;
}

expect(errors).toEqual(['child sad', 'parent sad']);
// Error should be the first thrown
expect(caughtError.message).toBe('child sad');
});
});
3 changes: 2 additions & 1 deletion packages/react-reconciler/src/ReactDebugFiberPerf.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type MeasurementPhase =
| 'componentWillUpdate'
| 'componentDidUpdate'
| 'componentDidMount'
| 'getChildContext';
| 'getChildContext'
| 'getSnapshotBeforeUpdate';

// Prefix measurements so that it's possible to filter them.
// Longer prefixes are hard to read in DevTools.
Expand Down
Loading

0 comments on commit 299d017

Please sign in to comment.