Skip to content

Commit

Permalink
Merge pull request facebook#2539 from sebmarkbage/cleanupinternals
Browse files Browse the repository at this point in the history
Clean up a bunch of internal methods and fields that are now unnecessary
  • Loading branch information
sebmarkbage committed Nov 17, 2014
2 parents a194e51 + 63644d5 commit e4218cb
Show file tree
Hide file tree
Showing 11 changed files with 164 additions and 199 deletions.
16 changes: 9 additions & 7 deletions src/browser/ui/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ ReactDOMComponent.Mixin = {
transaction,
mountDepth
);
assertValidProps(this.props);
this._previousStyleCopy = null;
assertValidProps(this._currentElement.props);
var closeTag = omittedCloseTags[this._tag] ? '' : '</' + this._tag + '>';
return (
this._createOpenTagMarkupAndPutListeners(transaction) +
Expand All @@ -197,7 +197,7 @@ ReactDOMComponent.Mixin = {
* @return {string} Markup of opening tag.
*/
_createOpenTagMarkupAndPutListeners: function(transaction) {
var props = this.props;
var props = this._currentElement.props;
var ret = '<' + this._tag;

for (var propKey in props) {
Expand Down Expand Up @@ -253,16 +253,18 @@ ReactDOMComponent.Mixin = {
prefix = '\n';
}

var props = this._currentElement.props;

// Intentional use of != to avoid catching zero/false.
var innerHTML = this.props.dangerouslySetInnerHTML;
var innerHTML = props.dangerouslySetInnerHTML;
if (innerHTML != null) {
if (innerHTML.__html != null) {
return prefix + innerHTML.__html;
}
} else {
var contentToUse =
CONTENT_TYPES[typeof this.props.children] ? this.props.children : null;
var childrenToUse = contentToUse != null ? null : this.props.children;
CONTENT_TYPES[typeof props.children] ? props.children : null;
var childrenToUse = contentToUse != null ? null : props.children;
if (contentToUse != null) {
return prefix + escapeTextForBrowser(contentToUse);
} else if (childrenToUse != null) {
Expand Down Expand Up @@ -338,7 +340,7 @@ ReactDOMComponent.Mixin = {
* @param {ReactReconcileTransaction} transaction
*/
_updateDOMProperties: function(lastProps, transaction) {
var nextProps = this.props;
var nextProps = this._currentElement.props;
var propKey;
var styleName;
var styleUpdates;
Expand Down Expand Up @@ -427,7 +429,7 @@ ReactDOMComponent.Mixin = {
* @param {ReactReconcileTransaction} transaction
*/
_updateDOMChildren: function(lastProps, transaction) {
var nextProps = this.props;
var nextProps = this._currentElement.props;

var lastContent =
CONTENT_TYPES[typeof lastProps.children] ? lastProps.children : null;
Expand Down
4 changes: 2 additions & 2 deletions src/browser/ui/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ describe('ReactDOMComponent', function() {
var ReactReconcileTransaction = require('ReactReconcileTransaction');

var NodeStub = function(initialProps) {
this.props = initialProps || {};
this._currentElement = { props: initialProps };
this._rootNodeID = 'test';
};
assign(NodeStub.prototype, ReactDOMComponent.Mixin);
Expand Down Expand Up @@ -276,7 +276,7 @@ describe('ReactDOMComponent', function() {
var ReactReconcileTransaction = require('ReactReconcileTransaction');

var NodeStub = function(initialProps) {
this.props = initialProps || {};
this._currentElement = { props: initialProps };
this._rootNodeID = 'test';
};
assign(NodeStub.prototype, ReactDOMComponent.Mixin);
Expand Down
100 changes: 4 additions & 96 deletions src/core/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@ var assign = require('Object.assign');
var invariant = require('invariant');
var keyMirror = require('keyMirror');

/**
* Every React component is in one of these life cycles.
*/
var ComponentLifeCycle = keyMirror({
/**
* Mounted components have a DOM node representation and are capable of
* receiving new props.
*/
MOUNTED: null,
/**
* Unmounted components are inactive and cannot receive new props.
*/
UNMOUNTED: null
});

var injected = false;

/**
Expand Down Expand Up @@ -115,11 +100,6 @@ var ReactComponent = {
}
},

/**
* @internal
*/
LifeCycle: ComponentLifeCycle,

/**
* Injected module that provides ability to mutate individual properties.
* Injected into the base class because many different subclasses need access
Expand All @@ -137,17 +117,6 @@ var ReactComponent = {
*/
Mixin: {

/**
* Checks whether or not this component is mounted.
*
* @return {boolean} True if mounted, false otherwise.
* @final
* @protected
*/
isMounted: function() {
return this._lifeCycleState === ComponentLifeCycle.MOUNTED;
},

/**
* Sets a subset of the props.
*
Expand Down Expand Up @@ -175,10 +144,6 @@ var ReactComponent = {
* @public
*/
replaceProps: function(props, callback) {
invariant(
this.isMounted(),
'replaceProps(...): Can only update a mounted component.'
);
invariant(
this._mountDepth === 0,
'replaceProps(...): You called `setProps` or `replaceProps` on a ' +
Expand Down Expand Up @@ -225,19 +190,6 @@ var ReactComponent = {
* @internal
*/
construct: function(element) {
// This is the public exposed props object after it has been processed
// with default props. The element's props represents the true internal
// state of the props.
this.props = element.props;
// Record the component responsible for creating this component.
// This is accessible through the element but we maintain an extra
// field for compatibility with devtools and as a way to make an
// incremental update. TODO: Consider deprecating this field.
this._owner = element._owner;

// All components start unmounted.
this._lifeCycleState = ComponentLifeCycle.UNMOUNTED;

// See ReactUpdates.
this._pendingCallbacks = null;

Expand All @@ -262,20 +214,12 @@ var ReactComponent = {
* @internal
*/
mountComponent: function(rootID, transaction, mountDepth) {
invariant(
!this.isMounted(),
'mountComponent(%s, ...): Can only mount an unmounted component. ' +
'Make sure to avoid storing components between renders or reusing a ' +
'single component instance in multiple places.',
rootID
);
var ref = this._currentElement.ref;
if (ref != null) {
var owner = this._currentElement._owner;
attachRef(ref, this, owner);
}
this._rootNodeID = rootID;
this._lifeCycleState = ComponentLifeCycle.MOUNTED;
this._mountDepth = mountDepth;
// Effectively: return '';
},
Expand All @@ -291,17 +235,15 @@ var ReactComponent = {
* @internal
*/
unmountComponent: function() {
invariant(
this.isMounted(),
'unmountComponent(): Can only unmount a mounted component.'
);
var ref = this._currentElement.ref;
if (ref != null) {
detachRef(ref, this, this._owner);
detachRef(ref, this, this._currentElement._owner);
}
unmountIDFromEnvironment(this._rootNodeID);
// Reset all fields
this._rootNodeID = null;
this._lifeCycleState = ComponentLifeCycle.UNMOUNTED;
this._pendingCallbacks = null;
this._pendingElement = null;
},

/**
Expand All @@ -316,10 +258,6 @@ var ReactComponent = {
* @internal
*/
receiveComponent: function(nextElement, transaction) {
invariant(
this.isMounted(),
'receiveComponent(...): Can only update a mounted component.'
);
this._pendingElement = nextElement;
this.performUpdateIfNecessary(transaction);
},
Expand All @@ -337,8 +275,6 @@ var ReactComponent = {
var prevElement = this._currentElement;
var nextElement = this._pendingElement;
this._currentElement = nextElement;
this.props = nextElement.props;
this._owner = nextElement._owner;
this._pendingElement = null;
this.updateComponent(transaction, prevElement, nextElement);
},
Expand Down Expand Up @@ -416,34 +352,6 @@ var ReactComponent = {
mountImageIntoNode(markup, container, shouldReuseMarkup);
},

/**
* Checks if this component is owned by the supplied `owner` component.
*
* @param {ReactComponent} owner Component to check.
* @return {boolean} True if `owners` owns this component.
* @final
* @internal
*/
isOwnedBy: function(owner) {
return this._owner === owner;
},

/**
* Gets another component, that shares the same owner as this one, by ref.
*
* @param {string} ref of a sibling Component.
* @return {?ReactComponent} the actual sibling Component.
* @final
* @internal
*/
getSiblingByRef: function(ref) {
var owner = this._owner;
if (!owner || !owner.refs) {
return null;
}
return owner.refs[ref];
},

/**
* Get the publicly accessible representation of this component - i.e. what
* is exposed by refs and renderComponent. Can be null for stateless
Expand Down
54 changes: 36 additions & 18 deletions src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ var ReactContext = require('ReactContext');
var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactElement = require('ReactElement');
var ReactInstanceMap = require('ReactInstanceMap');
var ReactOwner = require('ReactOwner');
var ReactPerf = require('ReactPerf');
var ReactPropTypeLocations = require('ReactPropTypeLocations');
var ReactUpdates = require('ReactUpdates');

var assign = require('Object.assign');
var emptyObject = require('emptyObject');
var invariant = require('invariant');
var keyMirror = require('keyMirror');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
var warning = require('warning');

function getDeclarationErrorAddendum(component) {
var owner = component._owner || null;
var owner = component._currentElement._owner || null;
if (owner) {
var constructor = owner._instance.constructor;
if (constructor && constructor.displayName) {
Expand Down Expand Up @@ -56,8 +56,8 @@ function validateLifeCycleOnReplaceState(instance) {
* `ReactCompositeComponent` maintains an auxiliary life cycle state in
* `this._compositeLifeCycleState` (which can be null).
*
* This is different from the life cycle state maintained by `ReactComponent` in
* `this._lifeCycleState`. The following diagram shows how the states overlap in
* This is different from the life cycle state maintained by `ReactComponent`.
* The following diagram shows how the states overlap in
* time. There are times when the CompositeLifeCycle is null - at those times it
* is only meaningful to look at ComponentLifeCycle alone.
*
Expand Down Expand Up @@ -100,8 +100,7 @@ var CompositeLifeCycle = keyMirror({
* @lends {ReactCompositeComponent.prototype}
*/
var ReactCompositeComponentMixin = assign({},
ReactComponent.Mixin,
ReactOwner.Mixin, {
ReactComponent.Mixin, {

/**
* Base constructor for all composite component.
Expand All @@ -114,13 +113,13 @@ var ReactCompositeComponentMixin = assign({},
this._instance.props = element.props;
this._instance.state = null;
this._instance.context = null;
this._instance.refs = emptyObject;

this._pendingState = null;
this._compositeLifeCycleState = null;

// Children can be either an array or more than one argument
ReactComponent.Mixin.construct.apply(this, arguments);
ReactOwner.Mixin.construct.apply(this, arguments);
},

/**
Expand All @@ -130,8 +129,7 @@ var ReactCompositeComponentMixin = assign({},
* @final
*/
isMounted: function() {
return ReactComponent.Mixin.isMounted.call(this) &&
this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING;
return this._compositeLifeCycleState !== CompositeLifeCycle.MOUNTING;
},

/**
Expand Down Expand Up @@ -233,6 +231,9 @@ var ReactCompositeComponentMixin = assign({},
this._renderedComponent.unmountComponent();
this._renderedComponent = null;

// Reset pending fields
this._pendingState = null;
this._pendingForceUpdate = false;
ReactComponent.Mixin.unmountComponent.call(this);

// Delete the reference from the instance to this internal representation
Expand Down Expand Up @@ -555,11 +556,6 @@ var ReactCompositeComponentMixin = assign({},
inst.props = nextProps;
inst.state = nextState;
inst.context = nextContext;

// Owner cannot change because shouldUpdateReactComponent doesn't allow
// it. TODO: Remove this._owner completely.
this._owner = nextParentElement._owner;

return;
}

Expand Down Expand Up @@ -607,10 +603,6 @@ var ReactCompositeComponentMixin = assign({},
inst.state = nextState;
inst.context = nextContext;

// Owner cannot change because shouldUpdateReactComponent doesn't allow
// it. TODO: Remove this._owner completely.
this._owner = nextElement._owner;

this._updateRenderedComponent(transaction);

if (inst.componentDidUpdate) {
Expand Down Expand Up @@ -699,6 +691,32 @@ var ReactCompositeComponentMixin = assign({},
}
),

/**
* Lazily allocates the refs object and stores `component` as `ref`.
*
* @param {string} ref Reference name.
* @param {component} component Component to store as `ref`.
* @final
* @private
*/
attachRef: function(ref, component) {
var inst = this.getPublicInstance();
var refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs;
refs[ref] = component.getPublicInstance();
},

/**
* Detaches a reference name.
*
* @param {string} ref Name to dereference.
* @final
* @private
*/
detachRef: function(ref) {
var refs = this.getPublicInstance().refs;
delete refs[ref];
},

/**
* Get the publicly accessible representation of this component - i.e. what
* is exposed by refs and renderComponent. Can be null for stateless
Expand Down
Loading

0 comments on commit e4218cb

Please sign in to comment.