@samitouri / QOS-React / commits / 358140679c

Use comment nodes for empty components

This makes more sense and avoids DOM nesting problems. ![image](https://cloud.githubusercontent.com/assets/6820/11098713/952348ca-885b-11e5-9757-e4a76467b0b8.png) (ReactSimpleEmptyComponent isn't used here but React Native can use it as it currently does.)

Ben Alpert committed Nov 11, 2015 at 10:04 UTC 358140679cbda686f8c3a7d04887342188572cb0
9 files changed +163 -45
src/renderers/dom/client/ReactDOMComponentTree.js
+3 -1
@@ -85,7 +85,9 @@ function precacheChildNodes(inst, node) {
85 // We assume the child nodes are in the same order as the child instances.
86 for (; childNode !== null; childNode = childNode.nextSibling) {
87 if (childNode.nodeType === 1 &&
88 - childNode.getAttribute(ATTR_NAME) === String(childID)) {
88 + childNode.getAttribute(ATTR_NAME) === String(childID) ||
89 + childNode.nodeType === 8 &&
90 + childNode.nodeValue === ' react-empty: ' + childID + ' ') {
91 precacheNode(childInst, childNode);
92 continue outer;
93 }
src/renderers/dom/shared/Danger.js
+1 -1
@@ -166,7 +166,7 @@ var Danger = {
166 );
167 invariant(markup, 'dangerouslyReplaceNodeWithMarkup(...): Missing markup.');
168 invariant(
169 - oldChild.tagName.toLowerCase() !== 'html',
169 + oldChild.nodeName !== 'HTML',
170 'dangerouslyReplaceNodeWithMarkup(...): Cannot replace markup of the ' +
171 '<html> node. This is because browser quirks make this unreliable ' +
172 'and/or slow. If you want to render to the root you must use ' +
src/renderers/dom/shared/ReactDOMEmptyComponent.js new
+68
@@ -0,0 +1,68 @@
1 +/**
2 + * Copyright 2014-2015, Facebook, Inc.
3 + * All rights reserved.
4 + *
5 + * This source code is licensed under the BSD-style license found in the
6 + * LICENSE file in the root directory of this source tree. An additional grant
7 + * of patent rights can be found in the PATENTS file in the same directory.
8 + *
9 + * @providesModule ReactDOMEmptyComponent
10 + */
11 +
12 +'use strict';
13 +
14 +var DOMLazyTree = require('DOMLazyTree');
15 +var ReactDOMComponentTree = require('ReactDOMComponentTree');
16 +
17 +var assign = require('Object.assign');
18 +
19 +var ReactDOMEmptyComponent = function(instantiate) {
20 + // ReactCompositeComponent uses this:
21 + this._currentElement = null;
22 + // ReactDOMComponentTree uses these:
23 + this._nativeNode = null;
24 + this._nativeParent = null;
25 + this._nativeContainerInfo = null;
26 + this._domID = null;
27 +};
28 +assign(ReactDOMEmptyComponent.prototype, {
29 + construct: function(element) {
30 + },
31 + mountComponent: function(
32 + transaction,
33 + nativeParent,
34 + nativeContainerInfo,
35 + context
36 + ) {
37 + var domID = nativeContainerInfo._idCounter++;
38 + this._domID = domID;
39 + this._nativeParent = nativeParent;
40 + this._nativeContainerInfo = nativeContainerInfo;
41 +
42 + var nodeValue = ' react-empty: ' + this._domID + ' ';
43 + if (transaction.useCreateElement) {
44 + var ownerDocument = nativeContainerInfo._ownerDocument;
45 + var node = ownerDocument.createComment(nodeValue);
46 + ReactDOMComponentTree.precacheNode(this, node);
47 + return DOMLazyTree(node);
48 + } else {
49 + if (transaction.renderToStaticMarkup) {
50 + // Normally we'd insert a comment node, but since this is a situation
51 + // where React won't take over (static pages), we can simply return
52 + // nothing.
53 + return '';
54 + }
55 + return '<!--' + nodeValue + '-->';
56 + }
57 + },
58 + receiveComponent: function() {
59 + },
60 + getNativeNode: function() {
61 + return ReactDOMComponentTree.getNodeFromInstance(this);
62 + },
63 + unmountComponent: function() {
64 + ReactDOMComponentTree.uncacheNode(this);
65 + },
66 +});
67 +
68 +module.exports = ReactDOMEmptyComponent;
src/renderers/dom/shared/ReactDOMTextComponent.js
+1 -1
@@ -53,8 +53,8 @@ assign(ReactDOMTextComponent.prototype, {
53 // TODO: This is really a ReactText (ReactNode), not a ReactElement
54 this._currentElement = text;
55 this._stringText = '' + text;
56 + // ReactDOMComponentTree uses these:
57 this._nativeNode = null;
57 - // ReactDOMComponentTree uses this:
58 this._nativeParent = null;
59
60 // Properties
src/renderers/dom/shared/ReactDefaultInjection.js
+6 -1
@@ -22,6 +22,7 @@ var ReactComponentBrowserEnvironment =
22 require('ReactComponentBrowserEnvironment');
23 var ReactDOMComponent = require('ReactDOMComponent');
24 var ReactDOMComponentTree = require('ReactDOMComponentTree');
25 +var ReactDOMEmptyComponent = require('ReactDOMEmptyComponent');
26 var ReactDOMTreeTraversal = require('ReactDOMTreeTraversal');
27 var ReactDOMTextComponent = require('ReactDOMTextComponent');
28 var ReactDefaultBatchingStrategy = require('ReactDefaultBatchingStrategy');
@@ -79,7 +80,11 @@ function inject() {
80 ReactInjection.DOMProperty.injectDOMPropertyConfig(HTMLDOMPropertyConfig);
81 ReactInjection.DOMProperty.injectDOMPropertyConfig(SVGDOMPropertyConfig);
82
82 - ReactInjection.EmptyComponent.injectEmptyComponent('noscript');
83 + ReactInjection.EmptyComponent.injectEmptyComponentFactory(
84 + function(instantiate) {
85 + return new ReactDOMEmptyComponent(instantiate);
86 + }
87 + );
88
89 ReactInjection.Updates.injectReconcileTransaction(
90 ReactReconcileTransaction
src/renderers/shared/reconciler/ReactEmptyComponent.js
+7 -38
@@ -11,50 +11,19 @@
11
12 'use strict';
13
14 -var ReactElement = require('ReactElement');
15 -var ReactReconciler = require('ReactReconciler');
16 -
17 -var assign = require('Object.assign');
18 -
19 -var placeholderElement;
14 +var emptyComponentFactory;
15
16 var ReactEmptyComponentInjection = {
22 - injectEmptyComponent: function(component) {
23 - placeholderElement = ReactElement.createElement(component);
17 + injectEmptyComponentFactory: function(factory) {
18 + emptyComponentFactory = factory;
19 },
20 };
21
27 -var ReactEmptyComponent = function(instantiate) {
28 - this._currentElement = null;
29 - this._renderedComponent = instantiate(placeholderElement);
30 -};
31 -assign(ReactEmptyComponent.prototype, {
32 - construct: function(element) {
33 - },
34 - mountComponent: function(
35 - transaction,
36 - nativeParent,
37 - nativeContainerInfo,
38 - context
39 - ) {
40 - return ReactReconciler.mountComponent(
41 - this._renderedComponent,
42 - transaction,
43 - nativeParent,
44 - nativeContainerInfo,
45 - context
46 - );
22 +var ReactEmptyComponent = {
23 + create: function(instantiate) {
24 + return emptyComponentFactory(instantiate);
25 },
48 - receiveComponent: function() {
49 - },
50 - getNativeNode: function() {
51 - return ReactReconciler.getNativeNode(this._renderedComponent);
52 - },
53 - unmountComponent: function() {
54 - ReactReconciler.unmountComponent(this._renderedComponent);
55 - this._renderedComponent = null;
56 - },
57 -});
26 +};
27
28 ReactEmptyComponent.injection = ReactEmptyComponentInjection;
29
src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js new
+50
@@ -0,0 +1,50 @@
1 +/**
2 + * Copyright 2014-2015, Facebook, Inc.
3 + * All rights reserved.
4 + *
5 + * This source code is licensed under the BSD-style license found in the
6 + * LICENSE file in the root directory of this source tree. An additional grant
7 + * of patent rights can be found in the PATENTS file in the same directory.
8 + *
9 + * @providesModule ReactSimpleEmptyComponent
10 + */
11 +
12 +'use strict';
13 +
14 +var ReactReconciler = require('ReactReconciler');
15 +
16 +var assign = require('Object.assign');
17 +
18 +var ReactSimpleEmptyComponent = function(placeholderElement, instantiate) {
19 + this._currentElement = null;
20 + this._renderedComponent = instantiate(placeholderElement);
21 +};
22 +assign(ReactSimpleEmptyComponent.prototype, {
23 + construct: function(element) {
24 + },
25 + mountComponent: function(
26 + transaction,
27 + nativeParent,
28 + nativeContainerInfo,
29 + context
30 + ) {
31 + return ReactReconciler.mountComponent(
32 + this._renderedComponent,
33 + transaction,
34 + nativeParent,
35 + nativeContainerInfo,
36 + context
37 + );
38 + },
39 + receiveComponent: function() {
40 + },
41 + getNativeNode: function() {
42 + return ReactReconciler.getNativeNode(this._renderedComponent);
43 + },
44 + unmountComponent: function() {
45 + ReactReconciler.unmountComponent(this._renderedComponent);
46 + this._renderedComponent = null;
47 + },
48 +});
49 +
50 +module.exports = ReactSimpleEmptyComponent;
src/renderers/shared/reconciler/__tests__/ReactEmptyComponent-test.js
+26 -2
@@ -106,6 +106,30 @@ describe('ReactEmptyComponent', function() {
106 expect(log.argsForCall[3][0]).toBe(null);
107 });
108
109 + it('should be able to switch in a list of children', () => {
110 + var instance1 =
111 + <TogglingComponent
112 + firstComponent={null}
113 + secondComponent={'div'}
114 + />;
115 +
116 + ReactTestUtils.renderIntoDocument(
117 + <div>
118 + {instance1}
119 + {instance1}
120 + {instance1}
121 + </div>
122 + );
123 +
124 + expect(log.argsForCall.length).toBe(6);
125 + expect(log.argsForCall[0][0]).toBe(null);
126 + expect(log.argsForCall[1][0]).toBe(null);
127 + expect(log.argsForCall[2][0]).toBe(null);
128 + expect(log.argsForCall[3][0].tagName).toBe('DIV');
129 + expect(log.argsForCall[4][0].tagName).toBe('DIV');
130 + expect(log.argsForCall[5][0].tagName).toBe('DIV');
131 + });
132 +
133 it('should distinguish between a script placeholder and an actual script tag',
134 () => {
135 var instance1 =
@@ -275,12 +299,12 @@ describe('ReactEmptyComponent', function() {
299
300 ReactDOM.render(<Empty />, container);
301 var noscript1 = container.firstChild;
278 - expect(noscript1.tagName).toBe('NOSCRIPT');
302 + expect(noscript1.nodeName).toBe('#comment');
303
304 // This update shouldn't create a DOM node
305 ReactDOM.render(<Empty />, container);
306 var noscript2 = container.firstChild;
283 - expect(noscript2.tagName).toBe('NOSCRIPT');
307 + expect(noscript2.nodeName).toBe('#comment');
308
309 expect(noscript1).toBe(noscript2);
310 });
src/renderers/shared/reconciler/instantiateReactComponent.js
+1 -1
@@ -67,7 +67,7 @@ function instantiateReactComponent(node) {
67 var instance;
68
69 if (node === null || node === false) {
70 - instance = new ReactEmptyComponent(instantiateReactComponent);
70 + instance = ReactEmptyComponent.create(instantiateReactComponent);
71 } else if (typeof node === 'object') {
72 var element = node;
73 invariant(