Skip to content

Commit 299d017

Browse files
bvaughnrhagigi
authored andcommitted
New commit phase lifecycle: getSnapshotBeforeUpdate (facebook#12404)
* 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
1 parent f273ac4 commit 299d017

12 files changed

+597
-149
lines changed

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

Lines changed: 235 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ describe('ReactComponentLifeCycle', () => {
591591
}
592592
componentDidMount = logger('outer componentDidMount');
593593
shouldComponentUpdate = logger('outer shouldComponentUpdate');
594+
getSnapshotBeforeUpdate = logger('outer getSnapshotBeforeUpdate');
594595
componentDidUpdate = logger('outer componentDidUpdate');
595596
componentWillUnmount = logger('outer componentWillUnmount');
596597
render() {
@@ -610,6 +611,7 @@ describe('ReactComponentLifeCycle', () => {
610611
}
611612
componentDidMount = logger('inner componentDidMount');
612613
shouldComponentUpdate = logger('inner shouldComponentUpdate');
614+
getSnapshotBeforeUpdate = logger('inner getSnapshotBeforeUpdate');
613615
componentDidUpdate = logger('inner componentDidUpdate');
614616
componentWillUnmount = logger('inner componentWillUnmount');
615617
render() {
@@ -635,6 +637,8 @@ describe('ReactComponentLifeCycle', () => {
635637
'outer shouldComponentUpdate',
636638
'inner getDerivedStateFromProps',
637639
'inner shouldComponentUpdate',
640+
'inner getSnapshotBeforeUpdate',
641+
'outer getSnapshotBeforeUpdate',
638642
'inner componentDidUpdate',
639643
'outer componentDidUpdate',
640644
]);
@@ -669,10 +673,38 @@ describe('ReactComponentLifeCycle', () => {
669673

670674
const container = document.createElement('div');
671675
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
672-
'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.',
673677
);
674678
});
675679

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+
676708
it('should not invoke new unsafe lifecycles (cWM/cWRP/cWU) if static gDSFP is present', () => {
677709
class Component extends React.Component {
678710
state = {};
@@ -694,9 +726,10 @@ describe('ReactComponentLifeCycle', () => {
694726
}
695727

696728
const container = document.createElement('div');
697-
expect(() => ReactDOM.render(<Component />, container)).toWarnDev(
698-
'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.',
699731
);
732+
ReactDOM.render(<Component value={2} />, container);
700733
});
701734

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

718751
expect(() => ReactDOM.render(<AllLegacyLifecycles />, container)).toWarnDev(
719-
'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' +
720753
'AllLegacyLifecycles uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
721754
' componentWillMount\n' +
722755
' UNSAFE_componentWillReceiveProps\n' +
@@ -737,7 +770,7 @@ describe('ReactComponentLifeCycle', () => {
737770
}
738771

739772
expect(() => ReactDOM.render(<WillMount />, container)).toWarnDev(
740-
'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' +
741774
'WillMount uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
742775
' UNSAFE_componentWillMount\n\n' +
743776
'The above lifecycles should be removed. Learn more about this warning here:\n' +
@@ -757,7 +790,7 @@ describe('ReactComponentLifeCycle', () => {
757790
}
758791

759792
expect(() => ReactDOM.render(<WillMountAndUpdate />, container)).toWarnDev(
760-
'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' +
761794
'WillMountAndUpdate uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
762795
' componentWillMount\n' +
763796
' UNSAFE_componentWillUpdate\n\n' +
@@ -777,14 +810,96 @@ describe('ReactComponentLifeCycle', () => {
777810
}
778811

779812
expect(() => ReactDOM.render(<WillReceiveProps />, container)).toWarnDev(
780-
'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' +
781814
'WillReceiveProps uses getDerivedStateFromProps() but also contains the following legacy lifecycles:\n' +
782815
' componentWillReceiveProps\n\n' +
783816
'The above lifecycles should be removed. Learn more about this warning here:\n' +
784817
'https://fb.me/react-async-component-lifecycle-hooks',
785818
);
786819
});
787820

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+
788903
it('calls effects on module-pattern component', function() {
789904
const log = [];
790905

@@ -961,4 +1076,117 @@ describe('ReactComponentLifeCycle', () => {
9611076
ReactTestUtils.Simulate.click(divRef.current);
9621077
expect(divRef.current.textContent).toBe('remote:2, local:2');
9631078
});
1079+
1080+
it('should pass the return value from getSnapshotBeforeUpdate to componentDidUpdate', () => {
1081+
const log = [];
1082+
1083+
class MyComponent extends React.Component {
1084+
state = {
1085+
value: 0,
1086+
};
1087+
static getDerivedStateFromProps(nextProps, prevState) {
1088+
return {
1089+
value: prevState.value + 1,
1090+
};
1091+
}
1092+
getSnapshotBeforeUpdate(prevProps, prevState) {
1093+
log.push(
1094+
`getSnapshotBeforeUpdate() prevProps:${prevProps.value} prevState:${
1095+
prevState.value
1096+
}`,
1097+
);
1098+
return 'abc';
1099+
}
1100+
componentDidUpdate(prevProps, prevState, snapshot) {
1101+
log.push(
1102+
`componentDidUpdate() prevProps:${prevProps.value} prevState:${
1103+
prevState.value
1104+
} snapshot:${snapshot}`,
1105+
);
1106+
}
1107+
render() {
1108+
log.push('render');
1109+
return null;
1110+
}
1111+
}
1112+
1113+
const div = document.createElement('div');
1114+
ReactDOM.render(
1115+
<div>
1116+
<MyComponent value="foo" />
1117+
</div>,
1118+
div,
1119+
);
1120+
expect(log).toEqual(['render']);
1121+
log.length = 0;
1122+
1123+
ReactDOM.render(
1124+
<div>
1125+
<MyComponent value="bar" />
1126+
</div>,
1127+
div,
1128+
);
1129+
expect(log).toEqual([
1130+
'render',
1131+
'getSnapshotBeforeUpdate() prevProps:foo prevState:1',
1132+
'componentDidUpdate() prevProps:foo prevState:1 snapshot:abc',
1133+
]);
1134+
log.length = 0;
1135+
1136+
ReactDOM.render(
1137+
<div>
1138+
<MyComponent value="baz" />
1139+
</div>,
1140+
div,
1141+
);
1142+
expect(log).toEqual([
1143+
'render',
1144+
'getSnapshotBeforeUpdate() prevProps:bar prevState:2',
1145+
'componentDidUpdate() prevProps:bar prevState:2 snapshot:abc',
1146+
]);
1147+
log.length = 0;
1148+
1149+
ReactDOM.render(<div />, div);
1150+
expect(log).toEqual([]);
1151+
});
1152+
1153+
it('should warn if getSnapshotBeforeUpdate returns undefined', () => {
1154+
class MyComponent extends React.Component {
1155+
getSnapshotBeforeUpdate() {}
1156+
componentDidUpdate() {}
1157+
render() {
1158+
return null;
1159+
}
1160+
}
1161+
1162+
const div = document.createElement('div');
1163+
ReactDOM.render(<MyComponent value="foo" />, div);
1164+
expect(() => ReactDOM.render(<MyComponent value="bar" />, div)).toWarnDev(
1165+
'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' +
1166+
'be returned. You have returned undefined.',
1167+
);
1168+
1169+
// De-duped
1170+
ReactDOM.render(<MyComponent value="baz" />, div);
1171+
});
1172+
1173+
it('should warn if getSnapshotBeforeUpdate is defined with no componentDidUpdate', () => {
1174+
class MyComponent extends React.Component {
1175+
getSnapshotBeforeUpdate() {
1176+
return null;
1177+
}
1178+
render() {
1179+
return null;
1180+
}
1181+
}
1182+
1183+
const div = document.createElement('div');
1184+
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
1185+
'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
1186+
'This component defines getSnapshotBeforeUpdate() only.',
1187+
);
1188+
1189+
// De-duped
1190+
ReactDOM.render(<MyComponent />, div);
1191+
});
9641192
});

packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2081,4 +2081,44 @@ describe('ReactErrorBoundaries', () => {
20812081
});
20822082
}).toThrow('foo error');
20832083
});
2084+
2085+
it('handles errors that occur in before-mutation commit hook', () => {
2086+
const errors = [];
2087+
let caughtError;
2088+
class Parent extends React.Component {
2089+
getSnapshotBeforeUpdate() {
2090+
errors.push('parent sad');
2091+
throw new Error('parent sad');
2092+
}
2093+
componentDidUpdate() {}
2094+
render() {
2095+
return <Child {...this.props} />;
2096+
}
2097+
}
2098+
class Child extends React.Component {
2099+
getSnapshotBeforeUpdate() {
2100+
errors.push('child sad');
2101+
throw new Error('child sad');
2102+
}
2103+
componentDidUpdate() {}
2104+
render() {
2105+
return <div />;
2106+
}
2107+
}
2108+
2109+
const container = document.createElement('div');
2110+
ReactDOM.render(<Parent value={1} />, container);
2111+
try {
2112+
ReactDOM.render(<Parent value={2} />, container);
2113+
} catch (e) {
2114+
if (e.message !== 'parent sad' && e.message !== 'child sad') {
2115+
throw e;
2116+
}
2117+
caughtError = e;
2118+
}
2119+
2120+
expect(errors).toEqual(['child sad', 'parent sad']);
2121+
// Error should be the first thrown
2122+
expect(caughtError.message).toBe('child sad');
2123+
});
20842124
});

packages/react-reconciler/src/ReactDebugFiberPerf.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ type MeasurementPhase =
3131
| 'componentWillUpdate'
3232
| 'componentDidUpdate'
3333
| 'componentDidMount'
34-
| 'getChildContext';
34+
| 'getChildContext'
35+
| 'getSnapshotBeforeUpdate';
3536

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

0 commit comments

Comments
 (0)