Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add React.PureComponent #7195

Merged
merged 1 commit into from
Jul 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/isomorphic/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

var ReactChildren = require('ReactChildren');
var ReactComponent = require('ReactComponent');
var ReactPureComponent = require('ReactPureComponent');
var ReactClass = require('ReactClass');
var ReactDOMFactories = require('ReactDOMFactories');
var ReactElement = require('ReactElement');
Expand Down Expand Up @@ -63,6 +64,7 @@ var React = {
},

Component: ReactComponent,
PureComponent: ReactPureComponent,

createElement: createElement,
cloneElement: cloneElement,
Expand Down
40 changes: 40 additions & 0 deletions src/isomorphic/modern/class/ReactPureComponent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactPureComponent
*/

'use strict';

var ReactComponent = require('ReactComponent');
var ReactNoopUpdateQueue = require('ReactNoopUpdateQueue');

var emptyObject = require('emptyObject');

/**
* Base class helpers for the updating state of a component.
*/
function ReactPureComponent(props, context, updater) {
// Duplicated from ReactComponent.
this.props = props;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a duplication, right? 🍺

this.context = context;
this.refs = emptyObject;
// We initialize the default updater but the real one gets injected by the
// renderer.
this.updater = updater || ReactNoopUpdateQueue;
}

function ComponentDummy() {}
ComponentDummy.prototype = ReactComponent.prototype;
ReactPureComponent.prototype = new ComponentDummy();
ReactPureComponent.prototype.constructor = ReactPureComponent;
// Avoid an extra prototype jump for these methods.
Object.assign(ReactPureComponent.prototype, ReactComponent.prototype);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we extend ReactComponent? What about people doing instanceof React.Component even though they shouldn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right.

ReactPureComponent.prototype.isPureReactComponent = true;

module.exports = ReactPureComponent;
97 changes: 97 additions & 0 deletions src/isomorphic/modern/class/__tests__/ReactPureComponent-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @emails react-core
*/

'use strict';

var React;
var ReactDOM;

describe('ReactPureComponent', function() {
beforeEach(function() {
React = require('React');
ReactDOM = require('ReactDOM');
});

it('should render', function() {
var renders = 0;
class Component extends React.PureComponent {
constructor() {
super();
this.state = {type: 'mushrooms'};
}
render() {
renders++;
return <div>{this.props.text[0]}</div>;
}
}

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

text = ['porcini'];
component = ReactDOM.render(<Component text={text} />, container);
expect(container.textContent).toBe('porcini');
expect(renders).toBe(1);

text = ['morel'];
component = ReactDOM.render(<Component text={text} />, container);
expect(container.textContent).toBe('morel');
expect(renders).toBe(2);

text[0] = 'portobello';
component = ReactDOM.render(<Component text={text} />, container);
expect(container.textContent).toBe('morel');
expect(renders).toBe(2);

// Setting state without changing it doesn't cause a rerender.
component.setState({type: 'mushrooms'});
expect(container.textContent).toBe('morel');
expect(renders).toBe(2);

// But changing state does.
component.setState({type: 'portobello mushrooms'});
expect(container.textContent).toBe('portobello');
expect(renders).toBe(3);
});

it('can override shouldComponentUpdate', function() {
var renders = 0;
class Component extends React.PureComponent {
render() {
renders++;
return <div />;
}
shouldComponentUpdate() {
return true;
}
}
var container = document.createElement('div');
ReactDOM.render(<Component />, container);
ReactDOM.render(<Component />, container);
expect(renders).toBe(2);
});

it('extends React.Component', function() {
var renders = 0;
class Component extends React.PureComponent {
render() {
expect(this instanceof React.Component).toBe(true);
expect(this instanceof React.PureComponent).toBe(true);
renders++;
return <div />;
}
}
ReactDOM.render(<Component />, document.createElement('div'));
expect(renders).toBe(1);
});

});
108 changes: 80 additions & 28 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,18 @@ var ReactPropTypeLocations = require('ReactPropTypeLocations');
var ReactReconciler = require('ReactReconciler');

var checkReactTypeSpec = require('checkReactTypeSpec');

var emptyObject = require('emptyObject');
var invariant = require('invariant');
var shallowEqual = require('shallowEqual');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
var warning = require('warning');

var CompositeTypes = {
ImpureClass: 0,
PureClass: 1,
StatelessFunctional: 2,
};

