Added DEV warnings and tests for new lifecycle

facebook · Mar 22, 2018 · 7022568 · 7022568
1 parent db84b9a
commit 7022568
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 1 deletion.
diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
@@ -955,4 +955,44 @@ describe('ReactComponentLifeCycle', () => {
     // De-duped
     ReactDOM.render(<MyComponent />, div);
   });
+
+  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);
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -42,13 +42,15 @@ let didWarnAboutStateAssignmentForComponent;
 let didWarnAboutUndefinedDerivedState;
 let didWarnAboutUninitializedState;
 let didWarnAboutWillReceivePropsAndDerivedState;
+let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate;
 let warnOnInvalidCallback;
 
 if (__DEV__) {
   didWarnAboutStateAssignmentForComponent = {};
   didWarnAboutUndefinedDerivedState = {};
   didWarnAboutUninitializedState = {};
   didWarnAboutWillReceivePropsAndDerivedState = {};
+  didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate = new Set();
 
   const didWarnOnInvalidCallback = {};
 
@@ -364,6 +366,20 @@ export default function(
         name,
         name,
       );
+
+      if (
+        typeof instance.getSnapshotBeforeUpdate === 'function' &&
+        typeof instance.componentDidUpdate !== 'function' &&
+        !didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.has(type)
+      ) {
+        didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate.add(type);
+        warning(
+          false,
+          '%s: getSnapshotBeforeUpdate() should be used with componentDidUpdate(). ' +
+            'This component defines getSnapshotBeforeUpdate() only.',
+          getComponentName(workInProgress),
+        );
+      }
     }
 
     const state = instance.state;

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -49,6 +49,11 @@ const {
   clearCaughtError,
 } = ReactErrorUtils;
 
+let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
+if (__DEV__) {
+  didWarnAboutUndefinedSnapshotBeforeUpdate = new Set();
+}
+
 function logError(boundary: Fiber, errorInfo: CapturedValue<mixed>) {
   const source = errorInfo.source;
   let stack = errorInfo.stack;
@@ -176,7 +181,23 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
               prevProps,
               prevState,
             );
-            // TODO Warn about undefined return value
+            if (__DEV__) {
+              const didWarnSet = ((didWarnAboutUndefinedSnapshotBeforeUpdate: any): Set<
+                mixed,
+              >);
+              if (
+                snapshot === undefined &&
+                !didWarnSet.has(finishedWork.type)
+              ) {
+                didWarnSet.add(finishedWork.type);
+                warning(
+                  false,
+                  '%s.getSnapshotBeforeUpdate(): A snapshot value (or null) ' +
+                    'must be returned. You have returned undefined.',
+                  getComponentName(finishedWork),
+                );
+              }
+            }
             instance.__reactInternalSnapshotBeforeUpdate = snapshot;
             stopPhaseTimer();
           }