Skip to content

Commit

Permalink
Merge pull request #2434 from preactjs/fix-check-prop-types-args
Browse files Browse the repository at this point in the history
Fix error messages logged by failed prop type checks
  • Loading branch information
robertknight committed Apr 1, 2020
2 parents 6a9f74c + 84453bf commit a299d1d
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
19 changes: 19 additions & 0 deletions debug/src/check-props.js
Expand Up @@ -2,6 +2,25 @@ const ReactPropTypesSecret = 'SECRET_DO_NOT_PASS_THIS_OR_YOU_WILL_BE_FIRED';

let loggedTypeFailures = {};

/**
* Reset the history of which prop type warnings have been logged.
*/
export function resetPropWarnings() {
loggedTypeFailures = {};
}

/**
* Assert that the values match with the type specs.
* Error messages are memorized and will only be shown once.
*
* Adapted from https://github.com/facebook/prop-types/blob/master/checkPropTypes.js
*
* @param {object} typeSpecs Map of name to a ReactPropType
* @param {object} values Runtime values that need to be type-checked
* @param {string} location e.g. "prop", "context", "child context"
* @param {string} componentName Name of the component for error messages.
* @param {?Function} getStack Returns the component stack.
*/
export function checkPropTypes(
typeSpecs,
values,
Expand Down
4 changes: 2 additions & 2 deletions debug/src/debug.js
Expand Up @@ -214,8 +214,8 @@ export function initDebug() {
checkPropTypes(
vnode.type.propTypes,
vnode.props,
getDisplayName(vnode),
serializeVNode(vnode)
'prop',
getDisplayName(vnode)
);
}

Expand Down
2 changes: 2 additions & 0 deletions debug/src/index.js
Expand Up @@ -2,3 +2,5 @@ import { initDebug } from './debug';
import 'preact/devtools';

initDebug();

export { resetPropWarnings } from './check-props';
41 changes: 39 additions & 2 deletions debug/test/browser/debug.test.js
Expand Up @@ -8,6 +8,9 @@ import './fakeDevTools';
import 'preact/debug';
import * as PropTypes from 'prop-types';

// eslint-disable-next-line no-duplicate-imports
import { resetPropWarnings } from 'preact/debug';

const h = createElement;
/** @jsx createElement */

Expand Down Expand Up @@ -506,6 +509,10 @@ describe('debug', () => {
});

describe('PropTypes', () => {
beforeEach(() => {
resetPropWarnings();
});

it("should fail if props don't match prop-types", () => {
function Foo(props) {
return <h1>{props.text}</h1>;
Expand All @@ -515,10 +522,40 @@ describe('debug', () => {
text: PropTypes.string.isRequired
};

render(<Foo />, scratch);
render(<Foo text={123} />, scratch);

expect(console.error).to.be.calledOnce;

// The message here may change when the "prop-types" library is updated,
// but we check it exactly to make sure all parameters were supplied
// correctly.
expect(console.error).to.be.calledWith(
'Failed prop type: Invalid prop `text` of type `number` supplied to `Foo`, expected `string`.'
);
});

it('should only log a given prop type error once', () => {
function Foo(props) {
return <h1>{props.text}</h1>;
}

Foo.propTypes = {
text: PropTypes.string.isRequired,
count: PropTypes.number
};

// Trigger the same error twice. The error should only be logged
// once.
render(<Foo text={123} />, scratch);
render(<Foo text={123} />, scratch);

expect(console.error).to.be.calledOnce;

// Trigger a different error. This should result in a new log
// message.
console.error.resetHistory();
render(<Foo text="ok" count="123" />, scratch);
expect(console.error).to.be.calledOnce;
expect(errors[0].includes('required')).to.equal(true);
});

it('should render with error logged when validator gets signal and throws exception', () => {
Expand Down

0 comments on commit a299d1d

Please sign in to comment.