Skip to content

Commit

Permalink
Fix slow performance of PropTypes.oneOfType() on misses (#7510)
Browse files Browse the repository at this point in the history
It used to be slow whenever a type miss occurred because expensive `Error` objects were being created. For example, with `oneOfType([number, data])`, passing a date would create an `Error` object in `number` typechecker for every item.

The savings depend on how much commonly you used `oneOfType()`, and how often it had “misses”. If you used it heavily, you might see 1.5x to 2x performance improvements in `__DEV__` after this fix.
(cherry picked from commit 680685b)
  • Loading branch information
gaearon authored and zpao committed Aug 19, 2016
1 parent 9e88467 commit 536f826
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
38 changes: 26 additions & 12 deletions src/isomorphic/classic/types/ReactPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,20 @@ function is(x, y) {
}
/*eslint-enable no-self-compare*/

/**
* We use an Error-like object for backward compatibility as people may call
* PropTypes directly and inspect their output. However we don't use real
* Errors anymore. We don't inspect their stack anyway, and creating them
* is prohibitively expensive if they are created too often, such as what
* happens in oneOfType() for any type before the one that matched.
*/
function PropTypeError(message) {
this.message = message;
this.stack = '';
}
// Make `instanceof Error` still work for returned errors.
PropTypeError.prototype = Error.prototype;

function createChainableTypeChecker(validate) {
if (__DEV__) {
var manualPropTypeCallCache = {};
Expand Down Expand Up @@ -144,7 +158,7 @@ function createChainableTypeChecker(validate) {
if (props[propName] == null) {
var locationName = ReactPropTypeLocationNames[location];
if (isRequired) {
return new Error(
return new PropTypeError(
`Required ${locationName} \`${propFullName}\` was not specified in ` +
`\`${componentName}\`.`
);
Expand Down Expand Up @@ -185,7 +199,7 @@ function createPrimitiveTypeChecker(expectedType) {
// 'of type `object`'.
var preciseType = getPreciseType(propValue);

return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type ` +
`\`${preciseType}\` supplied to \`${componentName}\`, expected ` +
`\`${expectedType}\`.`
Expand All @@ -203,15 +217,15 @@ function createAnyTypeChecker() {
function createArrayOfTypeChecker(typeChecker) {
function validate(props, propName, componentName, location, propFullName) {
if (typeof typeChecker !== 'function') {
return new Error(
return new PropTypeError(
`Property \`${propFullName}\` of component \`${componentName}\` has invalid PropType notation inside arrayOf.`
);
}
var propValue = props[propName];
if (!Array.isArray(propValue)) {
var locationName = ReactPropTypeLocationNames[location];
var propType = getPropType(propValue);
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type ` +
`\`${propType}\` supplied to \`${componentName}\`, expected an array.`
);
Expand Down Expand Up @@ -240,7 +254,7 @@ function createElementTypeChecker() {
if (!ReactElement.isValidElement(propValue)) {
var locationName = ReactPropTypeLocationNames[location];
var propType = getPropType(propValue);
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type ` +
`\`${propType}\` supplied to \`${componentName}\`, expected a single ReactElement.`
);
Expand All @@ -256,7 +270,7 @@ function createInstanceTypeChecker(expectedClass) {
var locationName = ReactPropTypeLocationNames[location];
var expectedClassName = expectedClass.name || ANONYMOUS;
var actualClassName = getClassName(props[propName]);
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type ` +
`\`${actualClassName}\` supplied to \`${componentName}\`, expected ` +
`instance of \`${expectedClassName}\`.`
Expand All @@ -283,7 +297,7 @@ function createEnumTypeChecker(expectedValues) {

var locationName = ReactPropTypeLocationNames[location];
var valuesString = JSON.stringify(expectedValues);
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of value \`${propValue}\` ` +
`supplied to \`${componentName}\`, expected one of ${valuesString}.`
);
Expand All @@ -294,15 +308,15 @@ function createEnumTypeChecker(expectedValues) {
function createObjectOfTypeChecker(typeChecker) {
function validate(props, propName, componentName, location, propFullName) {
if (typeof typeChecker !== 'function') {
return new Error(
return new PropTypeError(
`Property \`${propFullName}\` of component \`${componentName}\` has invalid PropType notation inside objectOf.`
);
}
var propValue = props[propName];
var propType = getPropType(propValue);
if (propType !== 'object') {
var locationName = ReactPropTypeLocationNames[location];
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type ` +
`\`${propType}\` supplied to \`${componentName}\`, expected an object.`
);
Expand Down Expand Up @@ -351,7 +365,7 @@ function createUnionTypeChecker(arrayOfTypeCheckers) {
}

var locationName = ReactPropTypeLocationNames[location];
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` supplied to ` +
`\`${componentName}\`.`
);
Expand All @@ -363,7 +377,7 @@ function createNodeChecker() {
function validate(props, propName, componentName, location, propFullName) {
if (!isNode(props[propName])) {
var locationName = ReactPropTypeLocationNames[location];
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` supplied to ` +
`\`${componentName}\`, expected a ReactNode.`
);
Expand All @@ -379,7 +393,7 @@ function createShapeTypeChecker(shapeTypes) {
var propType = getPropType(propValue);
if (propType !== 'object') {
var locationName = ReactPropTypeLocationNames[location];
return new Error(
return new PropTypeError(
`Invalid ${locationName} \`${propFullName}\` of type \`${propType}\` ` +
`supplied to \`${componentName}\`, expected \`object\`.`
);
Expand Down
3 changes: 0 additions & 3 deletions src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ describe('ReactPropTypes', function() {
expectWarningInDevelopment(PropTypes.object.isRequired, null);
expectWarningInDevelopment(PropTypes.object.isRequired, undefined);
});


});

describe('Any type', function() {
Expand All @@ -223,7 +221,6 @@ describe('ReactPropTypes', function() {
expectWarningInDevelopment(PropTypes.any.isRequired, null);
expectWarningInDevelopment(PropTypes.any.isRequired, undefined);
});

});

describe('ArrayOf Type', function() {
Expand Down

0 comments on commit 536f826

Please sign in to comment.