From 7d4491753158afc839f2934367ad2f7878ff7283 Mon Sep 17 00:00:00 2001 From: Jim Date: Tue, 7 Apr 2015 14:02:18 -0700 Subject: [PATCH] Switch to parent-based context. Fixes #2112. --- src/classic/element/ReactElement.js | 4 - .../element/__tests__/ReactElement-test.js | 24 -- src/core/ReactCompositeComponent.js | 41 +--- src/core/ReactContext.js | 44 +--- .../__tests__/ReactCompositeComponent-test.js | 210 +++--------------- 5 files changed, 36 insertions(+), 287 deletions(-) diff --git a/src/classic/element/ReactElement.js b/src/classic/element/ReactElement.js index bfc18a0dabbb9..4760c6311d370 100644 --- a/src/classic/element/ReactElement.js +++ b/src/classic/element/ReactElement.js @@ -99,10 +99,6 @@ var ReactElement = function(type, key, ref, owner, context, props) { // Record the component responsible for creating this element. this._owner = owner; - // TODO: Deprecate withContext, and then the context becomes accessible - // through the owner. - this._context = context; - if (__DEV__) { // The validation flag and props are currently mutative. We put them on // an external backing store so that we can freeze the whole object. diff --git a/src/classic/element/__tests__/ReactElement-test.js b/src/classic/element/__tests__/ReactElement-test.js index 526a0117ec55a..946158c26eed6 100644 --- a/src/classic/element/__tests__/ReactElement-test.js +++ b/src/classic/element/__tests__/ReactElement-test.js @@ -86,30 +86,6 @@ describe('ReactElement', function() { expect(element.props).toEqual({foo:'56'}); }); - it('preserves the legacy context on the element', function() { - var Component = React.createFactory(ComponentClass); - var element; - - var Wrapper = React.createClass({ - childContextTypes: { - foo: React.PropTypes.string - }, - getChildContext: function() { - return {foo: 'bar'}; - }, - render: function() { - element = Component(); - return element; - } - }); - - ReactTestUtils.renderIntoDocument( - React.createElement(Wrapper) - ); - - expect(element._context).toEqual({foo: 'bar'}); - }); - it('preserves the owner on the element', function() { var Component = React.createFactory(ComponentClass); var element; diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 15afc9952314d..e1f63ffa6a0df 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -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 @@ -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 @@ -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 @@ -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. diff --git a/src/core/ReactContext.js b/src/core/ReactContext.js index 21d8aaa2187c5..75cc7c4d0d438 100644 --- a/src/core/ReactContext.js +++ b/src/core/ReactContext.js @@ -11,11 +11,7 @@ 'use strict'; -var assign = require('Object.assign'); var emptyObject = require('emptyObject'); -var warning = require('warning'); - -var didWarn = false; /** * Keeps track of the current context. @@ -29,45 +25,7 @@ var ReactContext = { * @internal * @type {object} */ - current: emptyObject, - - /** - * Temporarily extends the current context while executing scopedCallback. - * - * A typical use case might look like - * - * render: function() { - * var children = ReactContext.withContext({foo: 'foo'}, () => ( - * - * )); - * return
{children}
; - * } - * - * @param {object} newContext New context to merge into the existing context - * @param {function} scopedCallback Callback to run with the new context - * @return {ReactComponent|array} - */ - withContext: function(newContext, scopedCallback) { - if (__DEV__) { - warning( - didWarn, - 'withContext is deprecated and will be removed in a future version. ' + - 'Use a wrapper component with getChildContext instead.' - ); - - didWarn = true; - } - - var result; - var previousContext = ReactContext.current; - ReactContext.current = assign({}, previousContext, newContext); - try { - result = scopedCallback(); - } finally { - ReactContext.current = previousContext; - } - return result; - } + current: emptyObject }; diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 3b4d3e6ff8e9e..d73fd0332eb3e 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -73,12 +73,7 @@ describe('ReactCompositeComponent', function() { } }); - // Ignore the first warning which is fired by using withContext at all. - // That way we don't have to reset and assert it on every subsequent test. - // This will be killed soon anyway. console.error = mocks.getMockFunction(); - React.withContext({}, function() { }); - spyOn(console, 'error'); }); @@ -621,178 +616,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
; - } - }); - - React.withContext({foo: 'bar'}, function() { - ReactTestUtils.renderIntoDocument(); - }); - - 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
{this.props.children}
; - } - }); - var Component = React.createClass({ - contextTypes: { - foo: ReactPropTypes.string.isRequired - }, - - render: function() { - return
; - } - }); - - var component = React.withContext({foo: 'noise'}, function() { - return ; - }); - - ReactTestUtils.renderIntoDocument({component}); - - // 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
{this.props.children}
; - } - }); - - var Component = React.createClass({ - contextTypes: { - foo: ReactPropTypes.string.isRequired - }, - - render: function() { - return
; - } - }); - - var div = document.createElement('div'); - - var componentWithSameContext = React.withContext({foo: 'bar'}, function() { - return ; - }); - React.render({componentWithSameContext}, div); - - expect(console.error.argsForCall.length).toBe(0); - - var componentWithDifferentContext = React.withContext({foo: 'noise'}, function() { - return ; - }); - React.render({componentWithDifferentContext}, 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
{this.props.children}
; - } - }); - - var Component = React.createClass({ - contextTypes: { - foo: ReactPropTypes.string.isRequired - }, - - render: function() { - return
; - } - }); - - var Wrapper = React.createClass({ - childContextTypes: { - foo: ReactPropTypes.string - }, - - getChildContext: function() { - return {foo: this.props.foo}; - }, - - render: function() { return ; } - - }); - - var div = document.createElement('div'); - React.render(, div); - React.render(, 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({ @@ -996,4 +819,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
{this.props.children}
; + } + }); + + var Component = React.createClass({ + contextTypes: { + foo: ReactPropTypes.string.isRequired + }, + + render: function() { + return
; + } + }); + + var div = document.createElement('div'); + React.render(, div); + + expect(console.error.argsForCall.length).toBe(0); + }); + });