Warn when "false" or "true" is the value of a boolean DOM prop (#13372)

* Warn when the string "false" is the value of a boolean DOM prop * Only warn on exact case match for "false" in DOM boolean props * Warn on string "true" as well as "false" in DOM boolean props * Clarify warnings on "true" / "false" values in DOM boolean props
facebook · Aug 14, 2018 · 2c59076 · 2c59076
1 parent 24b0ed7
commit 2c59076
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 1 deletion.
diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -2506,6 +2506,34 @@ describe('ReactDOMComponent', () => {
     });
   });
 
+  describe('Boolean attributes', function() {
+    it('warns on the ambiguous string value "false"', function() {
+      let el;
+      expect(() => {
+        el = ReactTestUtils.renderIntoDocument(<div hidden="false" />);
+      }).toWarnDev(
+        'Received the string `false` for the boolean attribute `hidden`. ' +
+          'The browser will interpret it as a truthy value. ' +
+          'Did you mean hidden={false}?',
+      );
+
+      expect(el.getAttribute('hidden')).toBe('');
+    });
+
+    it('warns on the potentially-ambiguous string value "true"', function() {
+      let el;
+      expect(() => {
+        el = ReactTestUtils.renderIntoDocument(<div hidden="true" />);
+      }).toWarnDev(
+        'Received the string `true` for the boolean attribute `hidden`. ' +
+          'Although this works, it will not work as expected if you pass the string "false". ' +
+          'Did you mean hidden={true}?',
+      );
+
+      expect(el.getAttribute('hidden')).toBe('');
+    });
+  });
+
   describe('Hyphenated SVG elements', function() {
     it('the font-face element is not a custom element', function() {
       let el;

diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js
@@ -444,7 +444,7 @@ describe('ReactDOMInput', () => {
 
   it('should take `defaultValue` when changing to uncontrolled input', () => {
     const node = ReactDOM.render(
-      <input type="text" value="0" readOnly="true" />,
+      <input type="text" value="0" readOnly={true} />,
       container,
     );
     expect(node.value).toBe('0');

diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
@@ -13,6 +13,7 @@ import warning from 'shared/warning';
 
 import {
   ATTRIBUTE_NAME_CHAR,
+  BOOLEAN,
   RESERVED,
   shouldRemoveAttributeWithWarning,
   getPropertyInfo,
@@ -226,6 +227,29 @@ if (__DEV__) {
       return false;
     }
 
+    // Warn when passing the strings 'false' or 'true' into a boolean prop
+    if (
+      (value === 'false' || value === 'true') &&
+      propertyInfo !== null &&
+      propertyInfo.type === BOOLEAN
+    ) {
+      warning(
+        false,
+        'Received the string `%s` for the boolean attribute `%s`. ' +
+          '%s ' +
+          'Did you mean %s={%s}?',
+        value,
+        name,
+        value === 'false'
+          ? 'The browser will interpret it as a truthy value.'
+          : 'Although this works, it will not work as expected if you pass the string "false".',
+        name,
+        value,
+      );
+      warnedProperties[name] = true;
+      return true;
+    }
+
     return true;
   };
 }