From 0b2172318cff19ff9596615757de5077d5717a3b Mon Sep 17 00:00:00 2001
From: Moti Zilberman <motiz88@gmail.com>
Date: Wed, 15 Aug 2018 10:22:58 +0100
Subject: [PATCH] Warn when an empty string is passed to a DOM boolean prop

---
 .../src/__tests__/ReactDOMComponent-test.js   | 39 +++++++++++++++++++
 ...eactDOMServerIntegrationAttributes-test.js |  2 +-
 .../src/shared/ReactDOMUnknownPropertyHook.js | 28 ++++++++++++-
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
index b83ce5d5e1f3..e425d6d1fc7f 100644
--- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -2532,6 +2532,45 @@ describe('ReactDOMComponent', () => {
 
       expect(el.getAttribute('hidden')).toBe('');
     });
+
+    it('warns on the ambiguous string value "" when it means false', function() {
+      let el;
+      expect(() => {
+        el = ReactTestUtils.renderIntoDocument(<div hidden="" />);
+      }).toWarnDev(
+        'Received the string "" for the boolean attribute `hidden`. ' +
+          'This value can mean `true` or `false`, depending on the attribute. ' +
+          'Did you mean hidden={false}?',
+      );
+
+      expect(el.hasAttribute('hidden')).toBe(false);
+    });
+
+    it('warns on the ambiguous string value "" when it means true', function() {
+      let el;
+      expect(() => {
+        el = ReactTestUtils.renderIntoDocument(<div spellCheck="" />);
+      }).toWarnDev(
+        'Received the string "" for the boolean attribute `spellCheck`. ' +
+          'This value can mean `true` or `false`, depending on the attribute. ' +
+          'Did you mean spellCheck={true}?',
+      );
+
+      expect(el.getAttribute('spellCheck')).toBe('');
+    });
+
+    it('warns on the ambiguous string value "" in an overloaded boolean prop', function() {
+      let el;
+      expect(() => {
+        el = ReactTestUtils.renderIntoDocument(<input capture="" />);
+      }).toWarnDev(
+        'Received the string "" for the boolean attribute `capture`. ' +
+          'This value can mean `true` or `false`, depending on the attribute. ' +
+          'Did you mean capture={true}?',
+      );
+
+      expect(el.getAttribute('capture')).toBe('');
+    });
   });
 
   describe('Hyphenated SVG elements', function() {
diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
index 2838ad660afd..7b410df5e58f 100644
--- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
@@ -98,7 +98,7 @@ describe('ReactDOMServerIntegration', () => {
       // that the boolean property is present. however, it is how the current code
       // behaves, so the test is included here.
       itRenders('boolean prop with "" value', async render => {
-        const e = await render(<div hidden="" />);
+        const e = await render(<div hidden="" />, 1);
         expect(e.getAttribute('hidden')).toBe(null);
       });
 
diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
index 3c42172f9eac..378908d09eb4 100644
--- a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
+++ b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
@@ -14,6 +14,8 @@ import warning from 'shared/warning';
 import {
   ATTRIBUTE_NAME_CHAR,
   BOOLEAN,
+  BOOLEANISH_STRING,
+  OVERLOADED_BOOLEAN,
   RESERVED,
   shouldRemoveAttributeWithWarning,
   getPropertyInfo,
@@ -227,7 +229,7 @@ if (__DEV__) {
       return false;
     }
 
-    // Warn when passing the strings 'false' or 'true' into a boolean prop
+    // Warn about passing the strings 'false' or 'true' into a boolean prop
     if (
       (value === 'false' || value === 'true') &&
       propertyInfo !== null &&
@@ -250,6 +252,30 @@ if (__DEV__) {
       return true;
     }
 
+    // Warn about passing an empty string into any kind of boolean prop, except
+    // 'value' which is modeled as a "booleanish string"
+    if (
+      value === '' &&
+      name !== 'value' &&
+      propertyInfo !== null &&
+      (propertyInfo.type === BOOLEAN ||
+        propertyInfo.type === BOOLEANISH_STRING ||
+        propertyInfo.type === OVERLOADED_BOOLEAN)
+    ) {
+      const isBoolean = propertyInfo.type === BOOLEAN;
+      warning(
+        false,
+        'Received the string "" for the boolean attribute `%s`. ' +
+          'This value can mean `true` or `false`, depending on the attribute. ' +
+          'Did you mean %s={%s}?',
+        name,
+        name,
+        isBoolean ? 'false' : 'true',
+      );
+      warnedProperties[name] = true;
+      return true;
+    }
+
     return true;
   };
 }