Skip to content

Commit

Permalink
Merge pull request #8590 from bvaughn/unexpected-context-pop
Browse files Browse the repository at this point in the history
`bailoutOnLowPriority` pushes context to mirror complete phase context pop
  • Loading branch information
bvaughn authored Dec 17, 2016
2 parents 70f704d + 441cc5c commit a5e7287
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* provides context when reusing work
* reads context when setState is below the provider
* reads context when setState is above the provider
* maintains the correct context index when context proviers are bailed out due to low priority

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
* catches render error in a boundary during full deferred mounting
Expand Down
13 changes: 11 additions & 2 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,17 @@ module.exports = function<T, P, I, TI, C, CX>(
}

function bailoutOnLowPriority(current, workInProgress) {
if (workInProgress.tag === HostPortal) {
pushHostContainer(workInProgress.stateNode.containerInfo);
// TODO: Handle HostComponent tags here as well and call pushHostContext()?
// See PR 8590 discussion for context
switch (workInProgress.tag) {
case ClassComponent:
if (isContextProvider(workInProgress)) {
pushContextProvider(workInProgress, false);
}
break;
case HostPortal:
pushHostContainer(workInProgress.stateNode.containerInfo);
break;
}
// TODO: What if this is currently in progress?
// How can that happen? How is this not being cloned?
Expand Down
43 changes: 43 additions & 0 deletions src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1932,4 +1932,47 @@ describe('ReactIncremental', () => {
'ShowLocaleFn:read {"locale":"gr"}',
]);
});

it('maintains the correct context index when context proviers are bailed out due to low priority', () => {
class Root extends React.Component {
render() {
return <Middle {...this.props} />;
}
}

let instance;

class Middle extends React.Component {
constructor(props, context) {
super(props, context);
instance = this;
}
shouldComponentUpdate() {
// Return false so that our child will get a NoWork priority (and get bailed out)
return false;
}
render() {
return <Child />;
}
}

// Child must be a context provider to trigger the bug
class Child extends React.Component {
static childContextTypes = {};
getChildContext() {
return {};
}
render() {
return <div />;
}
}

// Init
ReactNoop.render(<Root />);
ReactNoop.flush();

// Trigger an update in the middle of the tree
instance.setState({});
ReactNoop.flush();
});
});

0 comments on commit a5e7287

Please sign in to comment.