Context providers and consumers should bail-out on already finished work

Fixes bug where a consumer would re-render even if its props and context had not changed.
facebook · Feb 20, 2018 · 95cf3c1 · 95cf3c1
1 parent 48ffbf0
commit 95cf3c1
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 11 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -773,16 +773,6 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
     const newProps = workInProgress.pendingProps;
     const oldProps = workInProgress.memoizedProps;
 
-    if (hasLegacyContextChanged()) {
-      // Normally we can bail out on props equality but if context has changed
-      // we don't do the bailout and we have to reuse existing props instead.
-    } else if (oldProps === newProps) {
-      workInProgress.stateNode = 0;
-      pushProvider(workInProgress);
-      return bailoutOnAlreadyFinishedWork(current, workInProgress);
-    }
-    workInProgress.memoizedProps = newProps;
-
     const newValue = newProps.value;
 
     let changedBits: number;
@@ -830,9 +820,14 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
     workInProgress.stateNode = changedBits;
     pushProvider(workInProgress);
 
-    if (oldProps !== null && oldProps.children === newProps.children) {
+    if (hasLegacyContextChanged()) {
+      // Normally we can bail out on props equality but if context has changed
+      // we don't do the bailout and we have to reuse existing props instead.
+    } else if (oldProps === newProps) {
       return bailoutOnAlreadyFinishedWork(current, workInProgress);
     }
+
+    workInProgress.memoizedProps = newProps;
     const newChildren = newProps.children;
     reconcileChildren(current, workInProgress, newChildren);
     return workInProgress.child;
@@ -845,6 +840,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
   ) {
     const context: ReactContext<any> = workInProgress.type;
     const newProps = workInProgress.pendingProps;
+    const oldProps = workInProgress.memoizedProps;
 
     const newValue = context.currentValue;
     const changedBits = context.changedBits;
@@ -868,6 +864,14 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
     }
     workInProgress.stateNode = observedBits;
 
+    if (hasLegacyContextChanged()) {
+      // Normally we can bail out on props equality but if context has changed
+      // we don't do the bailout and we have to reuse existing props instead.
+    } else if (changedBits === 0 && oldProps === newProps) {
+      return bailoutOnAlreadyFinishedWork(current, workInProgress);
+    }
+
+    workInProgress.memoizedProps = newProps;
     const render = newProps.children;
     const newChildren = render(newValue);
     reconcileChildren(current, workInProgress, newChildren);

diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
@@ -644,6 +644,45 @@ describe('ReactNewContext', () => {
     }
   });
 
+  it("does not re-render if there's an update in a child", () => {
+    const Context = React.createContext(0);
+
+    let child;
+    class Child extends React.Component {
+      state = {step: 0};
+      render() {
+        ReactNoop.yield('Child');
+        return (
+          <span
+            prop={`Context: ${this.props.context}, Step: ${this.state.step}`}
+          />
+        );
+      }
+    }
+
+    function App(props) {
+      return (
+        <Context.Provider value={props.value}>
+          <Context.Consumer>
+            {value => {
+              ReactNoop.yield('Consumer render prop');
+              return <Child ref={inst => (child = inst)} context={value} />;
+            }}
+          </Context.Consumer>
+        </Context.Provider>
+      );
+    }
+
+    // Initial mount
+    ReactNoop.render(<App value={1} />);
+    expect(ReactNoop.flush()).toEqual(['Consumer render prop', 'Child']);
+    expect(ReactNoop.getChildren()).toEqual([span('Context: 1, Step: 0')]);
+
+    child.setState({step: 1});
+    expect(ReactNoop.flush()).toEqual(['Child']);
+    expect(ReactNoop.getChildren()).toEqual([span('Context: 1, Step: 1')]);
+  });
+
   describe('fuzz test', () => {
     const Fragment = React.Fragment;
     const contextKeys = ['A', 'B', 'C', 'D', 'E', 'F', 'G'];