From 998c22d4904e298a2b8d86421bee137a07d30079 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 4 Apr 2018 19:44:14 +0100 Subject: [PATCH] Don't render bitmask-bailing consumers even if there's a deeper matching child (#12543) * Don't render consumers that bailed out with bitmask even if there's a deeper matching child * 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. --- .../src/ReactFiberBeginWork.js | 4 + .../ReactNewContext-test.internal.js | 120 ++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index e8d69502062d3..6de5e54877ec3 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 c0aadacd1c030..8784622b7f973 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -580,6 +580,126 @@ 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 && props.children()} + + ); + }} + + ); + } + + function Bar(props) { + return ( + + {value => { + ReactNoop.yield('Bar'); + return ( + + + {props.children && props.children()} + + ); + }} + + ); + } + + class Indirection extends React.Component { + shouldComponentUpdate() { + return false; + } + render() { + return this.props.children; + } + } + + function App(props) { + return ( + + + + {/* Use a render prop so we don't test constant elements. */} + {() => ( + + + {() => ( + + + + )} + + + )} + + + + ); + } + + 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');