Skip to content

Commit

Permalink
Prevent performance regression in DEV due to warning arguments (#7461)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
gaearon authored Aug 10, 2016
1 parent 5514ea3 commit 178cb7d
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 35 deletions.
16 changes: 9 additions & 7 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
9 changes: 5 additions & 4 deletions src/renderers/dom/client/wrappers/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ if (__DEV__) {

if (standardName != null) {
warning(
standardName == null,
false,
'Unknown DOM property %s. Did you mean %s?%s',
name,
standardName,
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/renderers/shared/ReactDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 7 additions & 5 deletions src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
18 changes: 10 additions & 8 deletions src/renderers/shared/stack/reconciler/ReactChildReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 10 additions & 8 deletions src/shared/utils/flattenChildren.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 178cb7d

Please sign in to comment.