function StatelessComponent(Component) {
}
StatelessComponent.prototype.render = function() {
Expand Down Expand Up @@ -88,7 +94,11 @@ function invokeComponentDidUpdateWithTimer(prevProps, prevState, prevContext) {
}

function shouldConstruct(Component) {
return Component.prototype && Component.prototype.isReactComponent;
return !!(Component.prototype && Component.prototype.isReactComponent);
}

function isPureComponent(Component) {
return !!(Component.prototype && Component.prototype.isPureReactComponent);
}

/**
Expand Down Expand Up @@ -141,6 +151,7 @@ var ReactCompositeComponentMixin = {
construct: function(element) {
this._currentElement = element;
this._rootNodeID = null;
this._compositeType = null;
this._instance = null;
this._hostParent = null;
this._hostContainerInfo = null;
Expand Down Expand Up @@ -199,11 +210,17 @@ var ReactCompositeComponentMixin = {
var updateQueue = transaction.getUpdateQueue();

// Initialize the public class
var inst = this._constructComponent(publicProps, publicContext, updateQueue);
var doConstruct = shouldConstruct(Component);
var inst = this._constructComponent(
doConstruct,
publicProps,
publicContext,
updateQueue
);
var renderedElement;

// Support functional components
if (!shouldConstruct(Component) && (inst == null || inst.render == null)) {
if (!doConstruct && (inst == null || inst.render == null)) {
renderedElement = inst;
warnIfInvalidElement(Component, renderedElement);
invariant(
Expand All @@ -215,6 +232,13 @@ var ReactCompositeComponentMixin = {
Component.displayName || Component.name || 'Component'
);
inst = new StatelessComponent(Component);
this._compositeType = CompositeTypes.StatelessFunctional;
} else {
if (isPureComponent(Component)) {
this._compositeType = CompositeTypes.PureClass;
} else {
this._compositeType = CompositeTypes.ImpureClass;
}
}

if (__DEV__) {
Expand Down Expand Up @@ -353,23 +377,43 @@ var ReactCompositeComponentMixin = {
return markup;
},

_constructComponent: function(publicProps, publicContext, updateQueue) {
_constructComponent: function(
doConstruct,
publicProps,
publicContext,
updateQueue
) {
if (__DEV__) {
ReactCurrentOwner.current = this;
try {
return this._constructComponentWithoutOwner(publicProps, publicContext, updateQueue);
return this._constructComponentWithoutOwner(
doConstruct,
publicProps,
publicContext,
updateQueue
);
} finally {
ReactCurrentOwner.current = null;
}
} else {
return this._constructComponentWithoutOwner(publicProps, publicContext, updateQueue);
return this._constructComponentWithoutOwner(
doConstruct,
publicProps,
publicContext,
updateQueue
);
}
},

_constructComponentWithoutOwner: function(publicProps, publicContext, updateQueue) {
_constructComponentWithoutOwner: function(
doConstruct,
publicProps,
publicContext,
updateQueue
) {
var Component = this._currentElement.type;
var instanceOrElement;
if (shouldConstruct(Component)) {
if (doConstruct) {
if (__DEV__) {
if (this._debugID !== 0) {
ReactInstrumentation.debugTool.onBeginLifeCycleTimer(
Expand Down Expand Up @@ -763,7 +807,6 @@ var ReactCompositeComponentMixin = {

var willReceive = false;
var nextContext;
var nextProps;

// Determine if the context has changed or not
if (this._context === nextUnmaskedContext) {
Expand All @@ -773,7 +816,8 @@ var ReactCompositeComponentMixin = {
willReceive = true;
}

nextProps = nextParentElement.props;
var prevProps = prevParentElement.props;
var nextProps = nextParentElement.props;

// Not a simple state update but a props update
if (prevParentElement !== nextParentElement) {
Expand Down Expand Up @@ -806,22 +850,30 @@ var ReactCompositeComponentMixin = {
var nextState = this._processPendingState(nextProps, nextContext);
var shouldUpdate = true;

if (!this._pendingForceUpdate && inst.shouldComponentUpdate) {
if (__DEV__) {
if (this._debugID !== 0) {
ReactInstrumentation.debugTool.onBeginLifeCycleTimer(
this._debugID,
'shouldComponentUpdate'
);
if (!this._pendingForceUpdate) {
if (inst.shouldComponentUpdate) {
if (__DEV__) {
if (this._debugID !== 0) {
ReactInstrumentation.debugTool.onBeginLifeCycleTimer(
this._debugID,
'shouldComponentUpdate'
);
}
}
}
shouldUpdate = inst.shouldComponentUpdate(nextProps, nextState, nextContext);
if (__DEV__) {
if (this._debugID !== 0) {
ReactInstrumentation.debugTool.onEndLifeCycleTimer(
this._debugID,
'shouldComponentUpdate'
);
shouldUpdate = inst.shouldComponentUpdate(nextProps, nextState, nextContext);
if (__DEV__) {
if (this._debugID !== 0) {
ReactInstrumentation.debugTool.onEndLifeCycleTimer(
this._debugID,
'shouldComponentUpdate'
);
}
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could be else if for less nesting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be but I prefer parallel structure in my if/else-if cases so I left it like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be but I prefer parallel structure in my if/else-if cases so I left it like this.

IMO, always good to use least syntax~

if (this._compositeType === CompositeTypes.PureClass) {
shouldUpdate =
!shallowEqual(prevProps, nextProps) ||
!shallowEqual(inst.state, nextState);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you abandon the idea of comparing state by reference?

Copy link
Collaborator Author

@sophiebits sophiebits Jul 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now. There's a bunch of our internal components that rely on it so probably externally too. (FB-only link: https://fburl.com/373836543.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have to be a fb employee to see the internal link?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I meant by "internal".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
}
Expand Down Expand Up @@ -1084,7 +1136,7 @@ var ReactCompositeComponentMixin = {
*/
_renderValidatedComponent: function() {
var renderedComponent;
if (__DEV__ || !(this._instance instanceof StatelessComponent)) {
if (__DEV__ || this._compositeType !== CompositeTypes.StatelessFunctional) {
ReactCurrentOwner.current = this;
try {
renderedComponent =
Expand Down Expand Up @@ -1174,7 +1226,7 @@ var ReactCompositeComponentMixin = {
*/
getPublicInstance: function() {
var inst = this._instance;
if (inst instanceof StatelessComponent) {
if (this._compositeType === CompositeTypes.StatelessFunctional) {
return null;
}
return inst;
Expand Down