Skip to content

Commit

Permalink
Merge pull request #2853 from jsfb/warn-only-on-read
Browse files Browse the repository at this point in the history
Eliminate context warnings when component isn't reading the conflicting context variable
  • Loading branch information
jimfb committed Jan 20, 2015
2 parents c668676 + 8aa6171 commit f92967c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 53 deletions.
53 changes: 29 additions & 24 deletions src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,13 +457,13 @@ var ReactCompositeComponentMixin = assign({},

/**
* Filters the context object to only contain keys specified in
* `contextTypes`, and asserts that they are valid.
* `contextTypes`
*
* @param {object} context
* @return {?object}
* @private
*/
_processContext: function(context) {
_maskContext: function(context) {
var maskedContext = null;
var contextTypes = this._instance.constructor.contextTypes;
if (!contextTypes) {
Expand All @@ -473,6 +473,23 @@ var ReactCompositeComponentMixin = assign({},
for (var contextName in contextTypes) {
maskedContext[contextName] = context[contextName];
}
return maskedContext;
},

/**
* Filters the context object to only contain keys specified in
* `contextTypes`, and asserts that they are valid.
*
* @param {object} context
* @return {?object}
* @private
*/
_processContext: function(context) {
var maskedContext = this._maskContext(context);
var contextTypes = this._instance.constructor.contextTypes;
if (!contextTypes) {
return emptyObject;
}
if (__DEV__) {
this._checkPropTypes(
contextTypes,
Expand Down Expand Up @@ -648,35 +665,23 @@ var ReactCompositeComponentMixin = assign({},
* TODO: Remove this check when owner-context is removed
*/
_warnIfContextsDiffer: function(ownerBasedContext, parentBasedContext) {
ownerBasedContext = this._maskContext(ownerBasedContext);
parentBasedContext = this._maskContext(parentBasedContext);
var ownerKeys = Object.keys(ownerBasedContext).sort();
var parentKeys = Object.keys(parentBasedContext).sort();
var displayName = this.getName() || 'ReactCompositeComponent';
if (ownerKeys.length !== parentKeys.length ||
ownerKeys.toString() !== parentKeys.toString()) {
for (var i = 0; i < parentKeys.length; i++) {
var key = parentKeys[i];
warning(
ownerKeys.length === parentKeys.length &&
ownerKeys.toString() === parentKeys.toString(),
'owner based context (keys: %s) does not equal parent based ' +
'context (keys: %s) while mounting %s ' +
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)',
Object.keys(ownerBasedContext),
Object.keys(parentBasedContext),
ownerBasedContext[key],
parentBasedContext[key],
key,
displayName
);
} else {
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
);
}
}
},

Expand Down
36 changes: 7 additions & 29 deletions src/core/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,16 +581,11 @@ describe('ReactCompositeComponent', function() {

// 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.warn.mock.calls.length).toBe(2);
expect(console.warn.mock.calls.length).toBe(1);
expect(console.warn.mock.calls[0][0]).toBe(
'Warning: owner based context (keys: foo) does not equal parent based ' +
'context (keys: ) while mounting Component ' +
'(see: http://fb.me/react-context-by-parent)'
);
expect(console.warn.mock.calls[1][0]).toBe(
'Warning: owner based context (keys: foo) does not equal parent based ' +
'context (keys: ) while mounting ReactCompositeComponent ' +
'(see: http://fb.me/react-context-by-parent)'
'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)'
);

});
Expand Down Expand Up @@ -629,17 +624,12 @@ describe('ReactCompositeComponent', function() {

// 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.warn.mock.calls.length).toBe(2);
expect(console.warn.mock.calls.length).toBe(1);
expect(console.warn.mock.calls[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)'
);
expect(console.warn.mock.calls[1][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
'(see: http://fb.me/react-context-by-parent)'
);

});

Expand Down Expand Up @@ -686,18 +676,12 @@ describe('ReactCompositeComponent', function() {

// 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.warn.mock.calls.length).toBe(2);
expect(console.warn.mock.calls.length).toBe(1);
expect(console.warn.mock.calls[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)'
);
expect(console.warn.mock.calls[1][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
'(see: http://fb.me/react-context-by-parent)'
);

});

it('should warn if context values differ on update using wrapper', function() {
Expand Down Expand Up @@ -746,18 +730,12 @@ describe('ReactCompositeComponent', function() {

// 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.warn.mock.calls.length).toBe(2);
expect(console.warn.mock.calls.length).toBe(1);
expect(console.warn.mock.calls[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)'
);
expect(console.warn.mock.calls[1][0]).toBe(
'Warning: owner-based and parent-based contexts differ ' +
'(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
'(see: http://fb.me/react-context-by-parent)'
);

});

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

0 comments on commit f92967c

Please sign in to comment.