@samitouri / QOS-React-2 / commits / 6269ef62cf

Two more fixes for document.createElement mode

Is this all of them? I hope so. - Set text content after creating hierarchy, like the text is a child. I should've done this originally but I guessed it wouldn't matter. I was wrong (~20% perf difference in IE11). - IE throws when setting an enum-like property to an invalid value. I tried setting every property to the string 'a' and changed the ones that threw to be MUST_USE_ATTRIBUTE. I think these are all correct -- encType is the most suspicious one based on the existing comments but I tested in IE8 and it works fine as an attribute.

Ben Alpert committed Oct 23, 2015 at 22:17 UTC 6269ef62cfdbcb768f2294a26145fa3ac1085520
5 files changed +34 -27
src/renderers/dom/client/utils/DOMLazyTree.js
+14
@@ -11,6 +11,8 @@
11
12 'use strict';
13
14 +var setTextContent = require('setTextContent');
15 +
16 /**
17 * In IE (8-11) and Edge, appending nodes with no children is dramatically
18 * faster than appending a full subtree, so we essentially queue up the
@@ -43,6 +45,8 @@ function insertTreeChildren(tree) {
45 }
46 } else if (tree.html != null) {
47 node.innerHTML = tree.html;
48 + } else if (tree.text != null) {
49 + setTextContent(node, tree.text);
50 }
51 }
52
@@ -72,11 +76,20 @@ function queueHTML(tree, html) {
76 }
77 }
78
79 +function queueText(tree, text) {
80 + if (enableLazy) {
81 + tree.text = text;
82 + } else {
83 + setTextContent(tree.node, text);
84 + }
85 +}
86 +
87 function DOMLazyTree(node) {
88 return {
89 node: node,
90 children: [],
91 html: null,
92 + text: null,
93 };
94 }
95
@@ -84,5 +97,6 @@ DOMLazyTree.insertTreeBefore = insertTreeBefore;
97 DOMLazyTree.replaceChildWithTree = replaceChildWithTree;
98 DOMLazyTree.queueChild = queueChild;
99 DOMLazyTree.queueHTML = queueHTML;
100 +DOMLazyTree.queueText = queueText;
101
102 module.exports = DOMLazyTree;
src/renderers/dom/shared/HTMLDOMPropertyConfig.js
+15 -18
@@ -58,8 +58,8 @@ var HTMLDOMPropertyConfig = {
58 // autoFocus: HAS_BOOLEAN_VALUE,
59 autoPlay: HAS_BOOLEAN_VALUE,
60 capture: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,
61 - cellPadding: null,
62 - cellSpacing: null,
61 + cellPadding: MUST_USE_ATTRIBUTE,
62 + cellSpacing: MUST_USE_ATTRIBUTE,
63 charSet: MUST_USE_ATTRIBUTE,
64 challenge: MUST_USE_ATTRIBUTE,
65 checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
@@ -71,9 +71,9 @@ var HTMLDOMPropertyConfig = {
71 // regardless of whether the element is HTML or SVG.
72 className: hasSVG ? MUST_USE_ATTRIBUTE : MUST_USE_PROPERTY,
73 cols: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE,
74 - colSpan: null,
74 + colSpan: MUST_USE_ATTRIBUTE,
75 content: null,
76 - contentEditable: null,
76 + contentEditable: MUST_USE_ATTRIBUTE,
77 contextMenu: MUST_USE_ATTRIBUTE,
78 controls: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
79 coords: null,
@@ -82,11 +82,11 @@ var HTMLDOMPropertyConfig = {
82 dateTime: MUST_USE_ATTRIBUTE,
83 default: HAS_BOOLEAN_VALUE,
84 defer: HAS_BOOLEAN_VALUE,
85 - dir: null,
85 + dir: MUST_USE_ATTRIBUTE,
86 disabled: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,
87 download: HAS_OVERLOADED_BOOLEAN_VALUE,
88 draggable: null,
89 - encType: null,
89 + encType: MUST_USE_ATTRIBUTE,
90 form: MUST_USE_ATTRIBUTE,
91 formAction: MUST_USE_ATTRIBUTE,
92 formEncType: MUST_USE_ATTRIBUTE,
@@ -112,16 +112,16 @@ var HTMLDOMPropertyConfig = {
112 label: null,
113 lang: null,
114 list: MUST_USE_ATTRIBUTE,
115 - loop: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
115 + loop: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,
116 low: null,
117 manifest: MUST_USE_ATTRIBUTE,
118 - marginHeight: null,
119 - marginWidth: null,
118 + marginHeight: MUST_USE_ATTRIBUTE,
119 + marginWidth: MUST_USE_ATTRIBUTE,
120 max: null,
121 maxLength: MUST_USE_ATTRIBUTE,
122 media: MUST_USE_ATTRIBUTE,
123 mediaGroup: null,
124 - method: null,
124 + method: MUST_USE_ATTRIBUTE,
125 min: null,
126 minLength: MUST_USE_ATTRIBUTE,
127 multiple: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
@@ -140,23 +140,23 @@ var HTMLDOMPropertyConfig = {
140 required: HAS_BOOLEAN_VALUE,
141 role: MUST_USE_ATTRIBUTE,
142 rows: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE,
143 - rowSpan: null,
143 + rowSpan: MUST_USE_ATTRIBUTE | HAS_NUMERIC_VALUE,
144 sandbox: null,
145 scope: null,
146 scoped: HAS_BOOLEAN_VALUE,
147 - scrolling: null,
147 + scrolling: MUST_USE_ATTRIBUTE,
148 seamless: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,
149 selected: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
150 shape: null,
151 size: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE,
152 sizes: MUST_USE_ATTRIBUTE,
153 - span: HAS_POSITIVE_NUMERIC_VALUE,
153 + span: MUST_USE_ATTRIBUTE | HAS_POSITIVE_NUMERIC_VALUE,
154 spellCheck: null,
155 src: null,
156 srcDoc: MUST_USE_PROPERTY,
157 srcLang: null,
158 srcSet: MUST_USE_ATTRIBUTE,
159 - start: HAS_NUMERIC_VALUE,
159 + start: MUST_USE_ATTRIBUTE | HAS_NUMERIC_VALUE,
160 step: null,
161 style: null,
162 summary: null,
@@ -169,7 +169,7 @@ var HTMLDOMPropertyConfig = {
169 value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS,
170 width: MUST_USE_ATTRIBUTE,
171 wmode: MUST_USE_ATTRIBUTE,
172 - wrap: null,
172 + wrap: MUST_USE_ATTRIBUTE,
173
174 /**
175 * RDFa Properties
@@ -225,9 +225,6 @@ var HTMLDOMPropertyConfig = {
225 autoFocus: 'autofocus',
226 autoPlay: 'autoplay',
227 autoSave: 'autosave',
228 - // `encoding` is equivalent to `enctype`, IE8 lacks an `enctype` setter.
229 - // http://www.w3.org/TR/html5/forms.html#dom-fs-encoding
230 - encType: 'encoding',
228 hrefLang: 'hreflang',
229 radioGroup: 'radiogroup',
230 spellCheck: 'spellcheck',
src/renderers/dom/shared/ReactDOMComponent.js
+1 -2
@@ -42,7 +42,6 @@ var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
42 var invariant = require('invariant');
43 var isEventSupported = require('isEventSupported');
44 var keyOf = require('keyOf');
45 -var setTextContent = require('setTextContent');
45 var shallowEqual = require('shallowEqual');
46 var validateDOMNesting = require('validateDOMNesting');
47 var warning = require('warning');
@@ -828,7 +827,7 @@ ReactDOMComponent.Mixin = {
827 var childrenToUse = contentToUse != null ? null : props.children;
828 if (contentToUse != null) {
829 // TODO: Validate that text is allowed as a child of this node
831 - setTextContent(lazyTree.node, contentToUse);
830 + DOMLazyTree.queueText(lazyTree, contentToUse);
831 } else if (childrenToUse != null) {
832 var mountImages = this.mountChildren(
833 childrenToUse,
src/renderers/dom/shared/ReactDOMTextComponent.js
+3 -3
@@ -21,7 +21,6 @@ var ReactMount = require('ReactMount');
21
22 var assign = require('Object.assign');
23 var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
24 -var setTextContent = require('setTextContent');
24 var validateDOMNesting = require('validateDOMNesting');
25
26 function getNode(inst) {
@@ -106,8 +105,9 @@ assign(ReactDOMTextComponent.prototype, {
105 DOMPropertyOperations.setAttributeForID(el, rootID);
106 // Populate node cache
107 ReactMount.getID(el);
109 - setTextContent(el, this._stringText);
110 - return DOMLazyTree(el);
108 + var lazyTree = DOMLazyTree(el);
109 + DOMLazyTree.queueText(lazyTree, this._stringText);
110 + return lazyTree;
111 } else {
112 var escapedText = escapeTextContentForBrowser(this._stringText);
113
src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
+1 -4
@@ -432,10 +432,7 @@ describe('ReactDOMComponent', function() {
432 var node = ReactDOM.render(<div />, container);
433
434 var setter = mocks.getMockFunction();
435 - Object.defineProperty(node, 'dir', {
436 - get: function() {},
437 - set: setter,
438 - });
435 + node.setAttribute = setter;
436
437 ReactDOM.render(<div dir={null} />, container);
438 ReactDOM.render(<div dir={undefined} />, container);