Merge pull request #5451 from spicyj/empty-comments

Use comment nodes for empty components
facebook · Nov 20, 2015 · 907dee2 · 907dee2
2 parents 1cdbff2 + 3581406
commit 907dee2
Show file tree

Hide file tree

Showing 9 changed files with 163 additions and 45 deletions.
diff --git a/src/renderers/dom/client/ReactDOMComponentTree.js b/src/renderers/dom/client/ReactDOMComponentTree.js
@@ -88,7 +88,9 @@ function precacheChildNodes(inst, node) {
     // We assume the child nodes are in the same order as the child instances.
     for (; childNode !== null; childNode = childNode.nextSibling) {
       if (childNode.nodeType === 1 &&
-          childNode.getAttribute(ATTR_NAME) === String(childID)) {
+          childNode.getAttribute(ATTR_NAME) === String(childID) ||
+          childNode.nodeType === 8 &&
+          childNode.nodeValue === ' react-empty: ' + childID + ' ') {
         precacheNode(childInst, childNode);
         continue outer;
       }

diff --git a/src/renderers/dom/shared/Danger.js b/src/renderers/dom/shared/Danger.js
@@ -166,7 +166,7 @@ var Danger = {
     );
     invariant(markup, 'dangerouslyReplaceNodeWithMarkup(...): Missing markup.');
     invariant(
-      oldChild.tagName.toLowerCase() !== 'html',
+      oldChild.nodeName !== 'HTML',
       'dangerouslyReplaceNodeWithMarkup(...): Cannot replace markup of the ' +
       '<html> node. This is because browser quirks make this unreliable ' +
       'and/or slow. If you want to render to the root you must use ' +

diff --git a/src/renderers/dom/shared/ReactDOMEmptyComponent.js b/src/renderers/dom/shared/ReactDOMEmptyComponent.js
@@ -0,0 +1,68 @@
+/**
+ * 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 ReactDOMEmptyComponent
+ */
+
+'use strict';
+
+var DOMLazyTree = require('DOMLazyTree');
+var ReactDOMComponentTree = require('ReactDOMComponentTree');
+
+var assign = require('Object.assign');
+
+var ReactDOMEmptyComponent = function(instantiate) {
+  // ReactCompositeComponent uses this:
+  this._currentElement = null;
+  // ReactDOMComponentTree uses these:
+  this._nativeNode = null;
+  this._nativeParent = null;
+  this._nativeContainerInfo = null;
+  this._domID = null;
+};
+assign(ReactDOMEmptyComponent.prototype, {
+  construct: function(element) {
+  },
+  mountComponent: function(
+    transaction,
+    nativeParent,
+    nativeContainerInfo,
+    context
+  ) {
+    var domID = nativeContainerInfo._idCounter++;
+    this._domID = domID;
+    this._nativeParent = nativeParent;
+    this._nativeContainerInfo = nativeContainerInfo;
+
+    var nodeValue = ' react-empty: ' + this._domID + ' ';
+    if (transaction.useCreateElement) {
+      var ownerDocument = nativeContainerInfo._ownerDocument;
+      var node = ownerDocument.createComment(nodeValue);
+      ReactDOMComponentTree.precacheNode(this, node);
+      return DOMLazyTree(node);
+    } else {
+      if (transaction.renderToStaticMarkup) {
+        // Normally we'd insert a comment node, but since this is a situation
+        // where React won't take over (static pages), we can simply return
+        // nothing.
+        return '';
+      }
+      return '<!--' + nodeValue + '-->';
+    }
+  },
+  receiveComponent: function() {
+  },
+  getNativeNode: function() {
+    return ReactDOMComponentTree.getNodeFromInstance(this);
+  },
+  unmountComponent: function() {
+    ReactDOMComponentTree.uncacheNode(this);
+  },
+});
+
+module.exports = ReactDOMEmptyComponent;
diff --git a/src/renderers/dom/shared/ReactDOMTextComponent.js b/src/renderers/dom/shared/ReactDOMTextComponent.js
@@ -53,8 +53,8 @@ assign(ReactDOMTextComponent.prototype, {
     // TODO: This is really a ReactText (ReactNode), not a ReactElement
     this._currentElement = text;
     this._stringText = '' + text;
+    // ReactDOMComponentTree uses these:
     this._nativeNode = null;
-    // ReactDOMComponentTree uses this:
     this._nativeParent = null;
 
     // Properties

diff --git a/src/renderers/dom/shared/ReactDefaultInjection.js b/src/renderers/dom/shared/ReactDefaultInjection.js
@@ -22,6 +22,7 @@ var ReactComponentBrowserEnvironment =
   require('ReactComponentBrowserEnvironment');
 var ReactDOMComponent = require('ReactDOMComponent');
 var ReactDOMComponentTree = require('ReactDOMComponentTree');
+var ReactDOMEmptyComponent = require('ReactDOMEmptyComponent');
 var ReactDOMTreeTraversal = require('ReactDOMTreeTraversal');
 var ReactDOMTextComponent = require('ReactDOMTextComponent');
 var ReactDefaultBatchingStrategy = require('ReactDefaultBatchingStrategy');
@@ -79,7 +80,11 @@ function inject() {
   ReactInjection.DOMProperty.injectDOMPropertyConfig(HTMLDOMPropertyConfig);
   ReactInjection.DOMProperty.injectDOMPropertyConfig(SVGDOMPropertyConfig);
 
-  ReactInjection.EmptyComponent.injectEmptyComponent('noscript');
+  ReactInjection.EmptyComponent.injectEmptyComponentFactory(
+    function(instantiate) {
+      return new ReactDOMEmptyComponent(instantiate);
+    }
+  );
 
   ReactInjection.Updates.injectReconcileTransaction(
     ReactReconcileTransaction

diff --git a/src/renderers/shared/reconciler/ReactEmptyComponent.js b/src/renderers/shared/reconciler/ReactEmptyComponent.js
@@ -11,50 +11,19 @@
 
 'use strict';
 
-var ReactElement = require('ReactElement');
-var ReactReconciler = require('ReactReconciler');
-
-var assign = require('Object.assign');
-
-var placeholderElement;
+var emptyComponentFactory;
 
 var ReactEmptyComponentInjection = {
-  injectEmptyComponent: function(component) {
-    placeholderElement = ReactElement.createElement(component);
+  injectEmptyComponentFactory: function(factory) {
+    emptyComponentFactory = factory;
   },
 };
 
-var ReactEmptyComponent = function(instantiate) {
-  this._currentElement = null;
-  this._renderedComponent = instantiate(placeholderElement);
-};
-assign(ReactEmptyComponent.prototype, {
-  construct: function(element) {
-  },
-  mountComponent: function(
-    transaction,
-    nativeParent,
-    nativeContainerInfo,
-    context
-  ) {
-    return ReactReconciler.mountComponent(
-      this._renderedComponent,
-      transaction,
-      nativeParent,
-      nativeContainerInfo,
-      context
-    );
+var ReactEmptyComponent = {
+  create: function(instantiate) {
+    return emptyComponentFactory(instantiate);
   },
-  receiveComponent: function() {
-  },
-  getNativeNode: function() {
-    return ReactReconciler.getNativeNode(this._renderedComponent);
-  },
-  unmountComponent: function() {
-    ReactReconciler.unmountComponent(this._renderedComponent);
-    this._renderedComponent = null;
-  },
-});
+};
 
 ReactEmptyComponent.injection = ReactEmptyComponentInjection;
 

diff --git a/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js b/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js
@@ -0,0 +1,50 @@
+/**
+ * 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 ReactSimpleEmptyComponent
+ */
+
+'use strict';
+
+var ReactReconciler = require('ReactReconciler');
+
+var assign = require('Object.assign');
+
+var ReactSimpleEmptyComponent = function(placeholderElement, instantiate) {
+  this._currentElement = null;
+  this._renderedComponent = instantiate(placeholderElement);
+};
+assign(ReactSimpleEmptyComponent.prototype, {
+  construct: function(element) {
+  },
+  mountComponent: function(
+    transaction,
+    nativeParent,
+    nativeContainerInfo,
+    context
+  ) {
+    return ReactReconciler.mountComponent(
+      this._renderedComponent,
+      transaction,
+      nativeParent,
+      nativeContainerInfo,
+      context
+    );
+  },
+  receiveComponent: function() {
+  },
+  getNativeNode: function() {
+    return ReactReconciler.getNativeNode(this._renderedComponent);
+  },
+  unmountComponent: function() {
+    ReactReconciler.unmountComponent(this._renderedComponent);
+    this._renderedComponent = null;
+  },
+});
+
+module.exports = ReactSimpleEmptyComponent;
diff --git a/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js
@@ -106,6 +106,30 @@ describe('ReactEmptyComponent', function() {
     expect(log.argsForCall[3][0]).toBe(null);
   });
 
+  it('should be able to switch in a list of children', () => {
+    var instance1 =
+      <TogglingComponent
+        firstComponent={null}
+        secondComponent={'div'}
+      />;
+
+    ReactTestUtils.renderIntoDocument(
+      <div>
+        {instance1}
+        {instance1}
+        {instance1}
+      </div>
+    );
+
+    expect(log.argsForCall.length).toBe(6);
+    expect(log.argsForCall[0][0]).toBe(null);
+    expect(log.argsForCall[1][0]).toBe(null);
+    expect(log.argsForCall[2][0]).toBe(null);
+    expect(log.argsForCall[3][0].tagName).toBe('DIV');
+    expect(log.argsForCall[4][0].tagName).toBe('DIV');
+    expect(log.argsForCall[5][0].tagName).toBe('DIV');
+  });
+
   it('should distinguish between a script placeholder and an actual script tag',
     () => {
       var instance1 =
@@ -275,12 +299,12 @@ describe('ReactEmptyComponent', function() {
 
     ReactDOM.render(<Empty />, container);
     var noscript1 = container.firstChild;
-    expect(noscript1.tagName).toBe('NOSCRIPT');
+    expect(noscript1.nodeName).toBe('#comment');
 
     // This update shouldn't create a DOM node
     ReactDOM.render(<Empty />, container);
     var noscript2 = container.firstChild;
-    expect(noscript2.tagName).toBe('NOSCRIPT');
+    expect(noscript2.nodeName).toBe('#comment');
 
     expect(noscript1).toBe(noscript2);
   });

diff --git a/src/renderers/shared/reconciler/instantiateReactComponent.js b/src/renderers/shared/reconciler/instantiateReactComponent.js
@@ -67,7 +67,7 @@ function instantiateReactComponent(node) {
   var instance;
 
   if (node === null || node === false) {
-    instance = new ReactEmptyComponent(instantiateReactComponent);
+    instance = ReactEmptyComponent.create(instantiateReactComponent);
   } else if (typeof node === 'object') {
     var element = node;
     invariant(