From 9c633c95cf4c8e26a7aaf6124187c90c8a23db80 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 4 Apr 2018 18:51:38 +0100 Subject: [PATCH 1/2] Don't render consumers that bailed out with bitmask even if there's a deeper matching child --- .../src/ReactFiberBeginWork.js | 4 + .../ReactNewContext-test.internal.js | 115 ++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index e8d69502062..6de5e54877e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -994,6 +994,10 @@ export default function( changedBits, renderExpirationTime, ); + } else if (oldProps === newProps) { + // Skip over a memoized parent with a bitmask bailout even + // if we began working on it because of a deeper matching child. + return bailoutOnAlreadyFinishedWork(current, workInProgress); } // There is no bailout on `children` equality because we expect people // to often pass a bound method as a child, but it may reference diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index c0aadacd1c0..dca54cbe441 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -580,6 +580,121 @@ describe('ReactNewContext', () => { expect(ReactNoop.getChildren()).toEqual([span('Foo: 3'), span('Bar: 3')]); }); + it('can skip parents with bitmask bailout while updating their children', () => { + const Context = React.createContext({foo: 0, bar: 0}, (a, b) => { + let result = 0; + if (a.foo !== b.foo) { + result |= 0b01; + } + if (a.bar !== b.bar) { + result |= 0b10; + } + return result; + }); + + function Provider(props) { + return ( + + {props.children} + + ); + } + + function Foo(props) { + return ( + + {value => { + ReactNoop.yield('Foo'); + return ( + + + {props.children} + + ); + }} + + ); + } + + function Bar(props) { + return ( + + {value => { + ReactNoop.yield('Bar'); + return ( + + + {props.children} + + ); + }} + + ); + } + + class Indirection extends React.Component { + shouldComponentUpdate() { + return false; + } + render() { + return this.props.children; + } + } + + function App(props) { + return ( + + + + + + + + + + + + + + ); + } + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Foo', 'Bar', 'Foo']); + expect(ReactNoop.getChildren()).toEqual([ + span('Foo: 1'), + span('Bar: 1'), + span('Foo: 1'), + ]); + + // Update only foo + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Foo', 'Foo']); + expect(ReactNoop.getChildren()).toEqual([ + span('Foo: 2'), + span('Bar: 1'), + span('Foo: 2'), + ]); + + // Update only bar + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Bar']); + expect(ReactNoop.getChildren()).toEqual([ + span('Foo: 2'), + span('Bar: 2'), + span('Foo: 2'), + ]); + + // Update both + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Foo', 'Bar', 'Foo']); + expect(ReactNoop.getChildren()).toEqual([ + span('Foo: 3'), + span('Bar: 3'), + span('Foo: 3'), + ]); + }); + it('warns if calculateChangedBits returns larger than a 31-bit integer', () => { spyOnDev(console, 'error'); From 3ea56b0e6e3cbdee0dfc7ee547b74fd0db10076e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 4 Apr 2018 19:11:37 +0100 Subject: [PATCH 2/2] Use a render prop in the test Without it, doesn't do anything because we bail out on constant element anyway. That's not what we're testing, and could be confusing. --- .../ReactNewContext-test.internal.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index dca54cbe441..8784622b7f9 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -608,7 +608,7 @@ describe('ReactNewContext', () => { return ( - {props.children} + {props.children && props.children()} ); }} @@ -624,7 +624,7 @@ describe('ReactNewContext', () => { return ( - {props.children} + {props.children && props.children()} ); }} @@ -646,13 +646,18 @@ describe('ReactNewContext', () => { - - - - - - - + {/* Use a render prop so we don't test constant elements. */} + {() => ( + + + {() => ( + + + + )} + + + )}