Skip to content

Commit

Permalink
Inherit purity for functional components
Browse files Browse the repository at this point in the history
We've heard clearly that most React users intend for their functional components to be pure and to produce different output only if the component's props have changed (https://mobile.twitter.com/reactjs/status/736412808372314114). However, a significant fraction of users still rely on mutation in their apps; when used with mutation, comparing props on each render could lead to components not updating even when the data is changed.

Therefore, we're changing functional components to behave as pure when they're used inside a React.PureComponent but to rerender unconditionally when contained in a React.Component:

```js
class Post extends React.PureComponent {  // or React.Component
  render() {
    return (
      <div className="post">
        <PostHeader model={this.props.model} />
        <PostBody model={this.props.model} />
      </div>
    );
  }
}

function PostHeader(props) {
  // ...
}

function PostBody(props) {
  // ...
}
```

In this example, the functional components PostHeader and PostBody will be treated as pure because they're rendered by a pure parent component (Post). If our app used mutable models instead, Post should extend React.Component, which would cause PostHeader and PostBody to rerender whenever Post does, even if the model object is the same.

We anticipate that this behavior will work well in real-world apps: if you use immutable data, your class-based components can extend React.PureComponent and your functional components will be pure too; if you use mutable data, your class-based components will extend React.Component and your functional components will update accordingly.

In the future, we might adjust these heuristics to improve performance. For example, we might do runtime detection of components like

```js
function FancyButton(props) {
  return <Button style="fancy" text={props.text} />;
}
```

and optimize them to "inline" the child Button component and call it immediately, so that React doesn't need to store the props for Button nor allocate a backing instance for it -- causing less work to be performed and reducing GC pressure.
  • Loading branch information
sophiebits committed Jun 1, 2016
1 parent fdb84f4 commit 0af8c44
Show file tree
Hide file tree
Showing 12 changed files with 223 additions and 37 deletions.
109 changes: 109 additions & 0 deletions src/isomorphic/modern/class/__tests__/ReactPureComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,113 @@ describe('ReactPureComponent', function() {
expect(renders).toBe(2);
});

it('does not update functional components inside pure components', function() {
// Multiple levels of host components and functional components; make sure
// purity propagates down. So we render:
//
// <Impure>
// <Functional>
// <Functional>
// <Pure>
// <Functional>
// <Functional>
//
// with some host wrappers in between. The render code is a little
// convoluted because we want to make the props scalar-equal as long as
// `text` (threaded through the whole tree) is. The outer two Functional
// components should always rerender; the inner Functional components should
// only rerender if `text` changes to a different object.

var impureRenders = 0;
var pureRenders = 0;
var functionalRenders = 0;

var pureComponent;
class Impure extends React.Component {
render() {
impureRenders++;
return (
<div>
{/* These props will always be shallow-equal. */}
<Functional
depth={2}
thenRender="pureComponent"
text={this.props.text}
/>
</div>
);
}
}
class Pure extends React.PureComponent {
render() {
pureComponent = this;
pureRenders++;
return (
<div>
<Functional
depth={2}
thenRender="text"
text={this.props.text}
/>
</div>
);
}
}
function Functional(props) {
functionalRenders++;
if (props.depth <= 1) {
return (
<div>
{props.prefix}
{props.thenRender === 'pureComponent' ?
[props.text[0] + '/', <Pure key="pure" text={props.text} />] :
props.text[0]}
</div>
);
} else {
return (
<div>
<Functional
{...props}
depth={props.depth - 1}
/>
</div>
);
}
}

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

text = ['porcini'];
ReactDOM.render(<Impure text={text} />, container);
expect(container.textContent).toBe('porcini/porcini');
expect(impureRenders).toBe(1);
expect(pureRenders).toBe(1);
expect(functionalRenders).toBe(4);

text = ['morel'];
ReactDOM.render(<Impure text={text} />, container);
expect(container.textContent).toBe('morel/morel');
expect(impureRenders).toBe(2);
expect(pureRenders).toBe(2);
expect(functionalRenders).toBe(8);

text[0] = 'portobello';
ReactDOM.render(<Impure text={text} />, container);
// Updates happen down and stop at the pure component
expect(container.textContent).toBe('portobello/morel');
expect(impureRenders).toBe(3);
expect(pureRenders).toBe(2);
expect(functionalRenders).toBe(10);

// Forcing the pure component to update makes it rerender, but its
// functional children still don't.
pureComponent.forceUpdate();
expect(container.textContent).toBe('portobello/morel');
expect(impureRenders).toBe(3);
expect(pureRenders).toBe(3);
expect(functionalRenders).toBe(10);
});

});
33 changes: 26 additions & 7 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -846,10 +846,16 @@ ReactDOMComponent.Mixin = {
* @param {ReactReconcileTransaction|ReactServerRenderingTransaction} transaction
* @param {object} context
*/
receiveComponent: function(nextElement, transaction, context) {
receiveComponent: function(nextElement, transaction, context, isParentPure) {
var prevElement = this._currentElement;
this._currentElement = nextElement;
this.updateComponent(transaction, prevElement, nextElement, context);
this.updateComponent(
transaction,
prevElement,
nextElement,
context,
isParentPure
);
},

/**
Expand All @@ -862,7 +868,13 @@ ReactDOMComponent.Mixin = {
* @internal
* @overridable
*/
updateComponent: function(transaction, prevElement, nextElement, context) {
updateComponent: function(
transaction,
prevElement,
nextElement,
context,
isParentPure
) {
var lastProps = prevElement.props;
var nextProps = this._currentElement.props;

Expand Down Expand Up @@ -897,7 +909,8 @@ ReactDOMComponent.Mixin = {
lastProps,
nextProps,
transaction,
context
context,
isParentPure
);

if (this._tag === 'select') {
Expand Down Expand Up @@ -1053,7 +1066,13 @@ ReactDOMComponent.Mixin = {
* @param {ReactReconcileTransaction} transaction
* @param {object} context
*/
_updateDOMChildren: function(lastProps, nextProps, transaction, context) {
_updateDOMChildren: function(
lastProps,
nextProps,
transaction,
context,
isParentPure
) {
var lastContent =
CONTENT_TYPES[typeof lastProps.children] ? lastProps.children : null;
var nextContent =
Expand All @@ -1075,7 +1094,7 @@ ReactDOMComponent.Mixin = {
var lastHasContentOrHtml = lastContent != null || lastHtml != null;
var nextHasContentOrHtml = nextContent != null || nextHtml != null;
if (lastChildren != null && nextChildren == null) {
this.updateChildren(null, transaction, context);
this.updateChildren(null, transaction, context, isParentPure);
} else if (lastHasContentOrHtml && !nextHasContentOrHtml) {
this.updateTextContent('');
if (__DEV__) {
Expand All @@ -1102,7 +1121,7 @@ ReactDOMComponent.Mixin = {
setContentChildForInstrumentation.call(this, null);
}

this.updateChildren(nextChildren, transaction, context);
this.updateChildren(nextChildren, transaction, context, isParentPure);
}
},

Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/shared/ReactDOMEmptyComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Object.assign(ReactDOMEmptyComponent.prototype, {
return '<!--' + nodeValue + '-->';
}
},
receiveComponent: function() {
receiveComponent: function(nextElement, transaction, context, isParentPure) {
},
getHostNode: function() {
return ReactDOMComponentTree.getNodeFromInstance(this);
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/shared/ReactDOMTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Object.assign(ReactDOMTextComponent.prototype, {
* @param {ReactReconcileTransaction} transaction
* @internal
*/
receiveComponent: function(nextText, transaction) {
receiveComponent: function(nextText, transaction, context, isParentPure) {
if (nextText !== this._currentElement) {
this._currentElement = nextText;
var nextStringText = '' + nextText;
Expand Down
9 changes: 7 additions & 2 deletions src/renderers/native/ReactNativeBaseComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ ReactNativeBaseComponent.Mixin = {
* @param {object} context
* @internal
*/
receiveComponent: function(nextElement, transaction, context) {
receiveComponent: function(nextElement, transaction, context, isParentPure) {
var prevElement = this._currentElement;
this._currentElement = nextElement;

Expand Down Expand Up @@ -127,7 +127,12 @@ ReactNativeBaseComponent.Mixin = {
prevElement.props,
nextElement.props
);
this.updateChildren(nextElement.props.children, transaction, context);
this.updateChildren(
nextElement.props.children,
transaction,
context,
isParentPure
);
},

/**
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/native/ReactNativeTextComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Object.assign(ReactNativeTextComponent.prototype, {
return this._rootNodeID;
},

receiveComponent: function(nextText, transaction, context) {
receiveComponent: function(nextText, transaction, context, isParentPure) {
if (nextText !== this._currentElement) {
this._currentElement = nextText;
var nextStringText = '' + nextText;
Expand Down
5 changes: 3 additions & 2 deletions src/renderers/shared/stack/reconciler/ReactChildReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ var ReactChildReconciler = {
nextChildren,
removedNodes,
transaction,
context) {
context,
isParentPure) {
// We currently don't have a way to track moves here but if we use iterators
// instead of for..in we can zip the iterators and check if an item has
// moved.
Expand All @@ -116,7 +117,7 @@ var ReactChildReconciler = {
if (prevChild != null &&
shouldUpdateReactComponent(prevElement, nextElement)) {
ReactReconciler.receiveComponent(
prevChild, nextElement, transaction, context
prevChild, nextElement, transaction, context, isParentPure
);
nextChildren[name] = prevChild;
} else {
Expand Down
41 changes: 30 additions & 11 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,12 @@ var ReactCompositeComponentMixin = {
);
},

receiveComponent: function(nextElement, transaction, nextContext) {
receiveComponent: function(
nextElement,
transaction,
nextContext,
isParentPure
) {
var prevElement = this._currentElement;
var prevContext = this._context;

Expand All @@ -705,7 +710,8 @@ var ReactCompositeComponentMixin = {
prevElement,
nextElement,
prevContext,
nextContext
nextContext,
isParentPure
);
},

Expand All @@ -722,15 +728,21 @@ var ReactCompositeComponentMixin = {
this,
this._pendingElement,
transaction,
this._context
this._context,
// Element updates are enqueued only at the top level, which we consider
// impure
false
);
} else if (this._pendingStateQueue !== null || this._pendingForceUpdate) {
this.updateComponent(
transaction,
this._currentElement,
this._currentElement,
this._context,
this._context
this._context,
// isParentPure here doesn't matter because state updates don't happen to
// functional components.
true
);
} else {
this._updateBatchNumber = null;
Expand All @@ -757,7 +769,8 @@ var ReactCompositeComponentMixin = {
prevParentElement,
nextParentElement,
prevUnmaskedContext,
nextUnmaskedContext
nextUnmaskedContext,
isParentPure
) {
var inst = this._instance;
var willReceive = false;
Expand Down Expand Up @@ -805,6 +818,9 @@ var ReactCompositeComponentMixin = {
var nextState = this._processPendingState(nextProps, nextContext);
var shouldUpdate = true;

var pureSelf =
this._compositeType === CompositeTypes.PureClass ||
isParentPure && this._compositeType === CompositeTypes.StatelessFunctional;
if (!this._pendingForceUpdate) {
if (inst.shouldComponentUpdate) {
if (__DEV__) {
Expand All @@ -825,7 +841,7 @@ var ReactCompositeComponentMixin = {
}
}
} else {
if (this._compositeType === CompositeTypes.PureClass) {
if (pureSelf) {
shouldUpdate =
inst.state !== nextState || !shallowEqual(prevProps, nextProps);
}
Expand All @@ -851,7 +867,8 @@ var ReactCompositeComponentMixin = {
nextState,
nextContext,
transaction,
nextUnmaskedContext
nextUnmaskedContext,
pureSelf
);
} else {
// If it's determined that a component should not update, we still want
Expand Down Expand Up @@ -911,7 +928,8 @@ var ReactCompositeComponentMixin = {
nextState,
nextContext,
transaction,
unmaskedContext
unmaskedContext,
pureSelf
) {
var inst = this._instance;

Expand Down Expand Up @@ -951,7 +969,7 @@ var ReactCompositeComponentMixin = {
inst.state = nextState;
inst.context = nextContext;

this._updateRenderedComponent(transaction, unmaskedContext);
this._updateRenderedComponent(transaction, unmaskedContext, pureSelf);

if (hasComponentDidUpdate) {
if (__DEV__) {
Expand All @@ -974,7 +992,7 @@ var ReactCompositeComponentMixin = {
* @param {ReactReconcileTransaction} transaction
* @internal
*/
_updateRenderedComponent: function(transaction, context) {
_updateRenderedComponent: function(transaction, context, pureSelf) {
var prevComponentInstance = this._renderedComponent;
var prevRenderedElement = prevComponentInstance._currentElement;
var nextRenderedElement = this._renderValidatedComponent();
Expand All @@ -983,7 +1001,8 @@ var ReactCompositeComponentMixin = {
prevComponentInstance,
nextRenderedElement,
transaction,
this._processChildContext(context)
this._processChildContext(context),
pureSelf
);
} else {
var oldHostNode = ReactReconciler.getHostNode(prevComponentInstance);
Expand Down
Loading

0 comments on commit 0af8c44

Please sign in to comment.