diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 56e2c801ea..f34aca941e 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -16,7 +16,7 @@ var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var ReactElement = require('ReactElement'); -var ReactEmptyComponent = require('ReactEmptyComponent'); +var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactMarkupChecksum = require('ReactMarkupChecksum'); @@ -181,7 +181,7 @@ function getNode(id) { */ function getNodeFromInstance(instance) { var id = ReactInstanceMap.get(instance)._rootNodeID; - if (ReactEmptyComponent.isNullComponentID(id)) { + if (ReactEmptyComponentRegistry.isNullComponentID(id)) { return null; } if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { diff --git a/src/renderers/shared/reconciler/ReactChildReconciler.js b/src/renderers/shared/reconciler/ReactChildReconciler.js index fdcd80633f..2b2a63a666 100644 --- a/src/renderers/shared/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/reconciler/ReactChildReconciler.js @@ -90,7 +90,8 @@ var ReactChildReconciler = { var prevChild = prevChildren && prevChildren[name]; var prevElement = prevChild && prevChild._currentElement; var nextElement = nextChildren[name]; - if (shouldUpdateReactComponent(prevElement, nextElement)) { + if (prevChild != null && + shouldUpdateReactComponent(prevElement, nextElement)) { ReactReconciler.receiveComponent( prevChild, nextElement, transaction, context ); diff --git a/src/renderers/shared/reconciler/ReactEmptyComponent.js b/src/renderers/shared/reconciler/ReactEmptyComponent.js index 6420b1a397..715dece5f1 100644 --- a/src/renderers/shared/reconciler/ReactEmptyComponent.js +++ b/src/renderers/shared/reconciler/ReactEmptyComponent.js @@ -12,81 +12,47 @@ 'use strict'; var ReactElement = require('ReactElement'); -var ReactInstanceMap = require('ReactInstanceMap'); +var ReactEmptyComponentRegistry = require('ReactEmptyComponentRegistry'); +var ReactReconciler = require('ReactReconciler'); -var invariant = require('invariant'); +var assign = require('Object.assign'); -var component; -// This registry keeps track of the React IDs of the components that rendered to -// `null` (in reality a placeholder such as `noscript`) -var nullComponentIDsRegistry = {}; +var placeholderElement; var ReactEmptyComponentInjection = { - injectEmptyComponent: function(emptyComponent) { - component = ReactElement.createFactory(emptyComponent); + injectEmptyComponent: function(component) { + placeholderElement = ReactElement.createElement(component); }, }; -var ReactEmptyComponentType = function() {}; -ReactEmptyComponentType.isReactClass = {}; -ReactEmptyComponentType.prototype.componentDidMount = function() { - var internalInstance = ReactInstanceMap.get(this); - // TODO: Make sure we run these methods in the correct order, we shouldn't - // need this check. We're going to assume if we're here it means we ran - // componentWillUnmount already so there is no internal instance (it gets - // removed as part of the unmounting process). - if (!internalInstance) { - return; - } - registerNullComponentID(internalInstance._rootNodeID); -}; -ReactEmptyComponentType.prototype.componentWillUnmount = function() { - var internalInstance = ReactInstanceMap.get(this); - // TODO: Get rid of this check. See TODO in componentDidMount. - if (!internalInstance) { - return; - } - deregisterNullComponentID(internalInstance._rootNodeID); -}; -ReactEmptyComponentType.prototype.render = function() { - invariant( - component, - 'Trying to return null from a render, but no null placeholder component ' + - 'was injected.' - ); - return component(); +var ReactEmptyComponent = function(instantiate) { + this._currentElement = null; + this._rootNodeID = null; + this._renderedComponent = instantiate(placeholderElement); }; +assign(ReactEmptyComponent.prototype, { + construct: function(element) { + }, + mountComponent: function(rootID, transaction, context) { + ReactEmptyComponentRegistry.registerNullComponentID(rootID); + this._rootNodeID = rootID; + return ReactReconciler.mountComponent( + this._renderedComponent, + rootID, + transaction, + context + ); + }, + receiveComponent: function() { + }, + unmountComponent: function(rootID, transaction, context) { + ReactReconciler.unmountComponent(this._renderedComponent); + ReactEmptyComponentRegistry.deregisterNullComponentID(this._rootNodeID); + this._rootNodeID = null; + this._renderedComponent = null; + }, +}); -var emptyElement = ReactElement.createElement(ReactEmptyComponentType); - -/** - * Mark the component as having rendered to null. - * @param {string} id Component's `_rootNodeID`. - */ -function registerNullComponentID(id) { - nullComponentIDsRegistry[id] = true; -} - -/** - * Unmark the component as having rendered to null: it renders to something now. - * @param {string} id Component's `_rootNodeID`. - */ -function deregisterNullComponentID(id) { - delete nullComponentIDsRegistry[id]; -} - -/** - * @param {string} id Component's `_rootNodeID`. - * @return {boolean} True if the component is rendered to null. - */ -function isNullComponentID(id) { - return !!nullComponentIDsRegistry[id]; -} - -var ReactEmptyComponent = { - emptyElement: emptyElement, - injection: ReactEmptyComponentInjection, - isNullComponentID: isNullComponentID, -}; +ReactEmptyComponent.injection = ReactEmptyComponentInjection; module.exports = ReactEmptyComponent; diff --git a/src/renderers/shared/reconciler/ReactEmptyComponentRegistry.js b/src/renderers/shared/reconciler/ReactEmptyComponentRegistry.js new file mode 100644 index 0000000000..43f1808f62 --- /dev/null +++ b/src/renderers/shared/reconciler/ReactEmptyComponentRegistry.js @@ -0,0 +1,48 @@ +/** + * Copyright 2014-2015, 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 ReactEmptyComponentRegistry + */ + +'use strict'; + +// This registry keeps track of the React IDs of the components that rendered to +// `null` (in reality a placeholder such as `noscript`) +var nullComponentIDsRegistry = {}; + +/** + * @param {string} id Component's `_rootNodeID`. + * @return {boolean} True if the component is rendered to null. + */ +function isNullComponentID(id) { + return !!nullComponentIDsRegistry[id]; +} + +/** + * Mark the component as having rendered to null. + * @param {string} id Component's `_rootNodeID`. + */ +function registerNullComponentID(id) { + nullComponentIDsRegistry[id] = true; +} + +/** + * Unmark the component as having rendered to null: it renders to something now. + * @param {string} id Component's `_rootNodeID`. + */ +function deregisterNullComponentID(id) { + delete nullComponentIDsRegistry[id]; +} + +var ReactEmptyComponentRegistry = { + isNullComponentID: isNullComponentID, + registerNullComponentID: registerNullComponentID, + deregisterNullComponentID: deregisterNullComponentID, +}; + +module.exports = ReactEmptyComponentRegistry; diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index a5af42d2a2..ab3fbb0f6c 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -35,7 +35,8 @@ var ReactReconciler = { */ mountComponent: function(internalInstance, rootID, transaction, context) { var markup = internalInstance.mountComponent(rootID, transaction, context); - if (internalInstance._currentElement.ref != null) { + if (internalInstance._currentElement && + internalInstance._currentElement.ref != null) { transaction.getReactMountReady().enqueue(attachRefs, internalInstance); } return markup; @@ -93,7 +94,9 @@ var ReactReconciler = { internalInstance.receiveComponent(nextElement, transaction, context); - if (refsChanged) { + if (refsChanged && + internalInstance._currentElement && + internalInstance._currentElement.ref != null) { transaction.getReactMountReady().enqueue(attachRefs, internalInstance); } }, diff --git a/src/renderers/shared/reconciler/ReactRef.js b/src/renderers/shared/reconciler/ReactRef.js index 58005797ca..bcee29ebf2 100644 --- a/src/renderers/shared/reconciler/ReactRef.js +++ b/src/renderers/shared/reconciler/ReactRef.js @@ -34,6 +34,9 @@ function detachRef(ref, component, owner) { } ReactRef.attachRefs = function(instance, element) { + if (element === null || element === false) { + return; + } var ref = element.ref; if (ref != null) { attachRef(ref, instance, element._owner); @@ -53,13 +56,21 @@ ReactRef.shouldUpdateRefs = function(prevElement, nextElement) { // is made. It probably belongs where the key checking and // instantiateReactComponent is done. + var prevEmpty = prevElement === null || prevElement === false; + var nextEmpty = nextElement === null || nextElement === false; + return ( + // This has a few false positives w/r/t empty components. + prevEmpty || nextEmpty || nextElement._owner !== prevElement._owner || nextElement.ref !== prevElement.ref ); }; ReactRef.detachRefs = function(instance, element) { + if (element === null || element === false) { + return; + } var ref = element.ref; if (ref != null) { detachRef(ref, instance, element._owner); diff --git a/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js index e73e887c16..527826b8ea 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js @@ -13,7 +13,6 @@ var React; var ReactDOM; -var ReactEmptyComponent; var ReactTestUtils; var TogglingComponent; @@ -25,7 +24,6 @@ describe('ReactEmptyComponent', function() { React = require('React'); ReactDOM = require('ReactDOM'); - ReactEmptyComponent = require('ReactEmptyComponent'); ReactTestUtils = require('ReactTestUtils'); reactComponentExpect = require('reactComponentExpect'); @@ -64,10 +62,10 @@ describe('ReactEmptyComponent', function() { var instance2 = ReactTestUtils.renderIntoDocument(); reactComponentExpect(instance1) .expectRenderedChild() - .toBeComponentOfType(ReactEmptyComponent.emptyElement.type); + .toBeEmptyComponent(); reactComponentExpect(instance2) .expectRenderedChild() - .toBeComponentOfType(ReactEmptyComponent.emptyElement.type); + .toBeEmptyComponent(); }); it('should still throw when rendering to undefined', () => { @@ -97,10 +95,8 @@ describe('ReactEmptyComponent', function() { secondComponent={null} />; - expect(function() { - ReactTestUtils.renderIntoDocument(instance1); - ReactTestUtils.renderIntoDocument(instance2); - }).not.toThrow(); + ReactTestUtils.renderIntoDocument(instance1); + ReactTestUtils.renderIntoDocument(instance2); expect(console.log.argsForCall.length).toBe(4); expect(console.log.argsForCall[0][0]).toBe(null); @@ -269,4 +265,25 @@ describe('ReactEmptyComponent', function() { ReactTestUtils.renderIntoDocument(); }).not.toThrow(); }); + + it('preserves the dom node during updates', function() { + var Empty = React.createClass({ + render: function() { + return null; + }, + }); + + var container = document.createElement('div'); + + ReactDOM.render(, container); + var noscript1 = container.firstChild; + expect(noscript1.tagName).toBe('NOSCRIPT'); + + // This update shouldn't create a DOM node + ReactDOM.render(, container); + var noscript2 = container.firstChild; + expect(noscript2.tagName).toBe('NOSCRIPT'); + + expect(noscript1).toBe(noscript2); + }); }); diff --git a/src/renderers/shared/reconciler/instantiateReactComponent.js b/src/renderers/shared/reconciler/instantiateReactComponent.js index 2a76e600d0..8a320f4fdc 100644 --- a/src/renderers/shared/reconciler/instantiateReactComponent.js +++ b/src/renderers/shared/reconciler/instantiateReactComponent.js @@ -67,10 +67,8 @@ function instantiateReactComponent(node) { var instance; if (node === null || node === false) { - node = ReactEmptyComponent.emptyElement; - } - - if (typeof node === 'object') { + instance = new ReactEmptyComponent(instantiateReactComponent); + } else if (typeof node === 'object') { var element = node; invariant( element && (typeof element.type === 'function' || @@ -86,7 +84,7 @@ function instantiateReactComponent(node) { instance = ReactNativeComponent.createInternalComponent(element); } else if (isInternalComponentType(element.type)) { // This is temporarily available for custom components that are not string - // represenations. I.e. ART. Once those are updated to use the string + // representations. I.e. ART. Once those are updated to use the string // representation, we can drop this code path. instance = new element.type(element); } else { diff --git a/src/renderers/shared/reconciler/shouldUpdateReactComponent.js b/src/renderers/shared/reconciler/shouldUpdateReactComponent.js index f80cdde268..3ffb75625e 100644 --- a/src/renderers/shared/reconciler/shouldUpdateReactComponent.js +++ b/src/renderers/shared/reconciler/shouldUpdateReactComponent.js @@ -24,18 +24,22 @@ * @protected */ function shouldUpdateReactComponent(prevElement, nextElement) { - if (prevElement != null && nextElement != null) { - var prevType = typeof prevElement; - var nextType = typeof nextElement; - if (prevType === 'string' || prevType === 'number') { - return (nextType === 'string' || nextType === 'number'); - } else { - return ( - nextType === 'object' && - prevElement.type === nextElement.type && - prevElement.key === nextElement.key - ); - } + var prevEmpty = prevElement === null || prevElement === false; + var nextEmpty = nextElement === null || nextElement === false; + if (prevEmpty || nextEmpty) { + return prevEmpty === nextEmpty; + } + + var prevType = typeof prevElement; + var nextType = typeof nextElement; + if (prevType === 'string' || prevType === 'number') { + return (nextType === 'string' || nextType === 'number'); + } else { + return ( + nextType === 'object' && + prevElement.type === nextElement.type && + prevElement.key === nextElement.key + ); } return false; } diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index 5955046fb0..7bc8cd0455 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -17,7 +17,6 @@ var EventPropagators = require('EventPropagators'); var React = require('React'); var ReactDOM = require('ReactDOM'); var ReactElement = require('ReactElement'); -var ReactEmptyComponent = require('ReactEmptyComponent'); var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactCompositeComponent = require('ReactCompositeComponent'); var ReactInstanceHandles = require('ReactInstanceHandles'); @@ -359,9 +358,7 @@ ReactShallowRenderer.prototype.getRenderOutput = function() { var NoopInternalComponent = function(element) { this._renderedOutput = element; - this._currentElement = element === null || element === false ? - ReactEmptyComponent.emptyElement : - element; + this._currentElement = element; }; NoopInternalComponent.prototype = { @@ -371,9 +368,7 @@ NoopInternalComponent.prototype = { receiveComponent: function(element) { this._renderedOutput = element; - this._currentElement = element === null || element === false ? - ReactEmptyComponent.emptyElement : - element; + this._currentElement = element; }, unmountComponent: function() { diff --git a/src/test/reactComponentExpect.js b/src/test/reactComponentExpect.js index 7c40909dad..a35f0cd6df 100644 --- a/src/test/reactComponentExpect.js +++ b/src/test/reactComponentExpect.js @@ -150,6 +150,11 @@ assign(reactComponentExpectInternal.prototype, { return this; }, + toBeEmptyComponent: function() { + var element = this._instance._currentElement; + return element === null || element === false; + }, + toBePresent: function() { expect(this.instance()).toBeTruthy(); return this;