Fix slow performance of PropTypes.oneOfType() on misses #7510
Conversation
ghost
added
| this.stack = ''; | ||
| } | ||
| // Make `instanceof Error` still work for returned errors. | ||
| PropTypeError.prototype = Object.create(Error.prototype); |
There was a problem hiding this comment.
We got rid of the need to polyfill Object.create in other places I believe. Should we do the same here?
There was a problem hiding this comment.
We don't officially support IE8 any more so I'm not that worried. People can polyfill if they want.
There was a problem hiding this comment.
Just PropTypeError.prototype = Error.prototype should work in our case right?
|
Can you define toString on the prototype that just returns message? Or maybe you get that for free already. Otherwise good. |
Yeah, it works thanks to |
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.
| this.stack = ''; | ||
| } | ||
| // Make `instanceof Error` still work for returned errors. | ||
| PropTypeError.prototype = Error.prototype; |
There was a problem hiding this comment.
I think this is tricky. https://github.com/JsCommunity/make-error/blob/master/index.js#L12
Maybe I wrong, not sure.
There was a problem hiding this comment.
It's easier here since we don't want the stack so we don't need to call the Error constructor.
There was a problem hiding this comment.
I think code below could be better if we don't care about IE<9 (and I hope nobody does develop in IE<9 😅):
function PropTypeError(message) {
this.message = message;
}
PropTypeError.prototype = Object.create(Error.prototype, { stack: { value: '' }});
|
Crap, blames to me. It's my evil plan to push people to use Flow instead. |
|
Interesting, I want to do something similar with BuckleScript. raise real Error object in DEV mode, but faked Error object in Prod, did you have some benchmark |
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)
|
Should we update the PropTypes docs based on this? They currently strongly suggest returning an |
|
I think we should add support for returning strings in a minor release. Then we can document that and remove support for returning errors in the next major version. |
It used to be slow whenever a type miss occurred because expensive
Errorobjects were being created. For example, withoneOfType([number, data]), passing a date would create anErrorobject innumbertypechecker 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.Thanks to @spicyj @sebmarkbage for ideas on how to fix this.