Fixed invalid prop types error message to be more specific (#11308)

NicBonetto · gaearon · commit 544d5c720812 · 2017-10-31T13:02:41.000Z
* Modified tests and corrected error message. #3 * Fixed syntax issues. #3 * Modified test. #3 * Prettified. #3 * Changed warning message to handle true and false boolean values. #3 * Changed test to contain undefined instead of value. #3 * Simplified branch structure. #3 * Refactored branching logic. #3 * Refactored falsy warning message and tests. #3 * Changed condition to attribute name. #3 * Refactored falsy and truthy warning messages with tests updated. #3 * Added missing character. #3 * Fixed warning message. #3 * Cleared extra whitespace. #3 * Refactored warning messages to be clear. #3 * Prettified. #3 * Grammar fix * Tweak unrelated warning The message didn't make sense because it appears for *any* attributes, not just numeric ones. * Tweak the message for more clarity * Add a special message for false event handlers * Add missing whitespace * Revert size changes
diff --git a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js
@@ -54,8 +54,9 @@ describe('ReactDOM unknown attribute', () => {
       expectDev(
         normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
       ).toMatch(
-        'Warning: Received `true` for non-boolean attribute `unknown`. ' +
-          'If this is expected, cast the value to a string.\n' +
+        'Received `true` for a non-boolean attribute `unknown`.\n\n' +
+          'If you want to write it to the DOM, pass a string instead: ' +
+          'unknown="true" or unknown={value.toString()}.\n' +
           '    in div (at **)',
       );
       expectDev(console.error.calls.count()).toBe(1);
@@ -87,7 +88,7 @@ describe('ReactDOM unknown attribute', () => {
       expectDev(
         normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
       ).toMatch(
-        'Warning: Received NaN for numeric attribute `unknown`. ' +
+        'Warning: Received NaN for the `unknown` attribute. ' +
           'If this is expected, cast the value to a string.\n' +
           '    in div (at **)',
       );
diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -714,7 +714,7 @@ describe('ReactDOMComponent', () => {
 
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Received a `function` for string attribute `is`. If this is expected, cast ' +
+        'Received a `function` for a string attribute `is`. If this is expected, cast ' +
           'the value to a string.',
       );
     });
@@ -2042,7 +2042,9 @@ describe('ReactDOMComponent', () => {
       expect(el.hasAttribute('whatever')).toBe(false);
 
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Received `true` for non-boolean attribute `whatever`',
+        'Received `true` for a non-boolean attribute `whatever`.\n\n' +
+          'If you want to write it to the DOM, pass a string instead: ' +
+          'whatever="true" or whatever={value.toString()}.',
       );
     });
 
@@ -2055,7 +2057,9 @@ describe('ReactDOMComponent', () => {
       expect(el.hasAttribute('whatever')).toBe(false);
 
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Received `true` for non-boolean attribute `whatever`',
+        'Received `true` for a non-boolean attribute `whatever`.\n\n' +
+          'If you want to write it to the DOM, pass a string instead: ' +
+          'whatever="true" or whatever={value.toString()}.',
       );
     });
 
@@ -2128,7 +2132,7 @@ describe('ReactDOMComponent', () => {
       expect(el.getAttribute('whatever')).toBe('NaN');
 
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Received NaN for numeric attribute `whatever`. If this is ' +
+        'Warning: Received NaN for the `whatever` attribute. If this is ' +
           'expected, cast the value to a string.\n    in div',
       );
     });
@@ -2230,7 +2234,9 @@ describe('ReactDOMComponent', () => {
       expect(el.hasAttribute('whatever')).toBe(false);
 
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Received `true` for non-boolean attribute `whatever`.',
+        'Received `true` for a non-boolean attribute `whatever`.\n\n' +
+          'If you want to write it to the DOM, pass a string instead: ' +
+          'whatever="true" or whatever={value.toString()}.',
       );
     });
 
@@ -2283,7 +2289,11 @@ describe('ReactDOMComponent', () => {
 
       expectDev(console.error.calls.count()).toBe(1);
       expectDev(console.error.calls.argsFor(0)[0]).toContain(
-        'Warning: Received `false` for non-boolean attribute `whatever`.',
+        'Received `false` for a non-boolean attribute `whatever`.\n\n' +
+          'If you want to write it to the DOM, pass a string instead: ' +
+          'whatever="false" or whatever={value.toString()}.\n\n' +
+          'If you used to conditionally omit it with whatever={condition && value}, ' +
+          'pass whatever={condition ? value : undefined} instead.',
       );
     });
   });
diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js
@@ -945,6 +945,25 @@ describe('ReactDOMFiber', () => {
     );
   });
 
+  it('should warn with a special message for `false` event listeners', () => {
+    spyOn(console, 'error');
+    class Example extends React.Component {
+      render() {
+        return <div onClick={false} />;
+      }
+    }
+    ReactDOM.render(<Example />, container);
+    expectDev(console.error.calls.count()).toBe(1);
+    expectDev(
+      normalizeCodeLocInfo(console.error.calls.argsFor(0)[0]),
+    ).toContain(
+      'Expected `onClick` listener to be a function, instead got `false`.\n\n' +
+        'If you used to conditionally omit it with onClick={condition && value}, ' +
+        'pass onClick={condition ? value : undefined} instead.\n',
+      '    in div (at **)\n' + '    in Example (at **)',
+    );
+  });
+
   it('should not update event handlers until commit', () => {
     let ops = [];
     const handlerA = () => ops.push('A');
diff --git a/packages/react-dom/src/client/ReactDOMFiberComponent.js b/packages/react-dom/src/client/ReactDOMFiberComponent.js
@@ -166,13 +166,26 @@ if (__DEV__) {
   };
 
   var warnForInvalidEventListener = function(registrationName, listener) {
-    warning(
-      false,
-      'Expected `%s` listener to be a function, instead got a value of `%s` type.%s',
-      registrationName,
-      typeof listener,
-      getCurrentFiberStackAddendum(),
-    );
+    if (listener === false) {
+      warning(
+        false,
+        'Expected `%s` listener to be a function, instead got `false`.\n\n' +
+          'If you used to conditionally omit it with %s={condition && value}, ' +
+          'pass %s={condition ? value : undefined} instead.%s',
+        registrationName,
+        registrationName,
+        registrationName,
+        getCurrentFiberStackAddendum(),
+      );
+    } else {
+      warning(
+        false,
+        'Expected `%s` listener to be a function, instead got a value of `%s` type.%s',
+        registrationName,
+        typeof listener,
+        getCurrentFiberStackAddendum(),
+      );
+    }
   };
 
   // Parse the HTML and read it back to normalize the HTML string so that it
diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js
@@ -124,7 +124,7 @@ if (__DEV__) {
     ) {
       warning(
         false,
-        'Received a `%s` for string attribute `is`. If this is expected, cast ' +
+        'Received a `%s` for a string attribute `is`. If this is expected, cast ' +
           'the value to a string.%s',
         typeof value,
         getStackAddendum(),
@@ -136,7 +136,7 @@ if (__DEV__) {
     if (typeof value === 'number' && isNaN(value)) {
       warning(
         false,
-        'Received NaN for numeric attribute `%s`. If this is expected, cast ' +
+        'Received NaN for the `%s` attribute. If this is expected, cast ' +
           'the value to a string.%s',
         name,
         getStackAddendum(),
@@ -179,15 +179,41 @@ if (__DEV__) {
       return true;
     }
 
-    if (typeof value === 'boolean') {
-      warning(
-        DOMProperty.shouldAttributeAcceptBooleanValue(name),
-        'Received `%s` for non-boolean attribute `%s`. If this is expected, cast ' +
-          'the value to a string.%s',
-        value,
-        name,
-        getStackAddendum(),
-      );
+    if (
+      typeof value === 'boolean' &&
+      !DOMProperty.shouldAttributeAcceptBooleanValue(name)
+    ) {
+      if (value) {
+        warning(
+          false,
+          'Received `%s` for a non-boolean attribute `%s`.\n\n' +
+            'If you want to write it to the DOM, pass a string instead: ' +
+            '%s="%s" or %s={value.toString()}.%s',
+          value,
+          name,
+          name,
+          value,
+          name,
+          getStackAddendum(),
+        );
+      } else {
+        warning(
+          false,
+          'Received `%s` for a non-boolean attribute `%s`.\n\n' +
+            'If you want to write it to the DOM, pass a string instead: ' +
+            '%s="%s" or %s={value.toString()}.\n\n' +
+            'If you used to conditionally omit it with %s={condition && value}, ' +
+            'pass %s={condition ? value : undefined} instead.%s',
+          value,
+          name,
+          name,
+          value,
+          name,
+          name,
+          name,
+          getStackAddendum(),
+        );
+      }
       warnedProperties[name] = true;
       return true;
     }
