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

* 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

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 **)',
  );

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.',
  );
  });
  });

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');

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

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;
  }