From 9f1964c9b1df19c62adbc7dbd5de8d7f41fcdef1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 9 Apr 2018 23:07:18 +0100 Subject: [PATCH 1/2] Don't bail on new context Provider if a legacy provider rendered above --- .../src/ReactFiberBeginWork.js | 10 ++- .../ReactNewContext-test.internal.js | 66 +++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 3c20dceacf3..856994dfa10 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -859,8 +859,10 @@ export default function( 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( // 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( (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( 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 index 0a4763a4fec..db2703d3a70 100644 --- 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 ; + } + + const children = ; + + 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 ( + + {this.props.children} + + ); + } + } + + const legacyProviderRef = React.createRef(); + const appRef = React.createRef(); + + // Initial mount + ReactNoop.render( + + + {children} + + , + ); + 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); From 21f72a0c0e6b74742dc459f983e8e1fc708c46d8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 26 Apr 2018 20:23:42 +0100 Subject: [PATCH 2/2] Avoid an extra variable --- packages/react-reconciler/src/ReactFiberBeginWork.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 856994dfa10..527e6787d1f 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -893,11 +893,9 @@ export default function( // 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 (canBailOnChildren) { + if (oldProps.children === newProps.children && canBailOnProps) { workInProgress.stateNode = 0; pushProvider(workInProgress); return bailoutOnAlreadyFinishedWork(current, workInProgress); @@ -914,7 +912,7 @@ export default function( (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare ) { // No change. Bailout early if children are the same. - if (canBailOnChildren) { + if (oldProps.children === newProps.children && canBailOnProps) { workInProgress.stateNode = 0; pushProvider(workInProgress); return bailoutOnAlreadyFinishedWork(current, workInProgress); @@ -937,7 +935,7 @@ export default function( if (changedBits === 0) { // No change. Bailout early if children are the same. - if (canBailOnChildren) { + if (oldProps.children === newProps.children && canBailOnProps) { workInProgress.stateNode = 0; pushProvider(workInProgress); return bailoutOnAlreadyFinishedWork(current, workInProgress);