Clone on mount

This is the first step towards descriptors. This will start cloning the
component when it's mounted instead of mounting the first instance.

This avoids an issue where a reference to the first instance can hang around
in props. Since a mounted component gets mutated, the descriptor changes.

We don't need to clone the props object itself. Mutating the shallow props
object of a child that's passed into you is already flawed. Those cases need to
use cloneWithProps. A props object is considered shallow frozen after it leaves
the render it was created in.
This commit is contained in:
Sebastian Markbage
2014-03-10 15:26:31 -07:00
committed by Paul O’Shannessy
parent 2ca810fbf3
commit 3ea3274ca4
11 changed files with 312 additions and 102 deletions
+10
View File
@@ -45,6 +45,16 @@ var ReactTextComponent = function(initialText) {
this.construct({text: initialText});
};
/**
* Used to clone the text descriptor object before it's mounted.
*
* @param {object} props
* @return {object} A new ReactTextComponent instance
*/
ReactTextComponent.ConvenienceConstructor = function(props) {
return new ReactTextComponent(props.text);
};
mixInto(ReactTextComponent, ReactComponent.Mixin);
mixInto(ReactTextComponent, ReactBrowserComponentMixin);
mixInto(ReactTextComponent, {
+3 -2
View File
@@ -24,7 +24,7 @@ var ReactMarkupChecksum = require('ReactMarkupChecksum');
var ReactServerRenderingTransaction =
require('ReactServerRenderingTransaction');
var instantiateReactComponent = require('instantiateReactComponent');
var invariant = require('invariant');
/**
@@ -49,7 +49,8 @@ function renderComponentToString(component) {
transaction = ReactServerRenderingTransaction.getPooled(false);
return transaction.perform(function() {
var markup = component.mountComponent(id, transaction, 0);
var componentInstance = instantiateReactComponent(component);
var markup = componentInstance.mountComponent(id, transaction, 0);
return ReactMarkupChecksum.addChecksumToMarkup(markup);
}, null);
} finally {
+9 -3
View File
@@ -25,6 +25,7 @@ var ReactPerf = require('ReactPerf');
var containsNode = require('containsNode');
var getReactRootElementInContainer = require('getReactRootElementInContainer');
var instantiateReactComponent = require('instantiateReactComponent');
var invariant = require('invariant');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
@@ -296,8 +297,13 @@ var ReactMount = {
nextComponent,
container,
shouldReuseMarkup) {
var reactRootID = ReactMount._registerComponent(nextComponent, container);
nextComponent.mountComponentIntoNode(
var componentInstance = instantiateReactComponent(nextComponent);
var reactRootID = ReactMount._registerComponent(
componentInstance,
container
);
componentInstance.mountComponentIntoNode(
reactRootID,
container,
shouldReuseMarkup
@@ -309,7 +315,7 @@ var ReactMount = {
getReactRootElementInContainer(container);
}
return nextComponent;
return componentInstance;
}
),
+168 -54
View File
@@ -29,6 +29,7 @@ var ReactPropTypeLocations = require('ReactPropTypeLocations');
var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
var ReactUpdates = require('ReactUpdates');
var instantiateReactComponent = require('instantiateReactComponent');
var invariant = require('invariant');
var keyMirror = require('keyMirror');
var merge = require('merge');
@@ -595,19 +596,52 @@ if (__DEV__) {
constructor: true,
construct: true,
isOwnedBy: true, // should be deprecated but can have code mod (internal)
mountComponent: true,
mountComponentIntoNode: true,
props: true,
type: true,
_checkPropTypes: true,
_mountComponentIntoNode: true,
_processContext: true
props: true,
// currently private but belong on the descriptor and are valid for use
// inside the framework:
__keyValidated__: true,
_owner: true,
_currentContext: true
};
var componentInstanceProperties = {
__keyValidated__: true,
__keySetters: true,
_compositeLifeCycleState: true,
_currentContext: true,
_defaultProps: true,
_instance: true,
_lifeCycleState: true,
_mountDepth: true,
_owner: true,
_pendingCallbacks: true,
_pendingContext: true,
_pendingForceUpdate: true,
_pendingOwner: true,
_pendingProps: true,
_pendingState: true,
_renderedComponent: true,
_rootNodeID: true,
context: true,
props: true,
refs: true,
state: true,
// These are known instance properties coming from other sources
_pendingQueries: true,
_queryPropListeners: true,
queryParams: true
};
var hasWarnedOnComponentType = {};
var warnIfUnmounted = function(instance, key) {
if (instance.__hasBeenMounted) {
var warningStackCounter = 0;
var issueMembraneWarning = function(instance, key) {
var isWhitelisted = unmountedPropertyWhitelist.hasOwnProperty(key);
if (warningStackCounter > 0 || isWhitelisted) {
return;
}
var name = instance.constructor.displayName || 'Unknown';
@@ -631,6 +665,30 @@ if (__DEV__) {
);
};
var wrapInMembraneFunction = function(fn, thisBinding) {
if (fn.__reactMembraneFunction && fn.__reactMembraneSelf === thisBinding) {
return fn.__reactMembraneFunction;
}
return fn.__reactMembraneFunction = function() {
/**
* By getting this function, you've already received a warning. The
* internals of this function will likely cause more warnings. To avoid
* Spamming too much we disable any warning triggered inside of this
* stack.
*/
warningStackCounter++;
try {
// If the this binding is unchanged, we defer to the real component.
// This is important to keep some referential integrity in the
// internals. E.g. owner equality check.
var self = this === thisBinding ? this.__realComponentInstance : this;
return fn.apply(self, arguments);
} finally {
warningStackCounter--;
}
};
};
var defineMembraneProperty = function(membrane, prototype, key) {
Object.defineProperty(membrane, key, {
@@ -638,31 +696,33 @@ if (__DEV__) {
enumerable: true,
get: function() {
if (this !== membrane) {
// When this is accessed through a prototype chain we need to check if
// this component was mounted.
warnIfUnmounted(this, key);
if (this === membrane) {
// We're allowed to access the prototype directly.
return prototype[key];
}
return prototype[key];
issueMembraneWarning(this, key);
var realValue = this.__realComponentInstance[key];
// If the real value is a function, we need to provide a wrapper that
// disables nested warnings. The properties type and constructors are
// expected to the be constructors and therefore is often use with an
// equality check and we shouldn't try to rebind those.
if (typeof realValue === 'function' &&
key !== 'type' &&
key !== 'constructor') {
return wrapInMembraneFunction(realValue, this);
}
return realValue;
},
set: function(value) {
if (this !== membrane) {
// When this is accessed through a prototype chain, we first check if
// this component was mounted. Then we define a value on "this"
// instance, effectively disabling the membrane on that prototype
// chain.
warnIfUnmounted(this, key);
Object.defineProperty(this, key, {
enumerable: true,
configurable: true,
writable: true,
value: value
});
} else {
// Otherwise, this should modify the prototype
if (this === membrane) {
// We're allowed to set a value on the prototype directly.
prototype[key] = value;
return;
}
issueMembraneWarning(this, key);
this.__realComponentInstance[key] = value;
}
});
@@ -677,26 +737,51 @@ if (__DEV__) {
* @private
*/
var createMountWarningMembrane = function(prototype) {
try {
var membrane = Object.create(prototype);
for (var key in prototype) {
if (unmountedPropertyWhitelist.hasOwnProperty(key)) {
continue;
}
var membrane = {};
var key;
for (key in prototype) {
defineMembraneProperty(membrane, prototype, key);
}
// These are properties that goes into the instance but not the prototype.
// We can create the membrane on the prototype even though this will
// result in a faulty hasOwnProperty check it's better perf.
for (key in componentInstanceProperties) {
if (componentInstanceProperties.hasOwnProperty(key) &&
!(key in prototype)) {
defineMembraneProperty(membrane, prototype, key);
}
}
return membrane;
};
membrane.mountComponent = function() {
this.__hasBeenMounted = true;
return prototype.mountComponent.apply(this, arguments);
/**
* Creates a membrane constructor which wraps the component that gets mounted.
*
* @param {function} constructor Original constructor.
* @return {function} The membrane constructor.
* @private
*/
var createDescriptorProxy = function(constructor) {
try {
var ProxyConstructor = function() {
this.__realComponentInstance = new constructor();
// We can only safely pass through known instance variables. Unknown
// expandos are not safe. Use the real mounted instance to avoid this
// problem if it blows something up.
Object.freeze(this);
};
return membrane;
ProxyConstructor.prototype = createMountWarningMembrane(
constructor.prototype
);
return ProxyConstructor;
} catch(x) {
// In IE8 define property will fail on non-DOM objects. If anything in
// the membrane creation fails, we'll bail out and just use the prototype
// without warnings.
return prototype;
// the membrane creation fails, we'll bail out and just use the plain
// constructor without warnings.
return constructor;
}
};
@@ -772,13 +857,29 @@ var ReactCompositeComponentMixin = {
this.state = null;
this._pendingState = null;
this.context = this._processContext(ReactContext.current);
this.context = null;
this._currentContext = ReactContext.current;
this._pendingContext = null;
// The descriptor that was used to instantiate this component. Will be
// set by the instantiator instead of the constructor since this
// constructor is currently used by both instances and descriptors.
this._descriptor = null;
this._compositeLifeCycleState = null;
},
/**
* Components in the intermediate state now has cyclic references. To avoid
* breaking JSON serialization we expose a custom JSON format.
* @return {object} JSON compatible representation.
* @internal
* @final
*/
toJSON: function() {
return { type: this.type, props: this.props };
},
/**
* Checks whether or not this composite component is mounted.
* @return {boolean} True if mounted, false otherwise.
@@ -812,6 +913,7 @@ var ReactCompositeComponentMixin = {
);
this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING;
this.context = this._processContext(this._currentContext);
this._defaultProps = this.getDefaultProps ? this.getDefaultProps() : null;
this.props = this._processProps(this.props);
@@ -839,7 +941,9 @@ var ReactCompositeComponentMixin = {
}
}
this._renderedComponent = this._renderValidatedComponent();
this._renderedComponent = instantiateReactComponent(
this._renderValidatedComponent()
);
// Done with mounting, `setState` will now trigger UI changes.
this._compositeLifeCycleState = null;
@@ -1173,13 +1277,16 @@ var ReactCompositeComponentMixin = {
},
receiveComponent: function(nextComponent, transaction) {
if (nextComponent === this) {
if (nextComponent === this._descriptor) {
// Since props and context are immutable after the component is
// mounted, we can do a cheap identity compare here to determine
// if this is a superfluous reconcile.
return;
}
// Update the descriptor that was last used by this component instance
this._descriptor = nextComponent;
this._pendingContext = nextComponent._currentContext;
ReactComponent.Mixin.receiveComponent.call(
this,
@@ -1212,17 +1319,19 @@ var ReactCompositeComponentMixin = {
prevProps,
prevOwner
);
var prevComponent = this._renderedComponent;
var prevComponentInstance = this._renderedComponent;
var nextComponent = this._renderValidatedComponent();
if (shouldUpdateReactComponent(prevComponent, nextComponent)) {
prevComponent.receiveComponent(nextComponent, transaction);
if (shouldUpdateReactComponent(prevComponentInstance, nextComponent)) {
prevComponentInstance.receiveComponent(nextComponent, transaction);
} else {
// These two IDs are actually the same! But nothing should rely on that.
var thisID = this._rootNodeID;
var prevComponentID = prevComponent._rootNodeID;
prevComponent.unmountComponent();
this._renderedComponent = nextComponent;
var nextMarkup = nextComponent.mountComponent(
var prevComponentID = prevComponentInstance._rootNodeID;
prevComponentInstance.unmountComponent();
this._renderedComponent = instantiateReactComponent(nextComponent);
var nextMarkup = this._renderedComponent.mountComponent(
thisID,
transaction,
this._mountDepth + 1
@@ -1401,10 +1510,12 @@ var ReactCompositeComponent = {
Constructor.prototype = new ReactCompositeComponentBase();
Constructor.prototype.constructor = Constructor;
var DescriptorConstructor = Constructor;
var ConvenienceConstructor = function(props, children) {
var instance = new Constructor();
instance.construct.apply(instance, arguments);
return instance;
var descriptor = new DescriptorConstructor();
descriptor.construct.apply(descriptor, arguments);
return descriptor;
};
ConvenienceConstructor.componentConstructor = Constructor;
Constructor.ConvenienceConstructor = ConvenienceConstructor;
@@ -1452,7 +1563,10 @@ var ReactCompositeComponent = {
}
if (__DEV__) {
Constructor.prototype = createMountWarningMembrane(Constructor.prototype);
// In DEV the convenience constructor generates a proxy to another
// instance around it to warn about access to properties on the
// descriptor.
DescriptorConstructor = createDescriptorProxy(Constructor);
}
return ConvenienceConstructor;
+10 -3
View File
@@ -23,6 +23,7 @@ var ReactComponent = require('ReactComponent');
var ReactMultiChildUpdateTypes = require('ReactMultiChildUpdateTypes');
var flattenChildren = require('flattenChildren');
var instantiateReactComponent = require('instantiateReactComponent');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
/**
@@ -192,14 +193,18 @@ var ReactMultiChild = {
for (var name in children) {
var child = children[name];
if (children.hasOwnProperty(name)) {
// The rendered children must be turned into instances as they're
// mounted.
var childInstance = instantiateReactComponent(child);
children[name] = childInstance;
// Inlined for performance, see `ReactInstanceHandles.createReactID`.
var rootID = this._rootNodeID + name;
var mountImage = child.mountComponent(
var mountImage = childInstance.mountComponent(
rootID,
transaction,
this._mountDepth + 1
);
child._mountIndex = index;
childInstance._mountIndex = index;
mountImages.push(mountImage);
index++;
}
@@ -293,8 +298,10 @@ var ReactMultiChild = {
lastIndex = Math.max(prevChild._mountIndex, lastIndex);
this._unmountChildByName(prevChild, name);
}
// The child must be instantiated before it's mounted.
var nextChildInstance = instantiateReactComponent(nextChild);
this._mountChildByNameAtIndex(
nextChild, name, nextIndex, transaction
nextChildInstance, name, nextIndex, transaction
);
}
nextIndex++;
+11 -11
View File
@@ -61,13 +61,13 @@ describe('autobinding', function() {
});
var instance1 = <TestBindComponent />;
ReactTestUtils.renderIntoDocument(instance1);
var mountedInstance1 = ReactTestUtils.renderIntoDocument(instance1);
var rendered1 = reactComponentExpect(instance1)
.expectRenderedChild()
.instance();
var instance2 = <TestBindComponent />;
ReactTestUtils.renderIntoDocument(instance2);
var mountedInstance2 = ReactTestUtils.renderIntoDocument(instance2);
var rendered2 = reactComponentExpect(instance2)
.expectRenderedChild()
.instance();
@@ -77,25 +77,25 @@ describe('autobinding', function() {
badIdea();
}).toThrow();
expect(instance1.onMouseEnter).toBe(instance2.onMouseEnter);
expect(instance1.onMouseLeave).toBe(instance2.onMouseLeave);
expect(instance1.onClick).not.toBe(instance2.onClick);
expect(mountedInstance1.onMouseEnter).toBe(mountedInstance2.onMouseEnter);
expect(mountedInstance1.onMouseLeave).toBe(mountedInstance2.onMouseLeave);
expect(mountedInstance1.onClick).not.toBe(mountedInstance2.onClick);
ReactTestUtils.Simulate.click(rendered1);
expect(mouseDidClick.mock.instances.length).toBe(1);
expect(mouseDidClick.mock.instances[0]).toBe(instance1);
expect(mouseDidClick.mock.instances[0]).toBe(mountedInstance1);
ReactTestUtils.Simulate.click(rendered2);
expect(mouseDidClick.mock.instances.length).toBe(2);
expect(mouseDidClick.mock.instances[1]).toBe(instance2);
expect(mouseDidClick.mock.instances[1]).toBe(mountedInstance2);
ReactTestUtils.Simulate.mouseOver(rendered1);
expect(mouseDidEnter.mock.instances.length).toBe(1);
expect(mouseDidEnter.mock.instances[0]).toBe(instance1);
expect(mouseDidEnter.mock.instances[0]).toBe(mountedInstance1);
ReactTestUtils.Simulate.mouseOver(rendered2);
expect(mouseDidEnter.mock.instances.length).toBe(2);
expect(mouseDidEnter.mock.instances[1]).toBe(instance2);
expect(mouseDidEnter.mock.instances[1]).toBe(mountedInstance2);
ReactTestUtils.Simulate.mouseOut(rendered1);
expect(mouseDidLeave.mock.instances.length).toBe(1);
@@ -122,14 +122,14 @@ describe('autobinding', function() {
});
var instance1 = <TestBindComponent />;
ReactTestUtils.renderIntoDocument(instance1);
var mountedInstance1 = ReactTestUtils.renderIntoDocument(instance1);
var rendered1 = reactComponentExpect(instance1)
.expectRenderedChild()
.instance();
ReactTestUtils.Simulate.click(rendered1);
expect(mouseDidClick.mock.instances.length).toBe(1);
expect(mouseDidClick.mock.instances[0]).toBe(instance1);
expect(mouseDidClick.mock.instances[0]).toBe(mountedInstance1);
});
});
@@ -312,9 +312,7 @@ describe('ReactComponentLifeCycle', function() {
// A component that is merely "constructed" (as in "constructor") but not
// yet initialized, or rendered.
//
var instance = <LifeCycleComponent />;
expect(instance._lifeCycleState).toBe(ComponentLifeCycle.UNMOUNTED);
ReactTestUtils.renderIntoDocument(instance);
var instance = ReactTestUtils.renderIntoDocument(<LifeCycleComponent />);
// getInitialState
expect(instance._testJournal.returnedFromGetInitialState).toEqual(
@@ -183,21 +183,23 @@ describe('ReactCompositeComponent', function() {
// Next, prove that once mounted, the scope is bound correctly to the actual
// component.
ReactTestUtils.renderIntoDocument(instance);
var mountedInstance = ReactTestUtils.renderIntoDocument(instance);
expect(console.warn.argsForCall.length).toBe(3);
// This will result in a warning that has already been issued before.
var explicitlyBound = instance.methodToBeExplicitlyBound.bind(instance);
expect(console.warn.argsForCall.length).toBe(4);
expect(console.warn.argsForCall.length).toBe(3);
var autoBound = instance.methodAutoBound;
var explicitlyNotBound = instance.methodExplicitlyNotBound;
var context = {};
expect(explicitlyBound.call(context)).toBe(instance);
expect(autoBound.call(context)).toBe(instance);
expect(explicitlyBound.call(context)).toBe(mountedInstance);
expect(autoBound.call(context)).toBe(mountedInstance);
expect(explicitlyNotBound.call(context)).toBe(context);
expect(explicitlyBound.call(instance)).toBe(instance);
expect(autoBound.call(instance)).toBe(instance);
expect(explicitlyNotBound.call(instance)).toBe(instance);
expect(explicitlyBound.call(instance)).toBe(mountedInstance);
expect(autoBound.call(instance)).toBe(mountedInstance);
// This one is the weird one
expect(explicitlyNotBound.call(instance)).toBe(mountedInstance);
});
@@ -279,8 +281,7 @@ describe('ReactCompositeComponent', function() {
}
});
var instance = <Component />;
ReactTestUtils.renderIntoDocument(instance);
var instance = ReactTestUtils.renderIntoDocument(<Component />);
reactComponentExpect(instance).scalarPropsEqual({key: 'testKey'});
reactComponentExpect(instance).scalarStateEqual({key: 'testKeyState'});
@@ -1179,9 +1180,8 @@ describe('ReactCompositeComponent', function() {
expect(console.warn.argsForCall.length).toBe(0);
var unmountedInstance = <ComponentClass />;
var result = unmountedInstance.someMethod();
unmountedInstance.someMethod();
expect(console.warn.argsForCall.length).toBe(1);
expect(result).toBe(unmountedInstance);
var unmountedInstance2 = <ComponentClass />;
unmountedInstance2.someOtherMethod = 'override';
+70
View File
@@ -0,0 +1,70 @@
/**
* Copyright 2013-2014 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @providesModule instantiateReactComponent
* @typechecks static-only
*/
"use strict";
var warning = require('warning');
/**
* Validate a `componentDescriptor`. This should be exposed publicly in a follow
* up diff.
*
* @param {object} descriptor
* @return {boolean} Returns true if this is a valid descriptor of a Component.
*/
function isValidComponentDescriptor(descriptor) {
return (
typeof descriptor.constructor === 'function' &&
typeof descriptor.constructor.prototype.construct === 'function' &&
typeof descriptor.constructor.prototype.mountComponent === 'function' &&
typeof descriptor.constructor.prototype.receiveComponent === 'function'
);
}
/**
* Given a `componentDescriptor` create an instance that will actually be
* mounted. Currently it just extracts an existing clone from composite
* components but this is an implementation detail which will change.
*
* @param {object} descriptor
* @return {object} A new instance of componentDescriptor's constructor.
* @protected
*/
function instantiateReactComponent(descriptor) {
if (__DEV__) {
warning(
isValidComponentDescriptor(descriptor),
'Only React Components are valid for mounting.'
);
// We use the clone of a composite component instead of the original
// instance. This allows us to warn you if you're are accessing the wrong
// instance.
var instance = descriptor.__realComponentInstance || descriptor;
instance._descriptor = descriptor;
return instance;
}
// In prod we don't clone, we simply use the same instance for unaffected
// behavior. We have to keep the descriptor around for comparison later on.
// This should ideally be accepted in the constructor of the instance but
// since that is currently overloaded, we just manually attach it here.
descriptor._descriptor = descriptor;
return descriptor;
}
module.exports = instantiateReactComponent;
+17 -14
View File
@@ -20,33 +20,36 @@
"use strict";
/**
* Given a `prevComponent` and `nextComponent`, determines if `prevComponent`
* should be updated as opposed to being destroyed or replaced.
* Given a `prevComponentInstance` and `nextComponent`, determines if
* `prevComponentInstance` should be updated as opposed to being destroyed or
* replaced by a new instance. The second argument is a descriptor. Future
* versions of the reconciler should only compare descriptors to other
* descriptors.
*
* @param {?object} prevComponent
* @param {?object} nextComponent
* @return {boolean} True if `prevComponent` should be updated.
* @param {?object} prevComponentInstance
* @param {?object} nextDescriptor
* @return {boolean} True if `prevComponentInstance` should be updated.
* @protected
*/
function shouldUpdateReactComponent(prevComponent, nextComponent) {
function shouldUpdateReactComponent(prevComponentInstance, nextDescriptor) {
// TODO: Remove warning after a release.
if (prevComponent && nextComponent &&
prevComponent.constructor === nextComponent.constructor && (
(prevComponent.props && prevComponent.props.key) ===
(nextComponent.props && nextComponent.props.key)
if (prevComponentInstance && nextDescriptor &&
prevComponentInstance.constructor === nextDescriptor.constructor && (
(prevComponentInstance.props && prevComponentInstance.props.key) ===
(nextDescriptor.props && nextDescriptor.props.key)
)) {
if (prevComponent._owner === nextComponent._owner) {
if (prevComponentInstance._owner === nextDescriptor._owner) {
return true;
} else {
if (__DEV__) {
if (prevComponent.state) {
if (prevComponentInstance.state) {
console.warn(
'A recent change to React has been found to impact your code. ' +
'A mounted component will now be unmounted and replaced by a ' +
'component (of the same class) if their owners are different. ' +
'Previously, ownership was not considered when updating.',
prevComponent,
nextComponent
prevComponentInstance,
nextDescriptor
);
}
}
+2 -1
View File
@@ -126,7 +126,8 @@ var traverseAllChildrenImpl =
// All of the above are perceived as null.
callback(traverseContext, null, storageName, indexSoFar);
subtreeCount = 1;
} else if (children.mountComponentIntoNode) {
} else if (children.type && children.type.prototype &&
children.type.prototype.mountComponentIntoNode) {
callback(traverseContext, children, storageName, indexSoFar);
subtreeCount = 1;
} else {