Refactor DOM attribute code (take two)

packages/react-dom/src/__tests__/ReactDOMComponent-test.js

@@ -442,6 +442,97 @@ describe('ReactDOMComponent', () => {
       expect(container.firstChild.className).toEqual('');
     });
 
+    it('should not set null/undefined attributes', () => {
+      var container = document.createElement('div');
+      // Initial render.
+      ReactDOM.render(<img src={null} data-foo={undefined} />, container);
+      var node = container.firstChild;
+      expect(node.hasAttribute('src')).toBe(false);
+      expect(node.hasAttribute('data-foo')).toBe(false);
+      // Update in one direction.
+      ReactDOM.render(<img src={undefined} data-foo={null} />, container);
+      expect(node.hasAttribute('src')).toBe(false);
+      expect(node.hasAttribute('data-foo')).toBe(false);
+      // Update in another direction.
+      ReactDOM.render(<img src={null} data-foo={undefined} />, container);
+      expect(node.hasAttribute('src')).toBe(false);
+      expect(node.hasAttribute('data-foo')).toBe(false);
+      // Removal.
+      ReactDOM.render(<img />, container);
+      expect(node.hasAttribute('src')).toBe(false);
+      expect(node.hasAttribute('data-foo')).toBe(false);
+      // Addition.
+      ReactDOM.render(<img src={undefined} data-foo={null} />, container);
+      expect(node.hasAttribute('src')).toBe(false);
+      expect(node.hasAttribute('data-foo')).toBe(false);
+    });
+
+    it('should apply React-specific aliases to HTML elements', () => {
+      var container = document.createElement('div');
+      ReactDOM.render(<form acceptCharset="foo" />, container);
+      var node = container.firstChild;
+      // Test attribute initialization.
+      expect(node.getAttribute('accept-charset')).toBe('foo');
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Test attribute update.
+      ReactDOM.render(<form acceptCharset="boo" />, container);
+      expect(node.getAttribute('accept-charset')).toBe('boo');
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Test attribute removal by setting to null.
+      ReactDOM.render(<form acceptCharset={null} />, container);
+      expect(node.hasAttribute('accept-charset')).toBe(false);
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Restore.
+      ReactDOM.render(<form acceptCharset="foo" />, container);
+      expect(node.getAttribute('accept-charset')).toBe('foo');
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Test attribute removal by setting to undefined.
+      ReactDOM.render(<form acceptCharset={undefined} />, container);
+      expect(node.hasAttribute('accept-charset')).toBe(false);
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Restore.
+      ReactDOM.render(<form acceptCharset="foo" />, container);
+      expect(node.getAttribute('accept-charset')).toBe('foo');
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+      // Test attribute removal.
+      ReactDOM.render(<form />, container);
+      expect(node.hasAttribute('accept-charset')).toBe(false);
+      expect(node.hasAttribute('acceptCharset')).toBe(false);
+    });
+
+    it('should apply React-specific aliases to SVG elements', () => {
+      var container = document.createElement('div');
+      ReactDOM.render(<svg arabicForm="foo" />, container);
+      var node = container.firstChild;
+      // Test attribute initialization.
+      expect(node.getAttribute('arabic-form')).toBe('foo');
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Test attribute update.
+      ReactDOM.render(<svg arabicForm="boo" />, container);
+      expect(node.getAttribute('arabic-form')).toBe('boo');
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Test attribute removal by setting to null.
+      ReactDOM.render(<svg arabicForm={null} />, container);
+      expect(node.hasAttribute('arabic-form')).toBe(false);
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Restore.
+      ReactDOM.render(<svg arabicForm="foo" />, container);
+      expect(node.getAttribute('arabic-form')).toBe('foo');
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Test attribute removal by setting to undefined.
+      ReactDOM.render(<svg arabicForm={undefined} />, container);
+      expect(node.hasAttribute('arabic-form')).toBe(false);
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Restore.
+      ReactDOM.render(<svg arabicForm="foo" />, container);
+      expect(node.getAttribute('arabic-form')).toBe('foo');
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Test attribute removal.
+      ReactDOM.render(<svg />, container);
+      expect(node.hasAttribute('arabic-form')).toBe(false);
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+    });
+
     it('should properly update custom attributes on custom elements', () => {
       const container = document.createElement('div');
       ReactDOM.render(<some-custom-element foo="bar" />, container);
@@ -451,6 +542,25 @@ describe('ReactDOMComponent', () => {
       expect(node.getAttribute('bar')).toBe('buzz');
     });
 
+    it('should not apply React-specific aliases to custom elements', () => {
+      var container = document.createElement('div');
+      ReactDOM.render(<some-custom-element arabicForm="foo" />, container);
+      var node = container.firstChild;
+      // Should not get transformed to arabic-form as SVG would be.
+      expect(node.getAttribute('arabicForm')).toBe('foo');
+      expect(node.hasAttribute('arabic-form')).toBe(false);
+      // Test attribute update.
+      ReactDOM.render(<some-custom-element arabicForm="boo" />, container);
+      expect(node.getAttribute('arabicForm')).toBe('boo');
+      // Test attribute removal and addition.
+      ReactDOM.render(<some-custom-element acceptCharset="buzz" />, container);
+      // Verify the previous attribute was removed.
+      expect(node.hasAttribute('arabicForm')).toBe(false);
+      // Should not get transformed to accept-charset as HTML would be.
+      expect(node.getAttribute('acceptCharset')).toBe('buzz');
+      expect(node.hasAttribute('accept-charset')).toBe(false);
+    });
+
     it('should clear a single style prop when changing `style`', () => {
       let styles = {display: 'none', color: 'red'};
       const container = document.createElement('div');

packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js

@@ -62,6 +62,16 @@ describe('ReactDOMServerIntegration', () => {
         const e = await render(<div width={null} />);
         expect(e.hasAttribute('width')).toBe(false);
       });
+
+      itRenders('no string prop with function value', async render => {
+        const e = await render(<div width={function() {}} />, 1);
+        expect(e.hasAttribute('width')).toBe(false);
+      });
+
+      itRenders('no string prop with symbol value', async render => {
+        const e = await render(<div width={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('width')).toBe(false);
+      });
     });
 
     describe('boolean properties', function() {
@@ -122,6 +132,16 @@ describe('ReactDOMServerIntegration', () => {
         const e = await render(<div hidden={null} />);
         expect(e.hasAttribute('hidden')).toBe(false);
       });
+
+      itRenders('no boolean prop with function value', async render => {
+        const e = await render(<div hidden={function() {}} />, 1);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
+
+      itRenders('no boolean prop with symbol value', async render => {
+        const e = await render(<div hidden={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
     });
 
     describe('download property (combined boolean/string attribute)', function() {
@@ -164,6 +184,16 @@ describe('ReactDOMServerIntegration', () => {
         const e = await render(<div download={undefined} />);
         expect(e.hasAttribute('download')).toBe(false);
       });
+
+      itRenders('no download prop with function value', async render => {
+        const e = await render(<div download={function() {}} />, 1);
+        expect(e.hasAttribute('download')).toBe(false);
+      });
+
+      itRenders('no download prop with symbol value', async render => {
+        const e = await render(<div download={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('download')).toBe(false);
+      });
     });
 
     describe('className property', function() {
@@ -257,6 +287,11 @@ describe('ReactDOMServerIntegration', () => {
         },
       );
 
+      itRenders('numeric property with zero value', async render => {
+        const e = await render(<ol start={0} />);
+        expect(e.getAttribute('start')).toBe('0');
+      });
+
       itRenders(
         'no positive numeric property with zero value',
         async render => {
@@ -265,9 +300,27 @@ describe('ReactDOMServerIntegration', () => {
         },
       );
 
-      itRenders('numeric property with zero value', async render => {
-        const e = await render(<ol start={0} />);
-        expect(e.getAttribute('start')).toBe('0');
+      itRenders('no numeric prop with function value', async render => {
+        const e = await render(<ol start={function() {}} />, 1);
+        expect(e.hasAttribute('start')).toBe(false);
+      });
+
+      itRenders('no numeric prop with symbol value', async render => {
+        const e = await render(<ol start={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('start')).toBe(false);
+      });
+
+      itRenders(
+        'no positive numeric prop with function value',
+        async render => {
+          const e = await render(<input size={function() {}} />, 1);
+          expect(e.hasAttribute('size')).toBe(false);
+        },
+      );
+
+      itRenders('no positive numeric prop with symbol value', async render => {
+        const e = await render(<input size={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('size')).toBe(false);
       });
     });