Skip to content

Commit 1c7f6b9

Browse files
committed
Don't bail on new context Provider if a legacy provider rendered above
1 parent 76b4ba0 commit 1c7f6b9

File tree

2 files changed

+73
-3
lines changed

2 files changed

+73
-3
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -872,8 +872,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
872872

873873
const newProps = workInProgress.pendingProps;
874874
const oldProps = workInProgress.memoizedProps;
875+
let canBailOnProps = true;
875876

876877
if (hasLegacyContextChanged()) {
878+
canBailOnProps = false;
877879
// Normally we can bail out on props equality but if context has changed
878880
// we don't do the bailout and we have to reuse existing props instead.
879881
} else if (oldProps === newProps) {
@@ -890,9 +892,11 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
890892
// Initial render
891893
changedBits = MAX_SIGNED_31_BIT_INT;
892894
} else {
895+
const canBailOnChildren =
896+
canBailOnProps && oldProps.children === newProps.children;
893897
if (oldProps.value === newProps.value) {
894898
// No change. Bailout early if children are the same.
895-
if (oldProps.children === newProps.children) {
899+
if (canBailOnChildren) {
896900
workInProgress.stateNode = 0;
897901
pushProvider(workInProgress);
898902
return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -909,7 +913,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
909913
(oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare
910914
) {
911915
// No change. Bailout early if children are the same.
912-
if (oldProps.children === newProps.children) {
916+
if (canBailOnChildren) {
913917
workInProgress.stateNode = 0;
914918
pushProvider(workInProgress);
915919
return bailoutOnAlreadyFinishedWork(current, workInProgress);
@@ -932,7 +936,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
932936

933937
if (changedBits === 0) {
934938
// No change. Bailout early if children are the same.
935-
if (oldProps.children === newProps.children) {
939+
if (canBailOnChildren) {
936940
workInProgress.stateNode = 0;
937941
pushProvider(workInProgress);
938942
return bailoutOnAlreadyFinishedWork(current, workInProgress);

packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,72 @@ describe('ReactNewContext', () => {
847847
expect(ReactNoop.getChildren()).toEqual([span('Child')]);
848848
});
849849

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+
850916
it('consumer bails out if value is unchanged and something above bailed out', () => {
851917
const Context = React.createContext(0);
852918

0 commit comments

Comments
 (0)