Skip to content

Commit

Permalink
Don't render bitmask-bailing consumers even if there's a deeper match…
Browse files Browse the repository at this point in the history
…ing child (facebook#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, <Indirection> doesn't do anything because we bail out on constant element anyway.
That's not what we're testing, and could be confusing.
  • Loading branch information
gaearon authored and rhagigi committed Apr 19, 2018
1 parent a2530a1 commit 998c22d
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,10 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Context.Provider value={{foo: props.foo, bar: props.bar}}>
{props.children}
</Context.Provider>
);
}

function Foo(props) {
return (
<Context.Consumer unstable_observedBits={0b01}>
{value => {
ReactNoop.yield('Foo');
return (
<React.Fragment>
<span prop={'Foo: ' + value.foo} />
{props.children && props.children()}
</React.Fragment>
);
}}
</Context.Consumer>
);
}

function Bar(props) {
return (
<Context.Consumer unstable_observedBits={0b10}>
{value => {
ReactNoop.yield('Bar');
return (
<React.Fragment>
<span prop={'Bar: ' + value.bar} />
{props.children && props.children()}
</React.Fragment>
);
}}
</Context.Consumer>
);
}

class Indirection extends React.Component {
shouldComponentUpdate() {
return false;
}
render() {
return this.props.children;
}
}

function App(props) {
return (
<Provider foo={props.foo} bar={props.bar}>
<Indirection>
<Foo>
{/* Use a render prop so we don't test constant elements. */}
{() => (
<Indirection>
<Bar>
{() => (
<Indirection>
<Foo />
</Indirection>
)}
</Bar>
</Indirection>
)}
</Foo>
</Indirection>
</Provider>
);
}

ReactNoop.render(<App foo={1} bar={1} />);
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(<App foo={2} bar={1} />);
expect(ReactNoop.flush()).toEqual(['Foo', 'Foo']);
expect(ReactNoop.getChildren()).toEqual([
span('Foo: 2'),
span('Bar: 1'),
span('Foo: 2'),
]);

// Update only bar
ReactNoop.render(<App foo={2} bar={2} />);
expect(ReactNoop.flush()).toEqual(['Bar']);
expect(ReactNoop.getChildren()).toEqual([
span('Foo: 2'),
span('Bar: 2'),
span('Foo: 2'),
]);

// Update both
ReactNoop.render(<App foo={3} bar={3} />);
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');

Expand Down

0 comments on commit 998c22d

Please sign in to comment.