fix caching of styles #33
Conversation
| var argsLen = args.length; | ||
| var str = argsLen !== 0 && String(arguments[0]); | ||
| if (argsLen > 1) { | ||
| for (var a = 0; a < argsLen; a++) { |
There was a problem hiding this comment.
L43 already sets str to args[0]. The index in this loop should start at var a = 1
There was a problem hiding this comment.
Woops, I had this originally, and a must have lost it in a clean up.
|
Fantastic pull-request! 🌟 |
| return applyStyle.apply(builder, arguments); | ||
| }; | ||
| builder._styles = _styles; | ||
| builder.__proto__ = proto; |
There was a problem hiding this comment.
__proto__ is an anti-pattern and deprecated. See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto
There was a problem hiding this comment.
Do the Object.create dance instead 💃
There was a problem hiding this comment.
Oh, just saw your issue comment.
- defines the getters onto a proto - the function returned has its __proto__ set to our proto fixes chalk#32
~21% increase with multiple arguments
|
I fixed the for the loop, and add a couple comments as to why these changes exist, for a better future! |
fix caching of styles, fixes #32
|
@seanmonstar 👍 thanks for both the fantastic pull request, and your explanation of the steps you took to fix the bug! |
|
@seanmonstar you're awesome!
|
If you choose to go with option 2 that I listed in #32, this is the PR.