From a8ee43dc167053ce686bf427c5790a76917aa48e Mon Sep 17 00:00:00 2001 From: Daniel Berndt Date: Mon, 30 May 2016 20:11:34 +0200 Subject: [PATCH 1/4] create failing test case --- .../renderSubtreeIntoContainer-test.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/addons/__tests__/renderSubtreeIntoContainer-test.js b/src/addons/__tests__/renderSubtreeIntoContainer-test.js index 7898af3d7fba8..e57ddae291412 100644 --- a/src/addons/__tests__/renderSubtreeIntoContainer-test.js +++ b/src/addons/__tests__/renderSubtreeIntoContainer-test.js @@ -12,6 +12,7 @@ 'use strict'; var React = require('React'); +var ReactDOM = require('ReactDOM'); var ReactTestUtils = require('ReactTestUtils'); var renderSubtreeIntoContainer = require('renderSubtreeIntoContainer'); @@ -92,4 +93,61 @@ describe('renderSubtreeIntoContainer', function() { }, }); }); + + it('should pass updated context if context changes', 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
{this.context.foo + '-' + this.context.getFoo()}
; + }, + }); + + var Parent = React.createClass({ + childContextTypes: { + foo: React.PropTypes.string.isRequired, + getFoo: React.PropTypes.func.isRequired, + }, + + getChildContext: function() { + return { + foo: this.state.bar, + getFoo: function() { + return this.state.bar; + }.bind(this), + }; + }, + + getInitialState: function() { + return { + bar: 'initial', + }; + }, + + render: function() { + return null; + }, + + componentDidMount: function() { + renderSubtreeIntoContainer(this, , portal); + }, + + componentDidUpdate() { + renderSubtreeIntoContainer(this, , portal); + }, + }); + + var instance = ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('initial-initial'); + instance.setState({bar: 'changed'}); + expect(portal.firstChild.innerHTML).toBe('changed-changed'); + }); }); From c46541b24787c98bb6bf7411621b0a04a2f01598 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 27 Jun 2016 01:12:22 +0100 Subject: [PATCH 2/4] Fix renderSubtreeIntoContainer to update context Fixes #6599 --- .../__tests__/renderSubtreeIntoContainer-test.js | 1 - src/renderers/dom/client/ReactMount.js | 14 ++++++++------ src/renderers/native/ReactNativeMount.js | 2 +- .../shared/stack/reconciler/ReactUpdateQueue.js | 5 +++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/addons/__tests__/renderSubtreeIntoContainer-test.js b/src/addons/__tests__/renderSubtreeIntoContainer-test.js index e57ddae291412..35841775a5699 100644 --- a/src/addons/__tests__/renderSubtreeIntoContainer-test.js +++ b/src/addons/__tests__/renderSubtreeIntoContainer-test.js @@ -95,7 +95,6 @@ describe('renderSubtreeIntoContainer', function() { }); it('should pass updated context if context changes', function() { - var container = document.createElement('div'); document.body.appendChild(container); var portal = document.createElement('div'); diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 5e1827b746519..2021a99cec83e 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -287,10 +287,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); } @@ -439,6 +440,10 @@ var ReactMount = { null, nextElement ); + var nextContext = parentComponent ? + parentComponent._reactInternalInstance._processChildContext( + parentComponent._reactInternalInstance._context + ) : emptyObject; var prevComponent = getTopLevelWrapperInContainer(container); @@ -453,6 +458,7 @@ var ReactMount = { ReactMount._updateRootComponent( prevComponent, nextWrappedElement, + nextContext, container, updatedCallback ); @@ -501,11 +507,7 @@ var ReactMount = { nextWrappedElement, container, shouldReuseMarkup, - parentComponent != null ? - parentComponent._reactInternalInstance._processChildContext( - parentComponent._reactInternalInstance._context - ) : - emptyObject + nextContext )._renderedComponent.getPublicInstance(); if (callback) { callback.call(component); diff --git a/src/renderers/native/ReactNativeMount.js b/src/renderers/native/ReactNativeMount.js index 8fc575d97f930..d9fe2f28623fe 100644 --- a/src/renderers/native/ReactNativeMount.js +++ b/src/renderers/native/ReactNativeMount.js @@ -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); } diff --git a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js index 94369f43348da..10cbabb5e149b 100644 --- a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js @@ -246,8 +246,9 @@ var ReactUpdateQueue = { enqueueUpdate(internalInstance); }, - enqueueElementInternal: function(internalInstance, newElement) { - internalInstance._pendingElement = newElement; + enqueueElementInternal: function(internalInstance, nextElement, nextContext) { + internalInstance._pendingElement = nextElement; + internalInstance._context = nextContext; enqueueUpdate(internalInstance); }, From dc8d34d83c6c2a275826f8f5bee18e1f419423fd Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 27 Jun 2016 01:33:42 +0100 Subject: [PATCH 3/4] Also test re-rendering due to prop update --- .../renderSubtreeIntoContainer-test.js | 58 +++++++++++++++++-- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/src/addons/__tests__/renderSubtreeIntoContainer-test.js b/src/addons/__tests__/renderSubtreeIntoContainer-test.js index 35841775a5699..dcb70481a7fe6 100644 --- a/src/addons/__tests__/renderSubtreeIntoContainer-test.js +++ b/src/addons/__tests__/renderSubtreeIntoContainer-test.js @@ -19,7 +19,6 @@ 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({ @@ -94,7 +93,7 @@ describe('renderSubtreeIntoContainer', function() { }); }); - it('should pass updated context if context changes', 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'); @@ -119,9 +118,7 @@ describe('renderSubtreeIntoContainer', function() { getChildContext: function() { return { foo: this.state.bar, - getFoo: function() { - return this.state.bar; - }.bind(this), + getFoo: () => this.state.bar, }; }, @@ -144,9 +141,58 @@ describe('renderSubtreeIntoContainer', function() { }, }); - var instance = ReactDOM.render(, container); + var instance = ReactDOM.render(, 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
{this.context.foo + '-' + this.context.getFoo()}
; + }, + }); + + 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, , portal); + }, + + componentDidUpdate() { + renderSubtreeIntoContainer(this, , portal); + }, + }); + + ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('initial-initial'); + ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('changed-changed'); + }); + }); From a38c1b9a6e33ae49e9f591b19e114c50d928937e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 27 Jun 2016 21:37:23 +0100 Subject: [PATCH 4/4] Address review feedback --- src/renderers/dom/client/ReactMount.js | 15 ++++++++++----- .../shared/stack/reconciler/ReactUpdateQueue.js | 1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 2021a99cec83e..d7fb1b561f835 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -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'); @@ -390,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( @@ -440,10 +441,14 @@ var ReactMount = { null, nextElement ); - var nextContext = parentComponent ? - parentComponent._reactInternalInstance._processChildContext( - parentComponent._reactInternalInstance._context - ) : emptyObject; + + var nextContext; + if (parentComponent) { + var parentInst = ReactInstanceMap.get(parentComponent); + nextContext = parentInst._processChildContext(parentInst._context); + } else { + nextContext = emptyObject; + } var prevComponent = getTopLevelWrapperInContainer(container); diff --git a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js index 10cbabb5e149b..9d4071cc3ffe9 100644 --- a/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/stack/reconciler/ReactUpdateQueue.js @@ -248,6 +248,7 @@ var ReactUpdateQueue = { enqueueElementInternal: function(internalInstance, nextElement, nextContext) { internalInstance._pendingElement = nextElement; + // TODO: introduce _pendingContext instead of setting it directly. internalInstance._context = nextContext; enqueueUpdate(internalInstance); },