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

Don't set children attribute on SVG nodes

Author: gaearon

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)) {

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() {