From 974a4c84ce9d11d536ad3dd0237cc0017c1a6997 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 17 Nov 2014 00:26:03 -0800 Subject: [PATCH 1/4] Move mountComponentIntoNode and setProps out of ReactComponent This is part of moving more logic out of the base classes. setProps, replaceProps etc. are not accessible from anything other than ReactClass so we can safely move it to ReactCompositeComponent. mountComponentIntoNode is tightly coupled to ReactMount. It's part of the outer abstraction, the mount point, not the individual component. --- .../ui/ReactComponentBrowserEnvironment.js | 75 +----------- src/browser/ui/ReactMount.js | 84 ++++++++++++- src/core/ReactComponent.js | 115 ------------------ src/core/ReactCompositeComponent.js | 63 ++++++++++ src/test/ReactDefaultPerf.js | 2 +- src/test/ReactDefaultPerfAnalysis.js | 2 +- 6 files changed, 149 insertions(+), 192 deletions(-) diff --git a/src/browser/ui/ReactComponentBrowserEnvironment.js b/src/browser/ui/ReactComponentBrowserEnvironment.js index 787ed3f837b..ed8df43ae65 100644 --- a/src/browser/ui/ReactComponentBrowserEnvironment.js +++ b/src/browser/ui/ReactComponentBrowserEnvironment.js @@ -14,20 +14,9 @@ "use strict"; var ReactDOMIDOperations = require('ReactDOMIDOperations'); -var ReactMarkupChecksum = require('ReactMarkupChecksum'); var ReactMount = require('ReactMount'); -var ReactPerf = require('ReactPerf'); var ReactReconcileTransaction = require('ReactReconcileTransaction'); -var getReactRootElementInContainer = require('getReactRootElementInContainer'); -var invariant = require('invariant'); -var setInnerHTML = require('setInnerHTML'); - - -var ELEMENT_NODE_TYPE = 1; -var DOC_NODE_TYPE = 9; - - /** * Abstracts away all functionality of `ReactComponent` requires knowledge of * the browser context. @@ -46,70 +35,8 @@ var ReactComponentBrowserEnvironment = { */ unmountIDFromEnvironment: function(rootNodeID) { ReactMount.purgeID(rootNodeID); - }, - - /** - * @param {string} markup Markup string to place into the DOM Element. - * @param {DOMElement} container DOM Element to insert markup into. - * @param {boolean} shouldReuseMarkup Should reuse the existing markup in the - * container if possible. - */ - mountImageIntoNode: ReactPerf.measure( - 'ReactComponentBrowserEnvironment', - 'mountImageIntoNode', - function(markup, container, shouldReuseMarkup) { - invariant( - container && ( - container.nodeType === ELEMENT_NODE_TYPE || - container.nodeType === DOC_NODE_TYPE - ), - 'mountComponentIntoNode(...): Target container is not valid.' - ); - - if (shouldReuseMarkup) { - if (ReactMarkupChecksum.canReuseMarkup( - markup, - getReactRootElementInContainer(container))) { - return; - } else { - invariant( - container.nodeType !== DOC_NODE_TYPE, - 'You\'re trying to render a component to the document using ' + - 'server rendering but the checksum was invalid. This usually ' + - 'means you rendered a different component type or props on ' + - 'the client from the one on the server, or your render() ' + - 'methods are impure. React cannot handle this case due to ' + - 'cross-browser quirks by rendering at the document root. You ' + - 'should look for environment dependent code in your components ' + - 'and ensure the props are the same client and server side.' - ); - - if (__DEV__) { - console.warn( - 'React attempted to use reuse markup in a container but the ' + - 'checksum was invalid. This generally means that you are ' + - 'using server rendering and the markup generated on the ' + - 'server was not what the client was expecting. React injected ' + - 'new markup to compensate which works but you have lost many ' + - 'of the benefits of server rendering. Instead, figure out ' + - 'why the markup being generated is different on the client ' + - 'or server.' - ); - } - } - } - - invariant( - container.nodeType !== DOC_NODE_TYPE, - 'You\'re trying to render a component to the document but ' + - 'you didn\'t use server rendering. We can\'t do this ' + - 'without using server rendering due to cross-browser quirks. ' + - 'See renderComponentToString() for server rendering.' - ); + } - setInnerHTML(container, markup); - } - ) }; module.exports = ReactComponentBrowserEnvironment; diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index 79611c9479a..edd5c0d0d83 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -18,13 +18,16 @@ var ReactElement = require('ReactElement'); var ReactEmptyComponent = require('ReactEmptyComponent'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactInstanceMap = require('ReactInstanceMap'); +var ReactMarkupChecksum = require('ReactMarkupChecksum'); var ReactPerf = require('ReactPerf'); +var ReactUpdates = require('ReactUpdates'); var containsNode = require('containsNode'); var deprecated = require('deprecated'); var getReactRootElementInContainer = require('getReactRootElementInContainer'); var instantiateReactComponent = require('instantiateReactComponent'); var invariant = require('invariant'); +var setInnerHTML = require('setInnerHTML'); var shouldUpdateReactComponent = require('shouldUpdateReactComponent'); var warning = require('warning'); @@ -208,6 +211,23 @@ function findDeepestCachedAncestor(targetID) { return foundNode; } +/** + * Mounts this component and inserts it into the DOM. + * + * @param {string} rootID DOM ID of the root node. + * @param {DOMElement} container DOM element to mount into. + * @param {ReactReconcileTransaction} transaction + * @param {boolean} shouldReuseMarkup If true, do not insert markup + */ +function mountComponentIntoNode( + rootID, + container, + transaction, + shouldReuseMarkup) { + var markup = this.mountComponent(rootID, transaction, 0); + ReactMount._mountImageIntoNode(markup, container, shouldReuseMarkup); +} + /** * Mounting is the process of initializing a React component by creatings its * representative DOM elements and inserting them into a supplied `container`. @@ -321,11 +341,17 @@ var ReactMount = { componentInstance, container ); - componentInstance.mountComponentIntoNode( + + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(); + transaction.perform( + mountComponentIntoNode, + componentInstance, reactRootID, container, + transaction, shouldReuseMarkup ); + ReactUpdates.ReactReconcileTransaction.release(transaction); if (__DEV__) { // Record the root element in case it later gets transplanted. @@ -684,6 +710,62 @@ var ReactMount = { ); }, + _mountImageIntoNode: ReactPerf.measure( + 'ReactMount', + '_mountImageIntoNode', + function(markup, container, shouldReuseMarkup) { + invariant( + container && ( + container.nodeType === ELEMENT_NODE_TYPE || + container.nodeType === DOC_NODE_TYPE + ), + 'mountComponentIntoNode(...): Target container is not valid.' + ); + + if (shouldReuseMarkup) { + if (ReactMarkupChecksum.canReuseMarkup( + markup, + getReactRootElementInContainer(container))) { + return; + } else { + invariant( + container.nodeType !== DOC_NODE_TYPE, + 'You\'re trying to render a component to the document using ' + + 'server rendering but the checksum was invalid. This usually ' + + 'means you rendered a different component type or props on ' + + 'the client from the one on the server, or your render() ' + + 'methods are impure. React cannot handle this case due to ' + + 'cross-browser quirks by rendering at the document root. You ' + + 'should look for environment dependent code in your components ' + + 'and ensure the props are the same client and server side.' + ); + + if (__DEV__) { + console.warn( + 'React attempted to use reuse markup in a container but the ' + + 'checksum was invalid. This generally means that you are ' + + 'using server rendering and the markup generated on the ' + + 'server was not what the client was expecting. React injected ' + + 'new markup to compensate which works but you have lost many ' + + 'of the benefits of server rendering. Instead, figure out ' + + 'why the markup being generated is different on the client ' + + 'or server.' + ); + } + } + } + + invariant( + container.nodeType !== DOC_NODE_TYPE, + 'You\'re trying to render a component to the document but ' + + 'you didn\'t use server rendering. We can\'t do this ' + + 'without using server rendering due to cross-browser quirks. ' + + 'See renderComponentToString() for server rendering.' + ); + + setInnerHTML(container, markup); + } + ), /** * React ID utilities. diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 39f7381ccb0..fab20b257be 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -31,17 +31,6 @@ var injected = false; */ var unmountIDFromEnvironment = null; -/** - * The "image" of a component tree, is the platform specific (typically - * serialized) data that represents a tree of lower level UI building blocks. - * On the web, this "image" is HTML markup which describes a construction of - * low level `div` and `span` nodes. Other platforms may have different - * encoding of this "image". This must be injected. - * - * @private - */ -var mountImageIntoNode = null; - function attachRef(ref, component, owner) { if (ref instanceof ReactRef) { ReactRef.attachRef(ref, component); @@ -91,7 +80,6 @@ var ReactComponent = { !injected, 'ReactComponent: injectEnvironment() can only be called once.' ); - mountImageIntoNode = ReactComponentEnvironment.mountImageIntoNode; unmountIDFromEnvironment = ReactComponentEnvironment.unmountIDFromEnvironment; ReactComponent.BackendIDOperations = @@ -117,69 +105,6 @@ var ReactComponent = { */ Mixin: { - /** - * Sets a subset of the props. - * - * @param {object} partialProps Subset of the next props. - * @param {?function} callback Called after props are updated. - * @final - * @public - */ - setProps: function(partialProps, callback) { - // Merge with the pending element if it exists, otherwise with existing - // element props. - var element = this._pendingElement || this._currentElement; - this.replaceProps( - assign({}, element.props, partialProps), - callback - ); - }, - - /** - * Replaces all of the props. - * - * @param {object} props New props. - * @param {?function} callback Called after props are updated. - * @final - * @public - */ - replaceProps: function(props, callback) { - invariant( - this._mountDepth === 0, - 'replaceProps(...): You called `setProps` or `replaceProps` on a ' + - 'component with a parent. This is an anti-pattern since props will ' + - 'get reactively updated when rendered. Instead, change the owner\'s ' + - '`render` method to pass the correct value as props to the component ' + - 'where it is created.' - ); - // This is a deoptimized path. We optimize for always having an element. - // This creates an extra internal element. - this._pendingElement = ReactElement.cloneAndReplaceProps( - this._pendingElement || this._currentElement, - props - ); - ReactUpdates.enqueueUpdate(this, callback); - }, - - /** - * Schedule a partial update to the props. Only used for internal testing. - * - * @param {object} partialProps Subset of the next props. - * @param {?function} callback Called after props are updated. - * @final - * @internal - */ - _setPropsInternal: function(partialProps, callback) { - // This is a deoptimized path. We optimize for always having an element. - // This creates an extra internal element. - var element = this._pendingElement || this._currentElement; - this._pendingElement = ReactElement.cloneAndReplaceProps( - element, - assign({}, element.props, partialProps) - ); - ReactUpdates.enqueueUpdate(this, callback); - }, - /** * Base constructor for all React components. * @@ -312,46 +237,6 @@ var ReactComponent = { } }, - /** - * Mounts this component and inserts it into the DOM. - * - * @param {string} rootID DOM ID of the root node. - * @param {DOMElement} container DOM element to mount into. - * @param {boolean} shouldReuseMarkup If true, do not insert markup - * @final - * @internal - * @see {ReactMount.render} - */ - mountComponentIntoNode: function(rootID, container, shouldReuseMarkup) { - var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(); - transaction.perform( - this._mountComponentIntoNode, - this, - rootID, - container, - transaction, - shouldReuseMarkup - ); - ReactUpdates.ReactReconcileTransaction.release(transaction); - }, - - /** - * @param {string} rootID DOM ID of the root node. - * @param {DOMElement} container DOM element to mount into. - * @param {ReactReconcileTransaction} transaction - * @param {boolean} shouldReuseMarkup If true, do not insert markup - * @final - * @private - */ - _mountComponentIntoNode: function( - rootID, - container, - transaction, - shouldReuseMarkup) { - var markup = this.mountComponent(rootID, transaction, 0); - mountImageIntoNode(markup, container, shouldReuseMarkup); - }, - /** * Get the publicly accessible representation of this component - i.e. what * is exposed by refs and renderComponent. Can be null for stateless diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 2ead20ee739..c02f91e080e 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -248,6 +248,69 @@ var ReactCompositeComponentMixin = assign({}, // TODO: inst.context = null; }, + /** + * Sets a subset of the props. + * + * @param {object} partialProps Subset of the next props. + * @param {?function} callback Called after props are updated. + * @final + * @public + */ + setProps: function(partialProps, callback) { + // Merge with the pending element if it exists, otherwise with existing + // element props. + var element = this._pendingElement || this._currentElement; + this.replaceProps( + assign({}, element.props, partialProps), + callback + ); + }, + + /** + * Replaces all of the props. + * + * @param {object} props New props. + * @param {?function} callback Called after props are updated. + * @final + * @public + */ + replaceProps: function(props, callback) { + invariant( + this._mountDepth === 0, + 'replaceProps(...): You called `setProps` or `replaceProps` on a ' + + 'component with a parent. This is an anti-pattern since props will ' + + 'get reactively updated when rendered. Instead, change the owner\'s ' + + '`render` method to pass the correct value as props to the component ' + + 'where it is created.' + ); + // This is a deoptimized path. We optimize for always having an element. + // This creates an extra internal element. + this._pendingElement = ReactElement.cloneAndReplaceProps( + this._pendingElement || this._currentElement, + props + ); + ReactUpdates.enqueueUpdate(this, callback); + }, + + /** + * Schedule a partial update to the props. Only used for internal testing. + * + * @param {object} partialProps Subset of the next props. + * @param {?function} callback Called after props are updated. + * @final + * @internal + */ + _setPropsInternal: function(partialProps, callback) { + // This is a deoptimized path. We optimize for always having an element. + // This creates an extra internal element. + var element = this._pendingElement || this._currentElement; + this._pendingElement = ReactElement.cloneAndReplaceProps( + element, + assign({}, element.props, partialProps) + ); + ReactUpdates.enqueueUpdate(this, callback); + }, + /** * Sets a subset of the state. This only exists because _pendingState is * internal. This provides a merging strategy that is not available to deep diff --git a/src/test/ReactDefaultPerf.js b/src/test/ReactDefaultPerf.js index d91a61d70ed..cbc5d0150a4 100644 --- a/src/test/ReactDefaultPerf.js +++ b/src/test/ReactDefaultPerf.js @@ -169,7 +169,7 @@ var ReactDefaultPerf = { rv = func.apply(this, args); totalTime = performanceNow() - start; - if (fnName === 'mountImageIntoNode') { + if (fnName === '_mountImageIntoNode') { var mountID = ReactMount.getID(args[1]); ReactDefaultPerf._recordWrite(mountID, fnName, totalTime, args[0]); } else if (fnName === 'dangerouslyProcessChildrenUpdates') { diff --git a/src/test/ReactDefaultPerfAnalysis.js b/src/test/ReactDefaultPerfAnalysis.js index 57b6b957e39..79b22f5e2a9 100644 --- a/src/test/ReactDefaultPerfAnalysis.js +++ b/src/test/ReactDefaultPerfAnalysis.js @@ -14,7 +14,7 @@ var assign = require('Object.assign'); // Don't try to save users less than 1.2ms (a number I made up) var DONT_CARE_THRESHOLD = 1.2; var DOM_OPERATION_TYPES = { - 'mountImageIntoNode': 'set innerHTML', + '_mountImageIntoNode': 'set innerHTML', INSERT_MARKUP: 'set innerHTML', MOVE_EXISTING: 'move', REMOVE_NODE: 'remove', From bc4dd411b06b3ae22f16ec622c6f2ca8de44bf6b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 17 Nov 2014 00:48:08 -0800 Subject: [PATCH 2/4] Move _pendingX into ReactCompositeComponent Since setProps can no longer be called on anything but composites, we can move this complexity into ReactCompositeComponent. --- src/browser/ui/ReactDOMComponent.js | 8 +++--- src/core/ReactComponent.js | 39 ----------------------------- src/core/ReactCompositeComponent.js | 14 +++++++---- 3 files changed, 12 insertions(+), 49 deletions(-) diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 3bdae4cb36e..9ce81b14689 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -291,11 +291,9 @@ ReactDOMComponent.Mixin = { return; } - ReactComponent.Mixin.receiveComponent.call( - this, - nextElement, - transaction - ); + var prevElement = this._currentElement; + this._currentElement = nextElement; + this.updateComponent(transaction, prevElement, nextElement); }, /** diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index fab20b257be..3803f891f59 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -115,13 +115,9 @@ var ReactComponent = { * @internal */ construct: function(element) { - // See ReactUpdates. - this._pendingCallbacks = null; - // We keep the old element and a reference to the pending element // to track updates. this._currentElement = element; - this._pendingElement = null; }, /** @@ -167,41 +163,6 @@ var ReactComponent = { unmountIDFromEnvironment(this._rootNodeID); // Reset all fields this._rootNodeID = null; - this._pendingCallbacks = null; - this._pendingElement = null; - }, - - /** - * Given a new instance of this component, updates the rendered DOM nodes - * as if that instance was rendered instead. - * - * Subclasses that override this method should make sure to invoke - * `ReactComponent.Mixin.receiveComponent.call(this, ...)`. - * - * @param {object} nextComponent Next set of properties. - * @param {ReactReconcileTransaction} transaction - * @internal - */ - receiveComponent: function(nextElement, transaction) { - this._pendingElement = nextElement; - this.performUpdateIfNecessary(transaction); - }, - - /** - * If `_pendingElement` is set, update the component. - * - * @param {ReactReconcileTransaction} transaction - * @internal - */ - performUpdateIfNecessary: function(transaction) { - if (this._pendingElement == null) { - return; - } - var prevElement = this._currentElement; - var nextElement = this._pendingElement; - this._currentElement = nextElement; - this._pendingElement = null; - this.updateComponent(transaction, prevElement, nextElement); }, /** diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index c02f91e080e..bd076698bef 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -115,11 +115,15 @@ var ReactCompositeComponentMixin = assign({}, this._instance.context = null; this._instance.refs = emptyObject; + this._pendingElement = null; this._pendingState = null; this._compositeLifeCycleState = null; // Children can be either an array or more than one argument ReactComponent.Mixin.construct.apply(this, arguments); + + // See ReactUpdates. + this._pendingCallbacks = null; }, /** @@ -234,6 +238,9 @@ var ReactCompositeComponentMixin = assign({}, // Reset pending fields this._pendingState = null; this._pendingForceUpdate = false; + this._pendingCallbacks = null; + this._pendingElement = null; + ReactComponent.Mixin.unmountComponent.call(this); // Delete the reference from the instance to this internal representation @@ -505,11 +512,8 @@ var ReactCompositeComponentMixin = assign({}, return; } - ReactComponent.Mixin.receiveComponent.call( - this, - nextElement, - transaction - ); + this._pendingElement = nextElement; + this.performUpdateIfNecessary(transaction); }, /** From fb17e8ca56a9b7688e7ccad7833d8a805ffc6ad5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 17 Nov 2014 01:58:48 -0800 Subject: [PATCH 3/4] Ensure that all internal instances have consistent properties Use preventExtensions so that we can't add expando properties to internal instances. This ensures that the hidden class is kept more consistent. --- src/browser/ui/ReactDOMComponent.js | 2 ++ src/browser/ui/ReactDOMTextComponent.js | 4 ++++ src/core/ReactComponent.js | 4 ++++ src/core/ReactCompositeComponent.js | 3 +++ src/core/instantiateReactComponent.js | 8 ++++++++ 5 files changed, 21 insertions(+) diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 9ce81b14689..ac932dc90b2 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -147,6 +147,8 @@ function validateDangerousTag(tag) { function ReactDOMComponent(tag) { validateDangerousTag(tag); this._tag = tag; + this._renderedChildren = null; + this._previousStyleCopy = null; } ReactDOMComponent.displayName = 'ReactDOMComponent'; diff --git a/src/browser/ui/ReactDOMTextComponent.js b/src/browser/ui/ReactDOMTextComponent.js index 6e63d9dbacf..00e18fb8518 100644 --- a/src/browser/ui/ReactDOMTextComponent.js +++ b/src/browser/ui/ReactDOMTextComponent.js @@ -50,6 +50,10 @@ assign(ReactDOMTextComponent.prototype, { // TODO: This is really a ReactText (ReactNode), not a ReactElement this._currentElement = text; this._stringText = '' + text; + + // Properties + this._rootNodeID = null; + this._mountIndex = 0; }, /** diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 3803f891f59..8822ccb753f 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -118,6 +118,10 @@ var ReactComponent = { // We keep the old element and a reference to the pending element // to track updates. this._currentElement = element; + + this._rootNodeID = null; + this._mountIndex = 0; + this._mountDepth = 0; }, /** diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index bd076698bef..1d4c25c9f1a 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -117,8 +117,11 @@ var ReactCompositeComponentMixin = assign({}, this._pendingElement = null; this._pendingState = null; + this._pendingForceUpdate = false; this._compositeLifeCycleState = null; + this._renderedComponent = null; + // Children can be either an array or more than one argument ReactComponent.Mixin.construct.apply(this, arguments); diff --git a/src/core/instantiateReactComponent.js b/src/core/instantiateReactComponent.js index ef5887974de..593f54c432f 100644 --- a/src/core/instantiateReactComponent.js +++ b/src/core/instantiateReactComponent.js @@ -118,6 +118,14 @@ function instantiateReactComponent(node, parentCompositeType) { // Sets up the instance. This can probably just move into the constructor now. instance.construct(node); + // Internal instances should fully constructed at this point, so they should + // not get any new fields added to them at this point. + if (__DEV__) { + if (Object.preventExtensions) { + Object.preventExtensions(instance); + } + } + return instance; } From 6dddd60e332c2bcead082f29d241ecf209037c87 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 17 Nov 2014 11:07:16 -0800 Subject: [PATCH 4/4] Unused variables --- src/core/ReactComponent.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 8822ccb753f..2e2a30e477d 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -11,14 +11,10 @@ "use strict"; -var ReactElement = require('ReactElement'); var ReactOwner = require('ReactOwner'); var ReactRef = require('ReactRef'); -var ReactUpdates = require('ReactUpdates'); -var assign = require('Object.assign'); var invariant = require('invariant'); -var keyMirror = require('keyMirror'); var injected = false;