Fix reconciling when switching to/from innerHTML
There's no way that this can work if _updateDOMChildren doesn't know about dangerouslySetInnerHTML, so tell it. Fixes #377.
Ben Alpert committed
Sep 26, 2013 at 23:52 UTC
3ca507d73ffc07087ed4e95e010a101298f78146
4 files changed
+64
-39
src/core/ReactDOMIDOperations.js
+5
-3
@@ -49,6 +49,8 @@ var INVALID_PROPERTY_ERRORS = {
49
*/
50
var textContentAccessor = getTextContentAccessor() || 'NA';
51
52
+var LEADING_SPACE = /^ /;
53
+
54
/**
55
* Operations used to process updates to DOM nodes. This is made injectable via
56
* `ReactComponent.DOMIDOperations`.
@@ -133,17 +135,17 @@ var ReactDOMIDOperations = {
135
},
136
137
/**
136
- * Updates a DOM node's innerHTML set by `props.dangerouslySetInnerHTML`.
138
+ * Updates a DOM node's innerHTML.
139
*
140
* @param {string} id ID of the node to update.
139
- * @param {object} html An HTML object with the `__html` property.
141
+ * @param {string} html An HTML string.
142
* @internal
143
*/
144
updateInnerHTMLByID: function(id, html) {
145
var node = ReactMount.getNode(id);
146
// HACK: IE8- normalize whitespace in innerHTML, removing leading spaces.
147
// @see quirksmode.org/bugreports/archives/2004/11/innerhtml_and_t.html
146
- node.innerHTML = (html && html.__html || '').replace(/^ /g, ' ');
148
+ node.innerHTML = html.replace(LEADING_SPACE, ' ');
149
},
150
151
/**
src/core/ReactNativeComponent.js
+34
-34
@@ -42,7 +42,6 @@ var registrationNames = ReactEventEmitter.registrationNames;
42
// For quickly matching children type, to test if can be treated as content.
43
var CONTENT_TYPES = {'string': true, 'number': true};
44
45
-var DANGEROUSLY_SET_INNER_HTML = keyOf({dangerouslySetInnerHTML: null});
45
var STYLE = keyOf({style: null});
46
47
/**
@@ -231,15 +230,11 @@ ReactNativeComponent.Mixin = {
230
styleUpdates[styleName] = '';
231
}
232
}
234
- } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
235
- // http://jsperf.com/emptying-speed
236
- ReactComponent.DOMIDOperations.updateTextContentByID(
237
- this._rootNodeID,
238
- ''
239
- );
233
} else if (registrationNames[propKey]) {
234
deleteListener(this._rootNodeID, propKey);
242
- } else {
235
+ } else if (
236
+ DOMProperty.isStandardName[propKey] ||
237
+ DOMProperty.isCustomAttribute(propKey)) {
238
ReactComponent.DOMIDOperations.deletePropertyByID(
239
this._rootNodeID,
240
propKey
@@ -277,15 +272,6 @@ ReactNativeComponent.Mixin = {
272
// Relies on `updateStylesByID` not mutating `styleUpdates`.
273
styleUpdates = nextProp;
274
}
280
- } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
281
- var lastHtml = lastProp && lastProp.__html;
282
- var nextHtml = nextProp && nextProp.__html;
283
- if (lastHtml !== nextHtml) {
284
- ReactComponent.DOMIDOperations.updateInnerHTMLByID(
285
- this._rootNodeID,
286
- nextProp
287
- );
288
- }
275
} else if (registrationNames[propKey]) {
276
putListener(this._rootNodeID, propKey, nextProp);
277
} else if (
@@ -316,31 +302,45 @@ ReactNativeComponent.Mixin = {
302
_updateDOMChildren: function(lastProps, transaction) {
303
var nextProps = this.props;
304
319
- var lastUsedContent =
305
+ var lastContent =
306
CONTENT_TYPES[typeof lastProps.children] ? lastProps.children : null;
321
- var contentToUse =
307
+ var nextContent =
308
CONTENT_TYPES[typeof nextProps.children] ? nextProps.children : null;
309
310
+ var lastHtml =
311
+ lastProps.dangerouslySetInnerHTML &&
312
+ lastProps.dangerouslySetInnerHTML.__html;
313
+ var nextHtml =
314
+ nextProps.dangerouslySetInnerHTML &&
315
+ nextProps.dangerouslySetInnerHTML.__html;
316
+
317
// Note the use of `!=` which checks for null or undefined.
318
+ var lastChildren = lastContent != null ? null : lastProps.children;
319
+ var nextChildren = nextContent != null ? null : nextProps.children;
320
326
- var lastUsedChildren =
327
- lastUsedContent != null ? null : lastProps.children;
328
- var childrenToUse = contentToUse != null ? null : nextProps.children;
321
+ // If we're switching from children to content/html or vice versa, remove
322
+ // the old content
323
+ var lastHasContentOrHtml = lastContent != null || lastHtml != null;
324
+ var nextHasContentOrHtml = nextContent != null || nextHtml != null;
325
+ if (lastChildren != null && nextChildren == null) {
326
+ this.updateChildren(null, transaction);
327
+ } else if (lastHasContentOrHtml && !nextHasContentOrHtml) {
328
+ this.updateTextContent('');
329
+ }
330
330
- if (contentToUse != null) {
331
- var childrenRemoved = lastUsedChildren != null && childrenToUse == null;
332
- if (childrenRemoved) {
333
- this.updateChildren(null, transaction);
334
- }
335
- if (lastUsedContent !== contentToUse) {
336
- this.updateTextContent('' + contentToUse);
331
+ if (nextContent != null) {
332
+ if (lastContent !== nextContent) {
333
+ this.updateTextContent('' + nextContent);
334
}
338
- } else {
339
- var contentRemoved = lastUsedContent != null && contentToUse == null;
340
- if (contentRemoved) {
341
- this.updateTextContent('');
335
+ } else if (nextHtml != null) {
336
+ if (lastHtml !== nextHtml) {
337
+ ReactComponent.DOMIDOperations.updateInnerHTMLByID(
338
+ this._rootNodeID,
339
+ nextHtml
340
+ );
341
}
343
- this.updateChildren(flattenChildren(nextProps.children), transaction);
342
+ } else if (nextChildren != null) {
343
+ this.updateChildren(flattenChildren(nextChildren), transaction);
344
}
345
},
346
src/core/__tests__/ReactDOMIDOperations-test.js
+1
-1
@@ -53,7 +53,7 @@ describe('ReactDOMIDOperations', function() {
53
54
ReactDOMIDOperations.updateInnerHTMLByID(
55
'testID',
56
- {__html: ' testContent'}
56
+ ' testContent'
57
);
58
59
expect(
src/core/__tests__/ReactNativeComponent-test.js
+24
-1
@@ -152,7 +152,7 @@ describe('ReactNativeComponent', function() {
152
153
it("should empty element when removing innerHTML", function() {
154
var stub = ReactTestUtils.renderIntoDocument(
155
- <div dangerouslySetInnerHTML={{__html: ":)"}} />
155
+ <div dangerouslySetInnerHTML={{__html: ':)'}} />
156
);
157
158
expect(stub.getDOMNode().innerHTML).toEqual(':)');
@@ -160,6 +160,29 @@ describe('ReactNativeComponent', function() {
160
expect(stub.getDOMNode().innerHTML).toEqual('');
161
});
162
163
+ it("should transition from string content to innerHTML", function() {
164
+ var stub = ReactTestUtils.renderIntoDocument(
165
+ <div>hello</div>
166
+ );
167
+
168
+ expect(stub.getDOMNode().innerHTML).toEqual('hello');
169
+ stub.receiveProps(
170
+ {dangerouslySetInnerHTML: {__html: 'goodbye'}},
171
+ transaction
172
+ );
173
+ expect(stub.getDOMNode().innerHTML).toEqual('goodbye');
174
+ });
175
+
176
+ it("should transition from innerHTML to string content", function() {
177
+ var stub = ReactTestUtils.renderIntoDocument(
178
+ <div dangerouslySetInnerHTML={{__html: 'bonjour'}} />
179
+ );
180
+
181
+ expect(stub.getDOMNode().innerHTML).toEqual('bonjour');
182
+ stub.receiveProps({children: 'adieu'}, transaction);
183
+ expect(stub.getDOMNode().innerHTML).toEqual('adieu');
184
+ });
185
+
186
it("should not incur unnecessary DOM mutations", function() {
187
var stub = ReactTestUtils.renderIntoDocument(<div value="" />);
188