Don't bail on new context Provider if a legacy provider rendered above (#12586)
* Don't bail on new context Provider if a legacy provider rendered above * Avoid an extra variable
Dan Abramov committed
Apr 26, 2018 at 20:59 UTC
7c3932857195d0cfba2379875b65f1bd1fae39fd
2 files changed
+71
-3
packages/react-reconciler/src/ReactFiberBeginWork.js
+5
-3
@@ -859,8 +859,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
859
860
const newProps = workInProgress.pendingProps;
861
const oldProps = workInProgress.memoizedProps;
862
+ let canBailOnProps = true;
863
864
if (hasLegacyContextChanged()) {
865
+ canBailOnProps = false;
866
// Normally we can bail out on props equality but if context has changed
867
// we don't do the bailout and we have to reuse existing props instead.
868
} else if (oldProps === newProps) {
@@ -893,7 +895,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
895
} else {
896
if (oldProps.value === newProps.value) {
897
// No change. Bailout early if children are the same.
896
- if (oldProps.children === newProps.children) {
898
+ if (oldProps.children === newProps.children && canBailOnProps) {
899
workInProgress.stateNode = 0;
900
pushProvider(workInProgress);
901
return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -910,7 +912,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
912
(oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare
913
) {
914
// No change. Bailout early if children are the same.
913
- if (oldProps.children === newProps.children) {
915
+ if (oldProps.children === newProps.children && canBailOnProps) {
916
workInProgress.stateNode = 0;
917
pushProvider(workInProgress);
918
return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -933,7 +935,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
935
936
if (changedBits === 0) {
937
// No change. Bailout early if children are the same.
936
- if (oldProps.children === newProps.children) {
938
+ if (oldProps.children === newProps.children && canBailOnProps) {
939
workInProgress.stateNode = 0;
940
pushProvider(workInProgress);
941
return bailoutOnAlreadyFinishedWork(current, workInProgress);
packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
+66
@@ -847,6 +847,72 @@ describe('ReactNewContext', () => {
847
expect(ReactNoop.getChildren()).toEqual([span('Child')]);
848
});
849
850
+ it('provider does not bail out if legacy context changed above', () => {
851
+ const Context = React.createContext(0);
852
+
853
+ function Child() {
854
+ ReactNoop.yield('Child');
855
+ return <span prop="Child" />;
856
+ }
857
+
858
+ const children = <Child />;
859
+
860
+ class LegacyProvider extends React.Component {
861
+ static childContextTypes = {
862
+ legacyValue: () => {},
863
+ };
864
+ state = {legacyValue: 1};
865
+ getChildContext() {
866
+ return {legacyValue: this.state.legacyValue};
867
+ }
868
+ render() {
869
+ ReactNoop.yield('LegacyProvider');
870
+ return this.props.children;
871
+ }
872
+ }
873
+
874
+ class App extends React.Component {
875
+ state = {value: 1};
876
+ render() {
877
+ ReactNoop.yield('App');
878
+ return (
879
+ <Context.Provider value={this.state.value}>
880
+ {this.props.children}
881
+ </Context.Provider>
882
+ );
883
+ }
884
+ }
885
+
886
+ const legacyProviderRef = React.createRef();
887
+ const appRef = React.createRef();
888
+
889
+ // Initial mount
890
+ ReactNoop.render(
891
+ <LegacyProvider ref={legacyProviderRef}>
892
+ <App ref={appRef} value={1}>
893
+ {children}
894
+ </App>
895
+ </LegacyProvider>,
896
+ );
897
+ expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
898
+ expect(ReactNoop.getChildren()).toEqual([span('Child')]);
899
+
900
+ // Update App with same value (should bail out)
901
+ appRef.current.setState({value: 1});
902
+ expect(ReactNoop.flush()).toEqual(['App']);
903
+ expect(ReactNoop.getChildren()).toEqual([span('Child')]);
904
+
905
+ // Update LegacyProvider (should not bail out)
906
+ legacyProviderRef.current.setState({value: 1});
907
+ expect(ReactNoop.flush()).toEqual(['LegacyProvider', 'App', 'Child']);
908
+ expect(ReactNoop.getChildren()).toEqual([span('Child')]);
909
+
910
+ // Update App with same value (should bail out)
911
+ appRef.current.setState({value: 1});
912
+ expect(ReactNoop.flush()).toEqual(['App']);
913
+ expect(ReactNoop.getChildren()).toEqual([span('Child')]);
914
+ });
915
+
916
it('consumer bails out if value is unchanged and something above bailed out', () => {
917
const Context = React.createContext(0);
918