Skip to content

Commit

Permalink
Fix renderSubtreeIntoContainer to update context (#7125)
Browse files Browse the repository at this point in the history
* create failing test case

* Fix renderSubtreeIntoContainer to update context

Fixes #6599

* Also test re-rendering due to prop update

* Address review feedback

(cherry picked from commit 25f9f45)
  • Loading branch information
gaearon authored and zpao committed Jul 8, 2016
1 parent 32cadea commit 20d2398
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 11 deletions.
105 changes: 104 additions & 1 deletion src/addons/__tests__/renderSubtreeIntoContainer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
'use strict';

var React = require('React');
var ReactDOM = require('ReactDOM');
var ReactTestUtils = require('ReactTestUtils');
var renderSubtreeIntoContainer = require('renderSubtreeIntoContainer');

describe('renderSubtreeIntoContainer', function() {

it('should pass context when rendering subtree elsewhere', function() {

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

var Component = React.createClass({
Expand Down Expand Up @@ -92,4 +92,107 @@ describe('renderSubtreeIntoContainer', function() {
},
});
});

it('should update context if it changes due to setState', function() {
var container = document.createElement('div');
document.body.appendChild(container);
var portal = document.createElement('div');

var Component = React.createClass({
contextTypes: {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
},

render: function() {
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
},
});

var Parent = React.createClass({
childContextTypes: {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
},

getChildContext: function() {
return {
foo: this.state.bar,
getFoo: () => this.state.bar,
};
},

getInitialState: function() {
return {
bar: 'initial',
};
},

render: function() {
return null;
},

componentDidMount: function() {
renderSubtreeIntoContainer(this, <Component />, portal);
},

componentDidUpdate() {
renderSubtreeIntoContainer(this, <Component />, portal);
},
});

var instance = ReactDOM.render(<Parent />, container);
expect(portal.firstChild.innerHTML).toBe('initial-initial');
instance.setState({bar: 'changed'});
expect(portal.firstChild.innerHTML).toBe('changed-changed');
});

it('should update context if it changes due to re-render', function() {
var container = document.createElement('div');
document.body.appendChild(container);
var portal = document.createElement('div');

var Component = React.createClass({
contextTypes: {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
},

render: function() {
return <div>{this.context.foo + '-' + this.context.getFoo()}</div>;
},
});

var Parent = React.createClass({
childContextTypes: {
foo: React.PropTypes.string.isRequired,
getFoo: React.PropTypes.func.isRequired,
},

getChildContext: function() {
return {
foo: this.props.bar,
getFoo: () => this.props.bar,
};
},

render: function() {
return null;
},

componentDidMount: function() {
renderSubtreeIntoContainer(this, <Component />, portal);
},

componentDidUpdate() {
renderSubtreeIntoContainer(this, <Component />, portal);
},
});

ReactDOM.render(<Parent bar="initial" />, container);
expect(portal.firstChild.innerHTML).toBe('initial-initial');
ReactDOM.render(<Parent bar="changed" />, container);
expect(portal.firstChild.innerHTML).toBe('changed-changed');
});

});
21 changes: 14 additions & 7 deletions src/renderers/dom/client/ReactMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var ReactDOMContainerInfo = require('ReactDOMContainerInfo');
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
var ReactElement = require('ReactElement');
var ReactFeatureFlags = require('ReactFeatureFlags');
var ReactInstanceMap = require('ReactInstanceMap');
var ReactInstrumentation = require('ReactInstrumentation');
var ReactMarkupChecksum = require('ReactMarkupChecksum');
var ReactReconciler = require('ReactReconciler');
Expand Down Expand Up @@ -287,10 +288,11 @@ var ReactMount = {
_updateRootComponent: function(
prevComponent,
nextElement,
nextContext,
container,
callback) {
ReactMount.scrollMonitor(container, function() {
ReactUpdateQueue.enqueueElementInternal(prevComponent, nextElement);
ReactUpdateQueue.enqueueElementInternal(prevComponent, nextElement, nextContext);
if (callback) {
ReactUpdateQueue.enqueueCallbackInternal(prevComponent, callback);
}
Expand Down Expand Up @@ -389,7 +391,7 @@ var ReactMount = {
*/
renderSubtreeIntoContainer: function(parentComponent, nextElement, container, callback) {
invariant(
parentComponent != null && parentComponent._reactInternalInstance != null,
parentComponent != null && ReactInstanceMap.has(parentComponent),
'parentComponent must be a valid React Component'
);
return ReactMount._renderSubtreeIntoContainer(
Expand Down Expand Up @@ -440,6 +442,14 @@ var ReactMount = {
nextElement
);

var nextContext;
if (parentComponent) {
var parentInst = ReactInstanceMap.get(parentComponent);
nextContext = parentInst._processChildContext(parentInst._context);
} else {
nextContext = emptyObject;
}

var prevComponent = getTopLevelWrapperInContainer(container);

if (prevComponent) {
Expand All @@ -453,6 +463,7 @@ var ReactMount = {
ReactMount._updateRootComponent(
prevComponent,
nextWrappedElement,
nextContext,
container,
updatedCallback
);
Expand Down Expand Up @@ -501,11 +512,7 @@ var ReactMount = {
nextWrappedElement,
container,
shouldReuseMarkup,
parentComponent != null ?
parentComponent._reactInternalInstance._processChildContext(
parentComponent._reactInternalInstance._context
) :
emptyObject
nextContext
)._renderedComponent.getPublicInstance();
if (callback) {
callback.call(component);
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/native/ReactNativeMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ var ReactNativeMount = {
var prevWrappedElement = prevComponent._currentElement;
var prevElement = prevWrappedElement.props;
if (shouldUpdateReactComponent(prevElement, nextElement)) {
ReactUpdateQueue.enqueueElementInternal(prevComponent, nextWrappedElement);
ReactUpdateQueue.enqueueElementInternal(prevComponent, nextWrappedElement, emptyObject);
if (callback) {
ReactUpdateQueue.enqueueCallbackInternal(prevComponent, callback);
}
Expand Down
6 changes: 4 additions & 2 deletions src/renderers/shared/stack/reconciler/ReactUpdateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,10 @@ var ReactUpdateQueue = {
enqueueUpdate(internalInstance);
},

enqueueElementInternal: function(internalInstance, newElement) {
internalInstance._pendingElement = newElement;
enqueueElementInternal: function(internalInstance, nextElement, nextContext) {
internalInstance._pendingElement = nextElement;
// TODO: introduce _pendingContext instead of setting it directly.
internalInstance._context = nextContext;
enqueueUpdate(internalInstance);
},

Expand Down

0 comments on commit 20d2398

Please sign in to comment.