Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Extract the type checker into a separate module #6851

Merged
merged 4 commits into from
May 24, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions src/isomorphic/classic/__tests__/ReactContextValidator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ var ReactTestUtils;
var reactComponentExpect;

describe('ReactContextValidator', function() {
function normalizeCodeLocInfo(str) {
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
}

beforeEach(function() {
jest.resetModuleRegistry();

Expand Down Expand Up @@ -146,9 +150,10 @@ describe('ReactContextValidator', function() {
ReactTestUtils.renderIntoDocument(<Component />);

expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed Context Types: ' +
'Required context `foo` was not specified in `Component`.'
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
'Warning: Failed context type: ' +
'Required context `foo` was not specified in `Component`.\n' +
' in Component (at **)'
);

var ComponentInFooStringContext = React.createClass({
Expand Down Expand Up @@ -193,11 +198,12 @@ describe('ReactContextValidator', function() {
ReactTestUtils.renderIntoDocument(<ComponentInFooNumberContext fooValue={123} />);

expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: Failed Context Types: ' +
expect(normalizeCodeLocInfo(console.error.argsForCall[1][0])).toBe(
'Warning: Failed context type: ' +
'Invalid context `foo` of type `number` supplied ' +
'to `Component`, expected `string`.' +
' Check the render method of `ComponentInFooNumberContext`.'
'to `Component`, expected `string`.\n' +
' in Component (at **)\n' +
' in ComponentInFooNumberContext (at **)'
);
});

Expand All @@ -221,18 +227,20 @@ describe('ReactContextValidator', function() {

ReactTestUtils.renderIntoDocument(<Component testContext={{bar: 123}} />);
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed Context Types: ' +
'Required child context `foo` was not specified in `Component`.'
expect(normalizeCodeLocInfo(console.error.argsForCall[0][0])).toBe(
'Warning: Failed childContext type: ' +
'Required child context `foo` was not specified in `Component`.\n' +
' in Component (at **)'
);

ReactTestUtils.renderIntoDocument(<Component testContext={{foo: 123}} />);

expect(console.error.argsForCall.length).toBe(2);
expect(console.error.argsForCall[1][0]).toBe(
'Warning: Failed Context Types: ' +
expect(normalizeCodeLocInfo(console.error.argsForCall[1][0])).toBe(
'Warning: Failed childContext type: ' +
'Invalid child context `foo` of type `number` ' +
'supplied to `Component`, expected `string`.'
'supplied to `Component`, expected `string`.\n' +
' in Component (at **)'
);

ReactTestUtils.renderIntoDocument(
Expand Down
76 changes: 8 additions & 68 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
var ReactCurrentOwner = require('ReactCurrentOwner');
var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
var ReactElement = require('ReactElement');
var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');
var ReactPropTypeLocations = require('ReactPropTypeLocations');

var checkReactTypeSpec = require('checkReactTypeSpec');

var canDefineProperty = require('canDefineProperty');
var getIteratorFn = require('getIteratorFn');
var invariant = require('invariant');
var warning = require('warning');

function getDeclarationErrorAddendum() {
Expand All @@ -46,8 +46,6 @@ function getDeclarationErrorAddendum() {
*/
var ownerHasKeyUseWarning = {};

var loggedTypeFailures = {};

function getCurrentComponentErrorInfo(parentType) {
var info = getDeclarationErrorAddendum();

Expand Down Expand Up @@ -152,66 +150,6 @@ function validateChildKeys(node, parentType) {
}
}

/**
* Assert that the props are valid
*
* @param {object} element
* @param {string} componentName Name of the component for error messages.
* @param {object} propTypes Map of prop name to a ReactPropType
* @param {string} location e.g. "prop", "context", "child context"
* @private
*/
function checkPropTypes(element, componentName, propTypes, location) {
var props = element.props;
for (var propName in propTypes) {
if (propTypes.hasOwnProperty(propName)) {
var error;
// Prop type validation may throw. In case they do, we don't want to
// fail the render phase where it didn't fail before. So we log it.
// After these have been cleaned up, we'll let them throw.
try {
// This is intentionally an invariant that gets caught. It's the same
// behavior as without this statement except with a better message.
invariant(
typeof propTypes[propName] === 'function',
'%s: %s type `%s` is invalid; it must be a function, usually from ' +
'React.PropTypes.',
componentName || 'React class',
ReactPropTypeLocationNames[location],
propName
);
error = propTypes[propName](props, propName, componentName, location);
} catch (ex) {
error = ex;
}
warning(
!error || error instanceof Error,
'%s: type specification of %s `%s` is invalid; the type checker ' +
'function must return `null` or an `Error` but returned a %s. ' +
'You may have forgotten to pass an argument to the type checker ' +
'creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and ' +
'shape all require an argument).',
componentName || 'React class',
ReactPropTypeLocationNames[location],
propName,
typeof error
);
if (error instanceof Error && !(error.message in loggedTypeFailures)) {
// Only monitor this failure once because there tends to be a lot of the
// same error.
loggedTypeFailures[error.message] = true;

warning(
false,
'Failed propType: %s%s',
error.message,
ReactComponentTreeDevtool.getCurrentStackAddendum(element)
);
}
}
}
}

/**
* Given an element, validate that its props follow the propTypes definition,
* provided by the type.
Expand All @@ -225,11 +163,13 @@ function validatePropTypes(element) {
}
var name = componentClass.displayName || componentClass.name;
if (componentClass.propTypes) {
checkPropTypes(
element,
name,
checkReactTypeSpec(
componentClass.propTypes,
ReactPropTypeLocations.prop
element.props,
ReactPropTypeLocations.prop,
name,
element,
null
);
}
if (typeof componentClass.getDefaultProps === 'function') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ describe('ReactElementClone', function() {
ReactTestUtils.renderIntoDocument(React.createElement(GrandParent));
expect(console.error.argsForCall.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Invalid prop `color` of type `number` supplied to `Component`, ' +
'expected `string`.\n' +
' in Component (created by GrandParent)\n' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ describe('ReactElementValidator', function() {
});
ReactTestUtils.renderIntoDocument(React.createElement(ParentComp));
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Invalid prop `color` of type `number` supplied to `MyComp`, ' +
'expected `string`.\n' +
' in MyComp (created by ParentComp)\n' +
Expand Down Expand Up @@ -360,7 +360,7 @@ describe('ReactElementValidator', function() {

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);
Expand All @@ -385,7 +385,7 @@ describe('ReactElementValidator', function() {

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);
Expand All @@ -412,13 +412,13 @@ describe('ReactElementValidator', function() {

expect(console.error.calls.length).toBe(2);
expect(console.error.argsForCall[0][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `Component`.\n' +
' in Component'
);

expect(console.error.argsForCall[1][0]).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Invalid prop `prop` of type `number` supplied to ' +
'`Component`, expected `string`.\n' +
' in Component'
Expand Down
4 changes: 2 additions & 2 deletions src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ describe('ReactPropTypes', function() {
instance = ReactTestUtils.renderIntoDocument(instance);

expect(spy.argsForCall.length).toBe(1);
expect(spy.argsForCall[0][1]).toBe('num');
expect(spy.argsForCall[0][1]).toBe('num');
});

it('should have received the validator\'s return value', function() {
Expand All @@ -894,7 +894,7 @@ describe('ReactPropTypes', function() {
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: num must be 5!\n' +
'Warning: Failed prop type: num must be 5!\n' +
' in Component (at **)'
);
});
Expand Down
93 changes: 93 additions & 0 deletions src/isomorphic/classic/types/checkReactTypeSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule checkReactTypeSpec
*/

'use strict';

var ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');
var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames');

var invariant = require('invariant');
var warning = require('warning');

var loggedTypeFailures = {};

/**
* Assert that the values match with the type specs.
* Error messages are memorized and will only be shown once.
*
* @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 {?object} element The React element that is being type-checked
* @param {?number} debugID The React component instance that is being type-checked
* @private
*/
function checkReactTypeSpec(typeSpecs, values, location, componentName, element, debugID) {
for (var typeSpecName in typeSpecs) {
if (typeSpecs.hasOwnProperty(typeSpecName)) {
var error;
// Prop type validation may throw. In case they do, we don't want to
// fail the render phase where it didn't fail before. So we log it.
// After these have been cleaned up, we'll let them throw.
try {
// This is intentionally an invariant that gets caught. It's the same
// behavior as without this statement except with a better message.
invariant(
typeof typeSpecs[typeSpecName] === 'function',
'%s: %s type `%s` is invalid; it must be a function, usually from ' +
'React.PropTypes.',
componentName || 'React class',
ReactPropTypeLocationNames[location],
typeSpecName
);
error = typeSpecs[typeSpecName](values, typeSpecName, componentName, location);
} catch (ex) {
error = ex;
}
warning(
!error || error instanceof Error,
'%s: type specification of %s `%s` is invalid; the type checker ' +
'function must return `null` or an `Error` but returned a %s. ' +
'You may have forgotten to pass an argument to the type checker ' +
'creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and ' +
'shape all require an argument).',
componentName || 'React class',
ReactPropTypeLocationNames[location],
typeSpecName,
typeof error
);
if (error instanceof Error && !(error.message in loggedTypeFailures)) {
// Only monitor this failure once because there tends to be a lot of the
// same error.
loggedTypeFailures[error.message] = true;

var componentStackInfo = '';

if (debugID !== null) {
componentStackInfo = ReactComponentTreeDevtool.getStackAddendumByID(debugID);
} else if (element !== null) {
componentStackInfo = ReactComponentTreeDevtool.getCurrentStackAddendum(element);
}

warning(
false,
'Failed %s type: %s%s',
location,
error.message,
componentStackInfo
);
}
}
}
}

module.exports = checkReactTypeSpec;
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ describe('ReactJSXElementValidator', function() {
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Invalid prop `color` of type `number` supplied to `MyComp`, ' +
'expected `string`.\n' +
' in MyComp (at **)\n' +
Expand Down Expand Up @@ -249,7 +249,7 @@ describe('ReactJSXElementValidator', function() {
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
' in RequiredPropComponent (at **)'
);
Expand All @@ -264,7 +264,7 @@ describe('ReactJSXElementValidator', function() {
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
' in RequiredPropComponent (at **)'
);
Expand All @@ -280,15 +280,15 @@ describe('ReactJSXElementValidator', function() {
expect(
console.error.argsForCall[0][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Required prop `prop` was not specified in `RequiredPropComponent`.\n' +
' in RequiredPropComponent (at **)'
);

expect(
console.error.argsForCall[1][0].replace(/\(at .+?:\d+\)/g, '(at **)')
).toBe(
'Warning: Failed propType: ' +
'Warning: Failed prop type: ' +
'Invalid prop `prop` of type `number` supplied to ' +
'`RequiredPropComponent`, expected `string`.\n' +
' in RequiredPropComponent (at **)'
Expand Down
Loading