From 70225680787a20ed8da7c1ed59fea53bc291bcc4 Mon Sep 17 00:00:00 2001
From: Brian Vaughn <brian.david.vaughn@gmail.com>
Date: Tue, 20 Mar 2018 11:31:12 -0700
Subject: [PATCH] Added DEV warnings and tests for new lifecycle

---
 .../__tests__/ReactComponentLifeCycle-test.js | 40 +++++++++++++++++++
 .../src/ReactFiberClassComponent.js           | 16 ++++++++
 .../src/ReactFiberCommitWork.js               | 23 ++++++++++-
 3 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
index f36bc88e3fa2f..ab71128e530b4 100644
--- 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
index 4e83705fba7f5..2663dcc2f25c3 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -42,6 +42,7 @@ let didWarnAboutStateAssignmentForComponent;
 let didWarnAboutUndefinedDerivedState;
 let didWarnAboutUninitializedState;
 let didWarnAboutWillReceivePropsAndDerivedState;
+let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate;
 let warnOnInvalidCallback;
 
 if (__DEV__) {
@@ -49,6 +50,7 @@ if (__DEV__) {
   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
index f261f52e34750..101f751f34d38 100644
--- 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();
           }