Skip to content

Commit 7022568

Browse files
committed
Added DEV warnings and tests for new lifecycle
1 parent db84b9a commit 7022568

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,4 +955,44 @@ describe('ReactComponentLifeCycle', () => {
955955
// De-duped
956956
ReactDOM.render(<MyComponent />, div);
957957
});
958+
959+
it('should warn if getSnapshotBeforeUpdate returns undefined', () => {
960+
class MyComponent extends React.Component {
961+
getSnapshotBeforeUpdate() {}
962+
componentDidUpdate() {}
963+
render() {
964+
return null;
965+
}
966+
}
967+
968+
const div = document.createElement('div');
969+
ReactDOM.render(<MyComponent value="foo" />, div);
970+
expect(() => ReactDOM.render(<MyComponent value="bar" />, div)).toWarnDev(
971+
'MyComponent.getSnapshotBeforeUpdate(): A snapshot value (or null) must ' +
972+
'be returned. You have returned undefined.',
973+
);
974+
975+
// De-duped
976+
ReactDOM.render(<MyComponent value="baz" />, div);
977+
});
978+
979+
it('should warn if getSnapshotBeforeUpdate is defined with no componentDidUpdate', () => {
980+
class MyComponent extends React.Component {
981+
getSnapshotBeforeUpdate() {
982+
return null;
983+
}
984+
render() {
985+
return null;
986+
}
987+
}
988+
989+
const div = document.createElement('div');
990+
expect(() => ReactDOM.render(<MyComponent />, div)).toWarnDev(
991+
'MyComponent: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
992+
'This component defines getSnapshotBeforeUpdate() only.',
993+
);
994+
995+
// De-duped
996+
ReactDOM.render(<MyComponent />, div);
997+
});
958998
});

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,15 @@ let didWarnAboutStateAssignmentForComponent;
4242
let didWarnAboutUndefinedDerivedState;
4343
let didWarnAboutUninitializedState;
4444
let didWarnAboutWillReceivePropsAndDerivedState;
45+
let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate;
4546
let warnOnInvalidCallback;
4647

4748
if (__DEV__) {
4849
didWarnAboutStateAssignmentForComponent = {};
4950
didWarnAboutUndefinedDerivedState = {};
5051
didWarnAboutUninitializedState = {};
5152
didWarnAboutWillReceivePropsAndDerivedState = {};
53+
didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate = new Set();
5254

5355
const didWarnOnInvalidCallback = {};
5456

@@ -364,6 +366,20 @@ export default function(
364366
name,
365367
name,
366368
);
369+
370+
if (
371+
typeof instance.getSnapshotBeforeUpdate === 'function' &&
372+
typeof instance.componentDidUpdate !== 'function' &&
373+
!didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.has(type)
374+
) {
375+
didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.add(type);
376+
warning(
377+
false,
378+
'%s: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
379+
'This component defines getSnapshotBeforeUpdate() only.',
380+
getComponentName(workInProgress),
381+
);
382+
}
367383
}
368384

369385
const state = instance.state;

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ const {
4949
clearCaughtError,
5050
} = ReactErrorUtils;
5151

52+
let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
53+
if (__DEV__) {
54+
didWarnAboutUndefinedSnapshotBeforeUpdate = new Set();
55+
}
56+
5257
function logError(boundary: Fiber, errorInfo: CapturedValue<mixed>) {
5358
const source = errorInfo.source;
5459
let stack = errorInfo.stack;
@@ -176,7 +181,23 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
176181
prevProps,
177182
prevState,
178183
);
179-
// TODO Warn about undefined return value
184+
if (__DEV__) {
185+
const didWarnSet = ((didWarnAboutUndefinedSnapshotBeforeUpdate: any): Set<
186+
mixed,
187+
>);
188+
if (
189+
snapshot === undefined &&
190+
!didWarnSet.has(finishedWork.type)
191+
) {
192+
didWarnSet.add(finishedWork.type);
193+
warning(
194+
false,
195+
'%s.getSnapshotBeforeUpdate(): A snapshot value (or null) ' +
196+
'must be returned. You have returned undefined.',
197+
getComponentName(finishedWork),
198+
);
199+
}
200+
}
180201
instance.__reactInternalSnapshotBeforeUpdate = snapshot;
181202
stopPhaseTimer();
182203
}

0 commit comments

Comments
 (0)