Fix shallow renderer set instance state after gDSFP before calling sCU (

facebook#14613) * Fix shallow renderer set instance state after gDSFP before calling sCU * Update ReactShallowRenderer.js * Unwind abstraction * Fewer names
jetoneza · Jan 23, 2019 · 2f8843b · 2f8843b
1 parent f2f3954
commit 2f8843b
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 24 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
@@ -1201,6 +1201,40 @@ describe('ReactComponentLifeCycle', () => {
     expect(log).toEqual([]);
   });
 
+  it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => {
+    const divRef = React.createRef();
+    class SimpleComponent extends React.Component {
+      constructor(props) {
+        super(props);
+        this.state = {
+          value: props.value,
+        };
+      }
+
+      static getDerivedStateFromProps(nextProps, prevState) {
+        if (nextProps.value === prevState.value) {
+          return null;
+        }
+        return {value: nextProps.value};
+      }
+
+      shouldComponentUpdate(nextProps, nextState) {
+        return nextState.value !== this.state.value;
+      }
+
+      render() {
+        return <div ref={divRef}>value: {this.state.value}</div>;
+      }
+    }
+
+    const div = document.createElement('div');
+
+    ReactDOM.render(<SimpleComponent value="initial" />, div);
+    expect(divRef.current.textContent).toBe('value: initial');
+    ReactDOM.render(<SimpleComponent value="updated" />, div);
+    expect(divRef.current.textContent).toBe('value: updated');
+  });
+
   it('should call getSnapshotBeforeUpdate before mutations are committed', () => {
     const log = [];
 

diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js
@@ -529,7 +529,20 @@ class ReactShallowRenderer {
           this._updater,
         );
 
-        this._updateStateFromStaticLifecycle(element.props);
+        if (typeof element.type.getDerivedStateFromProps === 'function') {
+          const partialState = element.type.getDerivedStateFromProps.call(
+            null,
+            element.props,
+            this._instance.state,
+          );
+          if (partialState != null) {
+            this._instance.state = Object.assign(
+              {},
+              this._instance.state,
+              partialState,
+            );
+          }
+        }
 
         if (element.type.hasOwnProperty('contextTypes')) {
           currentlyValidatingElement = element;
@@ -653,10 +666,19 @@ class ReactShallowRenderer {
         }
       }
     }
-    this._updateStateFromStaticLifecycle(props);
 
     // Read state after cWRP in case it calls setState
-    const state = this._newState || oldState;
+    let state = this._newState || oldState;
+    if (typeof type.getDerivedStateFromProps === 'function') {
+      const partialState = type.getDerivedStateFromProps.call(
+        null,
+        props,
+        state,
+      );
+      if (partialState != null) {
+        state = Object.assign({}, state, partialState);
+      }
+    }
 
     let shouldUpdate = true;
     if (this._forcedUpdate) {
@@ -692,34 +714,14 @@ class ReactShallowRenderer {
     this._instance.context = context;
     this._instance.props = props;
     this._instance.state = state;
+    this._newState = null;
 
     if (shouldUpdate) {
       this._rendered = this._instance.render();
     }
     // Intentionally do not call componentDidUpdate()
     // because DOM refs are not available.
   }
-
-  _updateStateFromStaticLifecycle(props: Object) {
-    if (this._element === null) {
-      return;
-    }
-    const {type} = this._element;
-
-    if (typeof type.getDerivedStateFromProps === 'function') {
-      const oldState = this._newState || this._instance.state;
-      const partialState = type.getDerivedStateFromProps.call(
-        null,
-        props,
-        oldState,
-      );
-
-      if (partialState != null) {
-        const newState = Object.assign({}, oldState, partialState);
-        this._instance.state = this._newState = newState;
-      }
-    }
-  }
 }
 
 let currentlyValidatingElement = null;

diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js
@@ -942,6 +942,42 @@ describe('ReactShallowRenderer', () => {
     expect(result).toEqual(<div>value:1</div>);
   });
 
+  it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => {
+    class SimpleComponent extends React.Component {
+      constructor(props) {
+        super(props);
+        this.state = {
+          value: props.value,
+        };
+      }
+
+      static getDerivedStateFromProps(nextProps, prevState) {
+        if (nextProps.value === prevState.value) {
+          return null;
+        }
+        return {value: nextProps.value};
+      }
+
+      shouldComponentUpdate(nextProps, nextState) {
+        return nextState.value !== this.state.value;
+      }
+
+      render() {
+        return <div>{`value:${this.state.value}`}</div>;
+      }
+    }
+
+    const shallowRenderer = createRenderer();
+    const initialResult = shallowRenderer.render(
+      <SimpleComponent value="initial" />,
+    );
+    expect(initialResult).toEqual(<div>value:initial</div>);
+    const updatedResult = shallowRenderer.render(
+      <SimpleComponent value="updated" />,
+    );
+    expect(updatedResult).toEqual(<div>value:updated</div>);
+  });
+
   it('can setState with an updater function', () => {
     let instance;