Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/core/ReactDOMIDOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ var INVALID_PROPERTY_ERRORS = {
*/
var textContentAccessor = getTextContentAccessor() || 'NA';

var LEADING_SPACE = /^ /;

/**
* Operations used to process updates to DOM nodes. This is made injectable via
* `ReactComponent.DOMIDOperations`.
Expand Down Expand Up @@ -133,17 +135,17 @@ var ReactDOMIDOperations = {
},

/**
* Updates a DOM node's innerHTML set by `props.dangerouslySetInnerHTML`.
* Updates a DOM node's innerHTML.
*
* @param {string} id ID of the node to update.
* @param {object} html An HTML object with the `__html` property.
* @param {string} html An HTML string.
* @internal
*/
updateInnerHTMLByID: function(id, html) {
var node = ReactMount.getNode(id);
// HACK: IE8- normalize whitespace in innerHTML, removing leading spaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for this method is no longer correct, can you fix it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// @see quirksmode.org/bugreports/archives/2004/11/innerhtml_and_t.html
node.innerHTML = (html && html.__html || '').replace(/^ /g, ' ');
node.innerHTML = html.replace(LEADING_SPACE, ' ');
},

/**
Expand Down
68 changes: 34 additions & 34 deletions src/core/ReactNativeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ var registrationNames = ReactEventEmitter.registrationNames;
// For quickly matching children type, to test if can be treated as content.
var CONTENT_TYPES = {'string': true, 'number': true};

var DANGEROUSLY_SET_INNER_HTML = keyOf({dangerouslySetInnerHTML: null});
var STYLE = keyOf({style: null});

/**
Expand Down Expand Up @@ -231,15 +230,11 @@ ReactNativeComponent.Mixin = {
styleUpdates[styleName] = '';
}
}
} else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
// http://jsperf.com/emptying-speed
ReactComponent.DOMIDOperations.updateTextContentByID(
this._rootNodeID,
''
);
} else if (registrationNames[propKey]) {
deleteListener(this._rootNodeID, propKey);
} else {
} else if (
DOMProperty.isStandardName[propKey] ||
DOMProperty.isCustomAttribute(propKey)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an extra regex eval in a hot code path. I wonder if that's a real perf issue and if memoization would help.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Updating already checked isCustomAttribute, I just made deleting do it too.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems fine.

ReactComponent.DOMIDOperations.deletePropertyByID(
this._rootNodeID,
propKey
Expand Down Expand Up @@ -277,15 +272,6 @@ ReactNativeComponent.Mixin = {
// Relies on `updateStylesByID` not mutating `styleUpdates`.
styleUpdates = nextProp;
}
} else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
var lastHtml = lastProp && lastProp.__html;
var nextHtml = nextProp && nextProp.__html;
if (lastHtml !== nextHtml) {
ReactComponent.DOMIDOperations.updateInnerHTMLByID(
this._rootNodeID,
nextProp
);
}
} else if (registrationNames[propKey]) {
putListener(this._rootNodeID, propKey, nextProp);
} else if (
Expand Down Expand Up @@ -316,31 +302,45 @@ ReactNativeComponent.Mixin = {
_updateDOMChildren: function(lastProps, transaction) {
var nextProps = this.props;

var lastUsedContent =
var lastContent =
CONTENT_TYPES[typeof lastProps.children] ? lastProps.children : null;
var contentToUse =
var nextContent =
CONTENT_TYPES[typeof nextProps.children] ? nextProps.children : null;

var lastHtml =
lastProps.dangerouslySetInnerHTML &&
lastProps.dangerouslySetInnerHTML.__html;
var nextHtml =
nextProps.dangerouslySetInnerHTML &&
nextProps.dangerouslySetInnerHTML.__html;

// Note the use of `!=` which checks for null or undefined.
var lastChildren = lastContent != null ? null : lastProps.children;
var nextChildren = nextContent != null ? null : nextProps.children;

var lastUsedChildren =
lastUsedContent != null ? null : lastProps.children;
var childrenToUse = contentToUse != null ? null : nextProps.children;
// If we're switching from children to content/html or vice versa, remove
// the old content
var lastHasContentOrHtml = lastContent != null || lastHtml != null;
var nextHasContentOrHtml = nextContent != null || nextHtml != null;
if (lastChildren != null && nextChildren == null) {
this.updateChildren(null, transaction);
} else if (lastHasContentOrHtml && !nextHasContentOrHtml) {
this.updateTextContent('');
}

if (contentToUse != null) {
var childrenRemoved = lastUsedChildren != null && childrenToUse == null;
if (childrenRemoved) {
this.updateChildren(null, transaction);
}
if (lastUsedContent !== contentToUse) {
this.updateTextContent('' + contentToUse);
if (nextContent != null) {
if (lastContent !== nextContent) {
this.updateTextContent('' + nextContent);
}
} else {
var contentRemoved = lastUsedContent != null && contentToUse == null;
if (contentRemoved) {
this.updateTextContent('');
} else if (nextHtml != null) {
if (lastHtml !== nextHtml) {
ReactComponent.DOMIDOperations.updateInnerHTMLByID(
this._rootNodeID,
nextHtml
);
}
this.updateChildren(flattenChildren(nextProps.children), transaction);
} else if (nextChildren != null) {
this.updateChildren(flattenChildren(nextChildren), transaction);
}
},

Expand Down
2 changes: 1 addition & 1 deletion src/core/__tests__/ReactDOMIDOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('ReactDOMIDOperations', function() {

ReactDOMIDOperations.updateInnerHTMLByID(
'testID',
{__html: ' testContent'}
' testContent'
);

expect(
Expand Down
25 changes: 24 additions & 1 deletion src/core/__tests__/ReactNativeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,37 @@ describe('ReactNativeComponent', function() {

it("should empty element when removing innerHTML", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div dangerouslySetInnerHTML={{__html: ":)"}} />
<div dangerouslySetInnerHTML={{__html: ':)'}} />
);

expect(stub.getDOMNode().innerHTML).toEqual(':)');
stub.receiveProps({}, transaction);
expect(stub.getDOMNode().innerHTML).toEqual('');
});

it("should transition from string content to innerHTML", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div>hello</div>
);

expect(stub.getDOMNode().innerHTML).toEqual('hello');
stub.receiveProps(
{dangerouslySetInnerHTML: {__html: 'goodbye'}},
transaction
);
expect(stub.getDOMNode().innerHTML).toEqual('goodbye');
});

it("should transition from innerHTML to string content", function() {
var stub = ReactTestUtils.renderIntoDocument(
<div dangerouslySetInnerHTML={{__html: 'bonjour'}} />
);

expect(stub.getDOMNode().innerHTML).toEqual('bonjour');
stub.receiveProps({children: 'adieu'}, transaction);
expect(stub.getDOMNode().innerHTML).toEqual('adieu');
});

it("should not incur unnecessary DOM mutations", function() {
var stub = ReactTestUtils.renderIntoDocument(<div value="" />);

Expand Down