Warn against custom non-lowercase attributes (#10699)

* Warn against custom non-lowercase attributes * Update attribute table * Grammar tweaks
facebook · Sep 13, 2017 · c99bad9 · c99bad9
1 parent c55ffb3
commit c99bad9
Show file tree

Hide file tree

Showing 5 changed files with 148 additions and 81 deletions.
diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md
@@ -701,23 +701,23 @@
 ## `as` (on `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
-| `as=(string)`| (changed)| `"a string"` |
+| `as=(string)`| (initial)| `<empty string>` |
 | `as=(empty string)`| (initial)| `<empty string>` |
-| `as=(array with string)`| (changed)| `"string"` |
+| `as=(array with string)`| (initial)| `<empty string>` |
 | `as=(empty array)`| (initial)| `<empty string>` |
-| `as=(object)`| (changed)| `"result of toString()"` |
-| `as=(numeric string)`| (changed)| `"42"` |
-| `as=(-1)`| (changed)| `"-1"` |
-| `as=(0)`| (changed)| `"0"` |
-| `as=(integer)`| (changed)| `"1"` |
-| `as=(NaN)`| (changed, warning)| `"NaN"` |
-| `as=(float)`| (changed)| `"99.99"` |
+| `as=(object)`| (initial)| `<empty string>` |
+| `as=(numeric string)`| (initial)| `<empty string>` |
+| `as=(-1)`| (initial)| `<empty string>` |
+| `as=(0)`| (initial)| `<empty string>` |
+| `as=(integer)`| (initial)| `<empty string>` |
+| `as=(NaN)`| (initial, warning)| `<empty string>` |
+| `as=(float)`| (initial)| `<empty string>` |
 | `as=(true)`| (initial, warning)| `<empty string>` |
 | `as=(false)`| (initial, warning)| `<empty string>` |
-| `as=(string 'true')`| (changed)| `"true"` |
-| `as=(string 'false')`| (changed)| `"false"` |
-| `as=(string 'on')`| (changed)| `"on"` |
-| `as=(string 'off')`| (changed)| `"off"` |
+| `as=(string 'true')`| (initial)| `<empty string>` |
+| `as=(string 'false')`| (initial)| `<empty string>` |
+| `as=(string 'on')`| (initial)| `<empty string>` |
+| `as=(string 'off')`| (initial)| `<empty string>` |
 | `as=(symbol)`| (initial, warning)| `<empty string>` |
 | `as=(function)`| (initial, warning)| `<empty string>` |
 | `as=(null)`| (initial)| `<empty string>` |
@@ -5251,52 +5251,52 @@
 ## `initialChecked` (on `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
-| `initialChecked=(string)`| (changed)| `"a string"` |
-| `initialChecked=(empty string)`| (changed)| `<empty string>` |
-| `initialChecked=(array with string)`| (changed)| `"string"` |
-| `initialChecked=(empty array)`| (changed)| `<empty string>` |
-| `initialChecked=(object)`| (changed)| `"result of toString()"` |
-| `initialChecked=(numeric string)`| (changed)| `"42"` |
-| `initialChecked=(-1)`| (changed)| `"-1"` |
-| `initialChecked=(0)`| (changed)| `"0"` |
-| `initialChecked=(integer)`| (changed)| `"1"` |
+| `initialChecked=(string)`| (changed, warning)| `"a string"` |
+| `initialChecked=(empty string)`| (changed, warning)| `<empty string>` |
+| `initialChecked=(array with string)`| (changed, warning)| `"string"` |
+| `initialChecked=(empty array)`| (changed, warning)| `<empty string>` |
+| `initialChecked=(object)`| (changed, warning)| `"result of toString()"` |
+| `initialChecked=(numeric string)`| (changed, warning)| `"42"` |
+| `initialChecked=(-1)`| (changed, warning)| `"-1"` |
+| `initialChecked=(0)`| (changed, warning)| `"0"` |
+| `initialChecked=(integer)`| (changed, warning)| `"1"` |
 | `initialChecked=(NaN)`| (changed, warning)| `"NaN"` |
-| `initialChecked=(float)`| (changed)| `"99.99"` |
+| `initialChecked=(float)`| (changed, warning)| `"99.99"` |
 | `initialChecked=(true)`| (initial, warning)| `<null>` |
 | `initialChecked=(false)`| (initial, warning)| `<null>` |
-| `initialChecked=(string 'true')`| (changed)| `"true"` |
-| `initialChecked=(string 'false')`| (changed)| `"false"` |
-| `initialChecked=(string 'on')`| (changed)| `"on"` |
-| `initialChecked=(string 'off')`| (changed)| `"off"` |
+| `initialChecked=(string 'true')`| (changed, warning)| `"true"` |
+| `initialChecked=(string 'false')`| (changed, warning)| `"false"` |
+| `initialChecked=(string 'on')`| (changed, warning)| `"on"` |
+| `initialChecked=(string 'off')`| (changed, warning)| `"off"` |
 | `initialChecked=(symbol)`| (initial, warning)| `<null>` |
 | `initialChecked=(function)`| (initial, warning)| `<null>` |
-| `initialChecked=(null)`| (initial)| `<null>` |
-| `initialChecked=(undefined)`| (initial)| `<null>` |
+| `initialChecked=(null)`| (initial, warning)| `<null>` |
+| `initialChecked=(undefined)`| (initial, warning)| `<null>` |
 
 ## `initialValue` (on `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
-| `initialValue=(string)`| (changed)| `"a string"` |
-| `initialValue=(empty string)`| (changed)| `<empty string>` |
-| `initialValue=(array with string)`| (changed)| `"string"` |
-| `initialValue=(empty array)`| (changed)| `<empty string>` |
-| `initialValue=(object)`| (changed)| `"result of toString()"` |
-| `initialValue=(numeric string)`| (changed)| `"42"` |
-| `initialValue=(-1)`| (changed)| `"-1"` |
-| `initialValue=(0)`| (changed)| `"0"` |
-| `initialValue=(integer)`| (changed)| `"1"` |
+| `initialValue=(string)`| (changed, warning)| `"a string"` |
+| `initialValue=(empty string)`| (changed, warning)| `<empty string>` |
+| `initialValue=(array with string)`| (changed, warning)| `"string"` |
+| `initialValue=(empty array)`| (changed, warning)| `<empty string>` |
+| `initialValue=(object)`| (changed, warning)| `"result of toString()"` |
+| `initialValue=(numeric string)`| (changed, warning)| `"42"` |
+| `initialValue=(-1)`| (changed, warning)| `"-1"` |
+| `initialValue=(0)`| (changed, warning)| `"0"` |
+| `initialValue=(integer)`| (changed, warning)| `"1"` |
 | `initialValue=(NaN)`| (changed, warning)| `"NaN"` |
-| `initialValue=(float)`| (changed)| `"99.99"` |
+| `initialValue=(float)`| (changed, warning)| `"99.99"` |
 | `initialValue=(true)`| (initial, warning)| `<null>` |
 | `initialValue=(false)`| (initial, warning)| `<null>` |
-| `initialValue=(string 'true')`| (changed)| `"true"` |
-| `initialValue=(string 'false')`| (changed)| `"false"` |
-| `initialValue=(string 'on')`| (changed)| `"on"` |
-| `initialValue=(string 'off')`| (changed)| `"off"` |
+| `initialValue=(string 'true')`| (changed, warning)| `"true"` |
+| `initialValue=(string 'false')`| (changed, warning)| `"false"` |
+| `initialValue=(string 'on')`| (changed, warning)| `"on"` |
+| `initialValue=(string 'off')`| (changed, warning)| `"off"` |
 | `initialValue=(symbol)`| (initial, warning)| `<null>` |
 | `initialValue=(function)`| (initial, warning)| `<null>` |
-| `initialValue=(null)`| (initial)| `<null>` |
-| `initialValue=(undefined)`| (initial)| `<null>` |
+| `initialValue=(null)`| (initial, warning)| `<null>` |
+| `initialValue=(undefined)`| (initial, warning)| `<null>` |
 
 ## `inlist` (on `<div>`)
 | Test Case | Flags | Result |
@@ -9276,27 +9276,27 @@
 ## `selectedIndex` (on `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
-| `selectedIndex=(string)`| (initial)| `<number: -1>` |
-| `selectedIndex=(empty string)`| (initial)| `<number: -1>` |
-| `selectedIndex=(array with string)`| (initial)| `<number: -1>` |
-| `selectedIndex=(empty array)`| (initial)| `<number: -1>` |
-| `selectedIndex=(object)`| (initial)| `<number: -1>` |
-| `selectedIndex=(numeric string)`| (initial)| `<number: -1>` |
-| `selectedIndex=(-1)`| (initial)| `<number: -1>` |
-| `selectedIndex=(0)`| (initial)| `<number: -1>` |
-| `selectedIndex=(integer)`| (initial)| `<number: -1>` |
+| `selectedIndex=(string)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(empty string)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(array with string)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(empty array)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(object)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(numeric string)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(-1)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(0)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(integer)`| (initial, warning)| `<number: -1>` |
 | `selectedIndex=(NaN)`| (initial, warning)| `<number: -1>` |
-| `selectedIndex=(float)`| (initial)| `<number: -1>` |
+| `selectedIndex=(float)`| (initial, warning)| `<number: -1>` |
 | `selectedIndex=(true)`| (initial, warning)| `<number: -1>` |
 | `selectedIndex=(false)`| (initial, warning)| `<number: -1>` |
-| `selectedIndex=(string 'true')`| (initial)| `<number: -1>` |
-| `selectedIndex=(string 'false')`| (initial)| `<number: -1>` |
-| `selectedIndex=(string 'on')`| (initial)| `<number: -1>` |
-| `selectedIndex=(string 'off')`| (initial)| `<number: -1>` |
+| `selectedIndex=(string 'true')`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(string 'false')`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(string 'on')`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(string 'off')`| (initial, warning)| `<number: -1>` |
 | `selectedIndex=(symbol)`| (initial, warning)| `<number: -1>` |
 | `selectedIndex=(function)`| (initial, warning)| `<number: -1>` |
-| `selectedIndex=(null)`| (initial)| `<number: -1>` |
-| `selectedIndex=(undefined)`| (initial)| `<number: -1>` |
+| `selectedIndex=(null)`| (initial, warning)| `<number: -1>` |
+| `selectedIndex=(undefined)`| (initial, warning)| `<number: -1>` |
 
 ## `shape` (on `<div>`)
 | Test Case | Flags | Result |

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMAttribute-test.js b/src/renderers/dom/shared/__tests__/ReactDOMAttribute-test.js
@@ -96,7 +96,7 @@ describe('ReactDOM unknown attribute', () => {
       expectDev(console.error.calls.count()).toBe(1);
     });
 
-    it('coerces objects to strings **and warns**', () => {
+    it('coerces objects to strings and warns', () => {
       const lol = {
         toString() {
           return 'lol';
@@ -133,5 +133,25 @@ describe('ReactDOM unknown attribute', () => {
       );
       expectDev(console.error.calls.count()).toBe(1);
     });
+
+    it('allows camelCase unknown attributes and warns', () => {
+      spyOn(console, 'error');
+
+      var el = document.createElement('div');
+      ReactDOM.render(<div helloWorld="something" />, el);
+      expect(el.firstChild.getAttribute('helloworld')).toBe('something');
+
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(
+        normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
+      ).toMatch(
+        'React does not recognize the `helloWorld` prop on a DOM element. ' +
+          'If you intentionally want it to appear in the DOM as a custom ' +
+          'attribute, spell it as lowercase `helloworld` instead. ' +
+          'If you accidentally passed it from a parent component, remove ' +
+          'it from the DOM element.\n' +
+          '    in div (at **)',
+      );
+    });
   });
 });
diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -2125,13 +2125,41 @@ describe('ReactDOMComponent', () => {
     });
 
     it('allows cased data attributes', function() {
+      spyOn(console, 'error');
+
       var el = ReactTestUtils.renderIntoDocument(<div data-fooBar="true" />);
       expect(el.getAttribute('data-foobar')).toBe('true');
+
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(
+        normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
+      ).toMatch(
+        'React does not recognize the `data-fooBar` prop on a DOM element. ' +
+          'If you intentionally want it to appear in the DOM as a custom ' +
+          'attribute, spell it as lowercase `data-foobar` instead. ' +
+          'If you accidentally passed it from a parent component, remove ' +
+          'it from the DOM element.\n' +
+          '    in div (at **)',
+      );
     });
 
     it('allows cased custom attributes', function() {
+      spyOn(console, 'error');
+
       var el = ReactTestUtils.renderIntoDocument(<div fooBar="true" />);
       expect(el.getAttribute('foobar')).toBe('true');
+
+      expectDev(console.error.calls.count()).toBe(1);
+      expectDev(
+        normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
+      ).toMatch(
+        'React does not recognize the `fooBar` prop on a DOM element. ' +
+          'If you intentionally want it to appear in the DOM as a custom ' +
+          'attribute, spell it as lowercase `foobar` instead. ' +
+          'If you accidentally passed it from a parent component, remove ' +
+          'it from the DOM element.\n' +
+          '    in div (at **)',
+      );
     });
 
     it('warns on NaN attributes', function() {
@@ -2195,10 +2223,8 @@ describe('ReactDOMComponent', () => {
       ReactDOM.render(<svg arabicForm={obj} />, container);
       expect(container.firstChild.getAttribute('arabic-form')).toBe('hello');
 
-      ReactDOM.render(<div customAttribute={obj} />, container);
-      expect(container.firstChild.getAttribute('customAttribute')).toBe(
-        'hello',
-      );
+      ReactDOM.render(<div unknown={obj} />, container);
+      expect(container.firstChild.getAttribute('unknown')).toBe('hello');
     });
 
     it('passes objects on known SVG attributes if they do not define toString', () => {
@@ -2215,8 +2241,8 @@ describe('ReactDOMComponent', () => {
       var obj = {};
       var container = document.createElement('div');
 
-      ReactDOM.render(<div customAttribute={obj} />, container);
-      expect(container.firstChild.getAttribute('customAttribute')).toBe(
+      ReactDOM.render(<div unknown={obj} />, container);
+      expect(container.firstChild.getAttribute('unknown')).toBe(
         '[object Object]',
       );
     });

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
@@ -909,25 +909,25 @@ describe('ReactDOMServerIntegration', () => {
       });
 
       itRenders('unknown data- attributes with casing', async render => {
-        const e = await render(<div data-fooBar="true" />);
-        expect(e.getAttribute('data-fooBar')).toBe('true');
+        const e = await render(<div data-fooBar="true" />, 1);
+        expect(e.getAttribute('data-foobar')).toBe('true');
       });
 
       itRenders('unknown data- attributes with boolean true', async render => {
-        const e = await render(<div data-fooBar={true} />);
-        expect(e.getAttribute('data-fooBar')).toBe('true');
+        const e = await render(<div data-foobar={true} />);
+        expect(e.getAttribute('data-foobar')).toBe('true');
       });
 
       itRenders('unknown data- attributes with boolean false', async render => {
-        const e = await render(<div data-fooBar={false} />);
-        expect(e.getAttribute('data-fooBar')).toBe('false');
+        const e = await render(<div data-foobar={false} />);
+        expect(e.getAttribute('data-foobar')).toBe('false');
       });
 
       itRenders(
         'no unknown data- attributes with casing and null value',
         async render => {
-          const e = await render(<div data-fooBar={null} />);
-          expect(e.hasAttribute('data-fooBar')).toBe(false);
+          const e = await render(<div data-fooBar={null} />, 1);
+          expect(e.hasAttribute('data-foobar')).toBe(false);
         },
       );
 
@@ -972,8 +972,8 @@ describe('ReactDOMServerIntegration', () => {
       });
 
       itRenders('cased custom attributes', async render => {
-        const e = await render(<div fooBar="test" />);
-        expect(e.getAttribute('fooBar')).toBe('test');
+        const e = await render(<div fooBar="test" />, 1);
+        expect(e.getAttribute('foobar')).toBe('test');
       });
     });
 

diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
@@ -39,7 +39,10 @@ if (__DEV__) {
   var warnedProperties = {};
   var hasOwnProperty = Object.prototype.hasOwnProperty;
   var EVENT_NAME_REGEX = /^on[A-Z]/;
-  var ARIA_NAME_REGEX = /^aria-/i;
+  var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$');
+  var rARIACamel = new RegExp(
+    '^(aria)[A-Z][' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$',
+  );
   var possibleStandardNames = require('possibleStandardNames');
 
   var validateProperty = function(tagName, name, value, debugID) {
@@ -91,7 +94,7 @@ if (__DEV__) {
     }
 
     // Let the ARIA attribute hook validate ARIA attributes
-    if (ARIA_NAME_REGEX.test(name)) {
+    if (rARIA.test(name) || rARIACamel.test(name)) {
       return true;
     }
 
@@ -155,6 +158,8 @@ if (__DEV__) {
       return true;
     }
 
+    const isReserved = DOMProperty.isReservedProp(name);
+
     // Known attributes should match the casing specified in the property config.
     if (possibleStandardNames.hasOwnProperty(lowerCasedName)) {
       var standardName = possibleStandardNames[lowerCasedName];
@@ -169,6 +174,22 @@ if (__DEV__) {
         warnedProperties[name] = true;
         return true;
       }
+    } else if (!isReserved && name !== lowerCasedName) {
+      // Unknown attributes should have lowercase casing since that's how they
+      // will be cased anyway with server rendering.
+      warning(
+        false,
+        'React does not recognize the `%s` prop on a DOM element. If you ' +
+          'intentionally want it to appear in the DOM as a custom ' +
+          'attribute, spell it as lowercase `%s` instead. ' +
+          'If you accidentally passed it from a parent component, remove ' +
+          'it from the DOM element.%s',
+        name,
+        lowerCasedName,
+        getStackAddendum(debugID),
+      );
+      warnedProperties[name] = true;
+      return true;
     }
 
     if (typeof value === 'boolean') {
@@ -186,7 +207,7 @@ if (__DEV__) {
 
     // Now that we've validated casing, do not validate
     // data types for reserved props
-    if (DOMProperty.isReservedProp(name)) {
+    if (isReserved) {
       return true;
     }