Skip to content

Commit

Permalink
Switch to parent-based context. Fixes #2112.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimfb
jimfb committed Apr 7, 2015
1 parent ddbbaa9 commit a51686b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 211 deletions.
41 changes: 2 additions & 39 deletions src/core/ReactCompositeComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ var ReactCompositeComponentMixin = {
this._rootNodeID = rootID;

var publicProps = this._processProps(this._currentElement.props);
var publicContext = this._processContext(this._currentElement._context);
var publicContext = this._processContext(context);

var Component = ReactNativeComponent.getComponentClassForElement(
this._currentElement
Expand Down Expand Up @@ -158,10 +158,6 @@ var ReactCompositeComponentMixin = {
// Store a reference from the instance back to the internal representation
ReactInstanceMap.set(inst, this);

if (__DEV__) {
this._warnIfContextsDiffer(this._currentElement._context, context);
}

if (__DEV__) {
// Since plain JS classes are defined without any special initialization
// logic, we can not catch common errors early. Therefore, we have to
Expand Down Expand Up @@ -529,30 +525,6 @@ var ReactCompositeComponentMixin = {
}
},

/**
* Compare two contexts, warning if they are different
* TODO: Remove this check when owner-context is removed
*/
_warnIfContextsDiffer: function(ownerBasedContext, parentBasedContext) {
ownerBasedContext = this._maskContext(ownerBasedContext);
parentBasedContext = this._maskContext(parentBasedContext);
var parentKeys = Object.keys(parentBasedContext).sort();
var displayName = this.getName() || 'ReactCompositeComponent';
for (var i = 0; i < parentKeys.length; i++) {
var key = parentKeys[i];
warning(
ownerBasedContext[key] === parentBasedContext[key],
'owner-based and parent-based contexts differ ' +
'(values: `%s` vs `%s`) for key (%s) while mounting %s ' +
'(see: http://fb.me/react-context-by-parent)',
ownerBasedContext[key],
parentBasedContext[key],
key,
displayName
);
}
},

/**
* Perform an update to a mounted component. The componentWillReceiveProps and
* shouldComponentUpdate methods are called, then (assuming the update isn't
Expand Down Expand Up @@ -582,18 +554,9 @@ var ReactCompositeComponentMixin = {

// Distinguish between a props update versus a simple state update
if (prevParentElement !== nextParentElement) {
nextContext = this._processContext(nextParentElement._context);
nextContext = this._processContext(nextUnmaskedContext);
nextProps = this._processProps(nextParentElement.props);

if (__DEV__) {
if (nextUnmaskedContext != null) {
this._warnIfContextsDiffer(
nextParentElement._context,
nextUnmaskedContext
);
}
}

// An update here will schedule an update but immediately set
// _pendingStateQueue which will ensure that any state updates gets
// immediately reconciled instead of waiting for the next batch.
Expand Down
205 changes: 33 additions & 172 deletions src/core/__tests__/ReactCompositeComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,178 +621,6 @@ describe('ReactCompositeComponent', function() {
reactComponentExpect(childInstance).scalarContextEqual({foo: 'bar', depth: 0});
});

it('warn if context keys differ', function() {
var Component = React.createClass({
contextTypes: {
foo: ReactPropTypes.string.isRequired
},

render: function() {
return <div />;
}
});

React.withContext({foo: 'bar'}, function() {
ReactTestUtils.renderIntoDocument(<Component />);
});

expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `bar` vs `undefined`) for key (foo) ' +
'while mounting Component (see: http://fb.me/react-context-by-parent)'
);

});

it('warn if context values differ', function() {
var Parent = React.createClass({
childContextTypes: {
foo: ReactPropTypes.string
},

getChildContext: function() {
return {
foo: "bar"
};
},

render: function() {
return <div>{this.props.children}</div>;
}
});
var Component = React.createClass({
contextTypes: {
foo: ReactPropTypes.string.isRequired
},

render: function() {
return <div />;
}
});

var component = React.withContext({foo: 'noise'}, function() {
return <Component />;
});

ReactTestUtils.renderIntoDocument(<Parent>{component}</Parent>);

// Two warnings, one for the component and one for the div
// We may want to make this expect one warning in the future
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
'(see: http://fb.me/react-context-by-parent)'
);

});

it('should warn if context values differ on update using withContext', function() {
var Parent = React.createClass({
childContextTypes: {
foo: ReactPropTypes.string
},

getChildContext: function() {
return {
foo: "bar"
};
},

render: function() {
return <div>{this.props.children}</div>;
}
});

var Component = React.createClass({
contextTypes: {
foo: ReactPropTypes.string.isRequired
},

render: function() {
return <div />;
}
});

var div = document.createElement('div');

var componentWithSameContext = React.withContext({foo: 'bar'}, function() {
return <Component />;
});
React.render(<Parent>{componentWithSameContext}</Parent>, div);

expect(console.error.argsForCall.length).toBe(0);

var componentWithDifferentContext = React.withContext({foo: 'noise'}, function() {
return <Component />;
});
React.render(<Parent>{componentWithDifferentContext}</Parent>, div);

// Two warnings, one for the component and one for the div
// We may want to make this expect one warning in the future
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
'(see: http://fb.me/react-context-by-parent)'
);
});

it('should warn if context values differ on update using wrapper', function() {
var Parent = React.createClass({
childContextTypes: {
foo: ReactPropTypes.string
},

getChildContext: function() {
return {
foo: "bar"
};
},

render: function() {
return <div>{this.props.children}</div>;
}
});

var Component = React.createClass({
contextTypes: {
foo: ReactPropTypes.string.isRequired
},

render: function() {
return <div />;
}
});

var Wrapper = React.createClass({
childContextTypes: {
foo: ReactPropTypes.string
},

getChildContext: function() {
return {foo: this.props.foo};
},

render: function() { return <Parent><Component /></Parent>; }

});

var div = document.createElement('div');
React.render(<Wrapper foo='bar' />, div);
React.render(<Wrapper foo='noise' />, div);

// Two warnings, one for the component and one for the div
// We may want to make this expect one warning in the future
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
'(see: http://fb.me/react-context-by-parent)'
);
});

it('unmasked context propagates through updates', function() {

var Leaf = React.createClass({
Expand Down Expand Up @@ -996,4 +824,37 @@ describe('ReactCompositeComponent', function() {
);
});

it('context should be passed down from the parent', function() {
var Parent = React.createClass({
childContextTypes: {
foo: ReactPropTypes.string
},

getChildContext: function() {
return {
foo: "bar"
};
},

render: function() {
return <div>{this.props.children}</div>;
}
});

var Component = React.createClass({
contextTypes: {
foo: ReactPropTypes.string.isRequired
},

render: function() {
return <div />;
}
});

var div = document.createElement('div');
React.render(<Parent><Component /></Parent>, div);

expect(console.error.argsForCall.length).toBe(0);
});

});

0 comments on commit a51686b

Please sign in to comment.