Don't bail on new context Provider if a legacy provider rendered above

facebook · Apr 26, 2018 · 9f1964c · 9f1964c
1 parent d883d59
commit 9f1964c
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 3 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -859,8 +859,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
 
     const newProps = workInProgress.pendingProps;
     const oldProps = workInProgress.memoizedProps;
+    let canBailOnProps = true;
 
     if (hasLegacyContextChanged()) {
+      canBailOnProps = false;
       // 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) {
@@ -891,9 +893,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
       // Initial render
       changedBits = MAX_SIGNED_31_BIT_INT;
     } else {
+      const canBailOnChildren =
+        canBailOnProps && oldProps.children === newProps.children;
       if (oldProps.value === newProps.value) {
         // No change. Bailout early if children are the same.
-        if (oldProps.children === newProps.children) {
+        if (canBailOnChildren) {
           workInProgress.stateNode = 0;
           pushProvider(workInProgress);
           return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -910,7 +914,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
           (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare
         ) {
           // No change. Bailout early if children are the same.
-          if (oldProps.children === newProps.children) {
+          if (canBailOnChildren) {
             workInProgress.stateNode = 0;
             pushProvider(workInProgress);
             return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -933,7 +937,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
 
           if (changedBits === 0) {
             // No change. Bailout early if children are the same.
-            if (oldProps.children === newProps.children) {
+            if (canBailOnChildren) {
               workInProgress.stateNode = 0;
               pushProvider(workInProgress);
               return bailoutOnAlreadyFinishedWork(current, workInProgress);

diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
@@ -847,6 +847,72 @@ describe('ReactNewContext', () => {
     expect(ReactNoop.getChildren()).toEqual([span('Child')]);
   });
 
+  it('provider does not bail out if legacy context changed above', () => {
+    const Context = React.createContext(0);
+
+    function Child() {
+      ReactNoop.yield('Child');
+      return <span prop="Child" />;
+    }
+
+    const children = <Child />;
+
+    class LegacyProvider extends React.Component {
+      static childContextTypes = {
+        legacyValue: () => {},
+      };
+      state = {legacyValue: 1};
+      getChildContext() {
+        return {legacyValue: this.state.legacyValue};
+      }
+      render() {
+        ReactNoop.yield('LegacyProvider');
+        return this.props.children;
+      }
+    }
+
+    class App extends React.Component {
+      state = {value: 1};
+      render() {
+        ReactNoop.yield('App');
+        return (
+          <Context.Provider value={this.state.value}>
+            {this.props.children}
+          </Context.Provider>
+        );
+      }
+    }
+
+    const legacyProviderRef = React.createRef();
+    const appRef = React.createRef();
+
+    // Initial mount
+    ReactNoop.render(
+      <LegacyProvider ref={legacyProviderRef}>
+        <App ref={appRef} value={1}>
+          {children}
+        </App>
+      </LegacyProvider>,
+    );
+    expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
+    expect(ReactNoop.getChildren()).toEqual([span('Child')]);
+
+    // Update App with same value (should bail out)
+    appRef.current.setState({value: 1});
+    expect(ReactNoop.flush()).toEqual(['App']);
+    expect(ReactNoop.getChildren()).toEqual([span('Child')]);
+
+    // Update LegacyProvider (should not bail out)
+    legacyProviderRef.current.setState({value: 1});
+    expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
+    expect(ReactNoop.getChildren()).toEqual([span('Child')]);
+
+    // Update App with same value (should bail out)
+    appRef.current.setState({value: 1});
+    expect(ReactNoop.flush()).toEqual(['App']);
+    expect(ReactNoop.getChildren()).toEqual([span('Child')]);
+  });
+
   it('consumer bails out if value is unchanged and something above bailed out', () => {
     const Context = React.createContext(0);