Skip to content

Commit

Permalink
Merge pull request #8706 from bvaughn/dont-recreate-masked-context-un…
Browse files Browse the repository at this point in the history
…less-needed

Dont recreate maked context unless unmasked context changes
  • Loading branch information
bvaughn authored Jan 9, 2017
2 parents fa4f79f + a682059 commit 67c16e3
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 9 deletions.
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* reads context when setState is above the provider
* maintains the correct context when providers bail out due to low priority
* maintains the correct context when unwinding due to an error in render
* should not recreate masked context unless inputs have changed

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
* catches render error in a boundary during full deferred mounting
Expand Down
7 changes: 5 additions & 2 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var {
var ReactTypeOfWork = require('ReactTypeOfWork');
var {
getMaskedContext,
getUnmaskedContext,
hasContextChanged,
pushContextProvider,
pushTopLevelContextObject,
Expand Down Expand Up @@ -210,7 +211,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}

var context = getMaskedContext(workInProgress);
var unmaskedContext = getUnmaskedContext(workInProgress);
var context = getMaskedContext(workInProgress, unmaskedContext);

var nextChildren;

Expand Down Expand Up @@ -427,7 +429,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
}
var fn = workInProgress.type;
var props = workInProgress.pendingProps;
var context = getMaskedContext(workInProgress);
var unmaskedContext = getUnmaskedContext(workInProgress);
var context = getMaskedContext(workInProgress, unmaskedContext);

var value;

Expand Down
22 changes: 17 additions & 5 deletions src/renderers/shared/fiber/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { PriorityLevel } from 'ReactPriorityLevel';

var {
getMaskedContext,
getUnmaskedContext,
} = require('ReactFiberContext');
var {
addUpdate,
Expand Down Expand Up @@ -204,10 +205,17 @@ module.exports = function(
function constructClassInstance(workInProgress : Fiber) : any {
const ctor = workInProgress.type;
const props = workInProgress.pendingProps;
const context = getMaskedContext(workInProgress);
const unmaskedContext = getUnmaskedContext(workInProgress);
const context = getMaskedContext(workInProgress, unmaskedContext);
const instance = new ctor(props, context);
adoptClassInstance(workInProgress, instance);
checkClassInstance(workInProgress);

// Cache unmasked context so we can avoid recreating masked context unless necessary.
// ReactFiberContext usually updates this cache but can't for newly-created instances.
instance.__reactInternalMemoizedUnmaskedChildContext = unmaskedContext;
instance.__reactInternalMemoizedMaskedChildContext = context;

return instance;
}

Expand All @@ -221,9 +229,11 @@ module.exports = function(
throw new Error('There must be pending props for an initial mount.');
}

const unmaskedContext = getUnmaskedContext(workInProgress);

instance.props = props;
instance.state = state;
instance.context = getMaskedContext(workInProgress);
instance.context = getMaskedContext(workInProgress, unmaskedContext);

if (typeof instance.componentWillMount === 'function') {
instance.componentWillMount();
Expand Down Expand Up @@ -256,7 +266,8 @@ module.exports = function(
throw new Error('There should always be pending or memoized props.');
}
}
const newContext = getMaskedContext(workInProgress);
const newUnmaskedContext = getUnmaskedContext(workInProgress);
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);

// TODO: Should we deal with a setState that happened after the last
// componentWillMount and before this componentWillMount? Probably
Expand All @@ -277,7 +288,7 @@ module.exports = function(
const newInstance = constructClassInstance(workInProgress);
newInstance.props = newProps;
newInstance.state = newState = newInstance.state || null;
newInstance.context = getMaskedContext(workInProgress);
newInstance.context = newContext;

if (typeof newInstance.componentWillMount === 'function') {
newInstance.componentWillMount();
Expand Down Expand Up @@ -314,7 +325,8 @@ module.exports = function(
}
}
const oldContext = instance.context;
const newContext = getMaskedContext(workInProgress);
const newUnmaskedContext = getUnmaskedContext(workInProgress);
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);

// Note: During these life-cycles, instance.props/instance.state are what
// ever the previously attempted to render - not the "current". However,
Expand Down
21 changes: 19 additions & 2 deletions src/renderers/shared/fiber/ReactFiberContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,26 @@ function getUnmaskedContext(workInProgress : Fiber) : Object {
}
return contextStackCursor.current;
}
exports.getUnmaskedContext = getUnmaskedContext;

exports.getMaskedContext = function(workInProgress : Fiber) {
exports.getMaskedContext = function(workInProgress : Fiber, unmaskedContext : Object) {
const type = workInProgress.type;
const contextTypes = type.contextTypes;
if (!contextTypes) {
return emptyObject;
}

const unmaskedContext = getUnmaskedContext(workInProgress);
// Avoid recreating masked context unless unmasked context has changed.
// Failing to do this will result in unnecessary calls to componentWillReceiveProps.
// This may trigger infinite loops if componentWillReceiveProps calls setState.
const instance = workInProgress.stateNode;
if (
instance &&
instance.__reactInternalMemoizedUnmaskedChildContext === unmaskedContext
) {
return instance.__reactInternalMemoizedMaskedChildContext;
}

const context = {};
for (let key in contextTypes) {
context[key] = unmaskedContext[key];
Expand All @@ -74,6 +85,12 @@ exports.getMaskedContext = function(workInProgress : Fiber) {
checkReactTypeSpec(contextTypes, context, 'context', name, null, workInProgress);
}

// Cache unmasked context so we can avoid recreating masked context unless necessary.
if (instance) {
instance.__reactInternalMemoizedUnmaskedChildContext = unmaskedContext;
instance.__reactInternalMemoizedMaskedChildContext = context;
}

return context;
};

Expand Down
46 changes: 46 additions & 0 deletions src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2021,4 +2021,50 @@ describe('ReactIncremental', () => {
});
ReactNoop.flush();
});

it('should not recreate masked context unless inputs have changed', () => {
const ops = [];

let scuCounter = 0;

class MyComponent extends React.Component {
static contextTypes = {};
componentDidMount(prevProps, prevState) {
ops.push('componentDidMount');
this.setState({ setStateInCDU: true });
}
componentDidUpdate(prevProps, prevState) {
ops.push('componentDidUpdate');
if (this.state.setStateInCDU) {
this.setState({ setStateInCDU: false });
}
}
componentWillReceiveProps(nextProps) {
ops.push('componentWillReceiveProps');
this.setState({ setStateInCDU: true });
}
render() {
ops.push('render');
return null;
}
shouldComponentUpdate(nextProps, nextState) {
ops.push('shouldComponentUpdate');
return scuCounter++ < 5; // Don't let test hang
}
}

ReactNoop.render(<MyComponent />);
ReactNoop.flush();

expect(ops).toEqual([
'render',
'componentDidMount',
'shouldComponentUpdate',
'render',
'componentDidUpdate',
'shouldComponentUpdate',
'render',
'componentDidUpdate',
]);
});
});

0 comments on commit 67c16e3

Please sign in to comment.