Merge pull request facebook#6164 from gaearon/fix-svg

Don't set children attribute on SVG nodes
zpao · Mar 2, 2016 · 4bbd8d2 · 4bbd8d2
2 parents 56c423a + 5eab1bb
commit 4bbd8d2
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 20 deletions.
diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -53,9 +53,13 @@ var registrationNameModules = EventPluginRegistry.registrationNameModules;
 // For quickly matching children type, to test if can be treated as content.
 var CONTENT_TYPES = {'string': true, 'number': true};
 
-var CHILDREN = keyOf({children: null});
 var STYLE = keyOf({style: null});
 var HTML = keyOf({__html: null});
+var RESERVED_PROPS = {
+ children: null,
+ dangerouslySetInnerHTML: null,
+ suppressContentEditableWarning: null,
+};
 
 function getDeclarationErrorAddendum(internalInstance) {
  if (internalInstance) {
@@ -633,11 +637,13 @@ ReactDOMComponent.Mixin = {
  }
  var markup = null;
  if (this._tag != null && isCustomComponent(this._tag, props)) {
- if (propKey !== CHILDREN) {
+ if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
  markup = DOMPropertyOperations.createMarkupForCustomAttribute(propKey, propValue);
  }
  } else if (this._namespaceURI === DOMNamespaces.svg) {
- markup = DOMPropertyOperations.createMarkupForSVGAttribute(propKey, propValue);
+ if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
+ markup = DOMPropertyOperations.createMarkupForSVGAttribute(propKey, propValue);
+ }
  } else {
  markup = DOMPropertyOperations.createMarkupForProperty(propKey, propValue);
  }
@@ -912,20 +918,21 @@ ReactDOMComponent.Mixin = {
  deleteListener(this, propKey);
  }
  } else if (isCustomComponent(this._tag, nextProps)) {
- if (propKey === CHILDREN) {
- nextProp = null;
+ if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
+ DOMPropertyOperations.setValueForAttribute(
+ getNode(this),
+ propKey,
+ nextProp
+ );
  }
- DOMPropertyOperations.setValueForAttribute(
- getNode(this),
- propKey,
- nextProp
- );
  } else if (this._namespaceURI === DOMNamespaces.svg) {
- DOMPropertyOperations.setValueForSVGAttribute(
- getNode(this),
- propKey,
- nextProp
- );
+ if (!RESERVED_PROPS.hasOwnProperty(propKey)) {
+ DOMPropertyOperations.setValueForSVGAttribute(
+ getNode(this),
+ propKey,
+ nextProp
+ );
+ }
  } else if (
  DOMProperty.properties[propKey] ||
  DOMProperty.isCustomAttribute(propKey)) {

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -244,16 +244,100 @@ describe('ReactDOMComponent', function() {
  expect(stubStyle.display).toEqual('');
  });
 
- it('should skip child object attribute on web components', function() {
+ it('should skip reserved props on web components', function() {
  var container = document.createElement('div');
 
- // Test initial render to null
- ReactDOM.render(<my-component children={['foo']} />, container);
+ ReactDOM.render(
+ <my-component
+ children={['foo']}
+ suppressContentEditableWarning={true}
+ />,
+ container
+ );
  expect(container.firstChild.hasAttribute('children')).toBe(false);
+ expect(
+ container.firstChild.hasAttribute('suppressContentEditableWarning')
+ ).toBe(false);
 
- // Test updates to null
- ReactDOM.render(<my-component children={['foo']} />, container);
+ ReactDOM.render(
+ <my-component
+ children={['bar']}
+ suppressContentEditableWarning={false}
+ />,
+ container
+ );
  expect(container.firstChild.hasAttribute('children')).toBe(false);
+ expect(
+ container.firstChild.hasAttribute('suppressContentEditableWarning')
+ ).toBe(false);
+ });
+
+ it('should skip dangerouslySetInnerHTML on web components', function() {
+ var container = document.createElement('div');
+
+ ReactDOM.render(
+ <my-component dangerouslySetInnerHTML={{__html: 'hi'}} />,
+ container
+ );
+ expect(
+ container.firstChild.hasAttribute('dangerouslySetInnerHTML')
+ ).toBe(false);
+
+ ReactDOM.render(
+ <my-component dangerouslySetInnerHTML={{__html: 'bye'}} />,
+ container
+ );
+ expect(
+ container.firstChild.hasAttribute('dangerouslySetInnerHTML')
+ ).toBe(false);
+ });
+
+ it('should skip reserved props on SVG components', function() {
+ var container = document.createElement('div');
+
+ ReactDOM.render(
+ <svg
+ children={['foo']}
+ suppressContentEditableWarning={true}
+ />,
+ container
+ );
+ expect(container.firstChild.hasAttribute('children')).toBe(false);
+ expect(
+ container.firstChild.hasAttribute('suppressContentEditableWarning')
+ ).toBe(false);
+
+ ReactDOM.render(
+ <svg
+ children={['bar']}
+ suppressContentEditableWarning={false}
+ />,
+ container
+ );
+ expect(container.firstChild.hasAttribute('children')).toBe(false);
+ expect(
+ container.firstChild.hasAttribute('suppressContentEditableWarning')
+ ).toBe(false);
+ });
+
+ it('should skip dangerouslySetInnerHTML on SVG components', function() {
+ var container = document.createElement('div');
+
+ ReactDOM.render(
+ <svg dangerouslySetInnerHTML={{__html: 'hi'}} />,
+ container
+ );
+ expect(
+ container.firstChild.hasAttribute('dangerouslySetInnerHTML')
+ ).toBe(false);
+
+ ReactDOM.render(
+ <svg dangerouslySetInnerHTML={{__html: 'bye'}} />,
+ container
+ );
+ expect(
+ container.firstChild.hasAttribute('dangerouslySetInnerHTML')
+ ).toBe(false);
  });
 
  it('should remove attributes', function() {