Prevent performance regression in DEV due to warning arguments (#7461)

* Prevent internal performance regression

This only affects Facebook website, not open source version of React.

On the Facebook website, we don't have a transform for warnings and invariants.
Therefore, expensive arguments will be calculated even if the warning doesn't fire.
This fixes a few cases where that calculation might be more expensive than usually.

In my testing, this brings down average row click time in Power Editor from ~300ms to ~220ms in __DEV__ (vs ~40ms in prod).

* Put warning() that shows up in profile behind condition
This commit is contained in:
Dan Abramov
2016-08-10 19:52:46 +01:00
committed by GitHub
parent 5514ea369d
commit 178cb7d339
7 changed files with 46 additions and 35 deletions
@@ -188,13 +188,15 @@ var ReactElementValidator = {
(type !== null && typeof type === 'object');
// We warn in this case but don't throw. We expect the element creation to
// succeed and there will likely be errors in render.
warning(
validType,
'React.createElement: type should not be null, undefined, boolean, or ' +
'number. It should be a string (for DOM elements) or a ReactClass ' +
'(for composite components).%s',
getDeclarationErrorAddendum()
);
if (!validType) {
warning(
false,
'React.createElement: type should not be null, undefined, boolean, or ' +
'number. It should be a string (for DOM elements) or a ReactClass ' +
'(for composite components).%s',
getDeclarationErrorAddendum()
);
}
var element = ReactElement.createElement.apply(this, arguments);
@@ -71,17 +71,18 @@ function checkSelectPropTypes(inst, props) {
if (props[propName] == null) {
continue;
}
if (props.multiple) {
var isArray = Array.isArray(props[propName]);
if (props.multiple && !isArray) {
warning(
Array.isArray(props[propName]),
false,
'The `%s` prop supplied to <select> must be an array if ' +
'`multiple` is true.%s',
propName,
getDeclarationErrorAddendum(owner)
);
} else {
} else if (!props.multiple && isArray) {
warning(
!Array.isArray(props[propName]),
false,
'The `%s` prop supplied to <select> must be a scalar ' +
'value if `multiple` is false.%s',
propName,
@@ -69,7 +69,7 @@ if (__DEV__) {
if (standardName != null) {
warning(
standardName == null,
false,
'Unknown DOM property %s. Did you mean %s?%s',
name,
standardName,
@@ -78,7 +78,7 @@ if (__DEV__) {
return true;
} else if (registrationName != null) {
warning(
registrationName == null,
false,
'Unknown event handler property %s. Did you mean `%s`?%s',
name,
registrationName,
+3 -1
View File
@@ -104,7 +104,9 @@ function resetMeasurements() {
}
function checkDebugID(debugID) {
warning(debugID, 'ReactDebugTool: debugID may not be empty.');
if (!debugID) {
warning(false, 'ReactDebugTool: debugID may not be empty.');
}
}
function beginLifeCycleTimer(debugID, timerType) {
@@ -37,11 +37,13 @@ function handleElement(debugID, element) {
isMutated = true;
}
}
warning(
Array.isArray(element._shadowChildren) && !isMutated,
'Component\'s children should not be mutated.%s',
ReactComponentTreeHook.getStackAddendumByID(debugID),
);
if (!Array.isArray(element._shadowChildren) || isMutated) {
warning(
false,
'Component\'s children should not be mutated.%s',
ReactComponentTreeHook.getStackAddendumByID(debugID),
);
}
}
var ReactChildrenMutationWarningHook = {
@@ -41,14 +41,16 @@ function instantiateChild(childInstances, child, name, selfDebugID) {
if (!ReactComponentTreeHook) {
ReactComponentTreeHook = require('ReactComponentTreeHook');
}
warning(
keyUnique,
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.%s',
KeyEscapeUtils.unescape(name),
ReactComponentTreeHook.getStackAddendumByID(selfDebugID)
);
if (!keyUnique) {
warning(
false,
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.%s',
KeyEscapeUtils.unescape(name),
ReactComponentTreeHook.getStackAddendumByID(selfDebugID)
);
}
}
if (child != null && keyUnique) {
childInstances[name] = instantiateReactComponent(child, true);
+10 -8
View File
@@ -51,14 +51,16 @@ function flattenSingleChildIntoContext(
if (!ReactComponentTreeHook) {
ReactComponentTreeHook = require('ReactComponentTreeHook');
}
warning(
keyUnique,
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.%s',
KeyEscapeUtils.unescape(name),
ReactComponentTreeHook.getStackAddendumByID(selfDebugID)
);
if (!keyUnique) {
warning(
false,
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.%s',
KeyEscapeUtils.unescape(name),
ReactComponentTreeHook.getStackAddendumByID(selfDebugID)
);
}
}
if (keyUnique && child != null) {
result[name] = child;