-
Notifications
You must be signed in to change notification settings - Fork 687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace lodash's each/map with ES5 native forEach/map #689
Conversation
d543f0c
to
f446c9b
Compare
Maybe worth noting that according to this, lodash's forEach is faster than the native implementation |
It's slightly more complicated than that: it will be faster on some browsers and similar in others because of Lodash's implementation and how browsers implement That being said, while it's good to keep in mind in case this ever becomes a bottleneck, I don't think we should worry about that. While we're at it, why not using Thanks for bringing that up though, I didn't know that before you posted it. |
Keep in mind we only use lodash on the server, so the nide is the only engine. 👍 for using native methods when possible. |
|
||
chan.users.push(user); | ||
}); | ||
data.users.forEach(u => chan.users.push(new User(u, network.prefixLookup))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map
should work fine here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haz test webhuks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another test!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review comment
Re: micro-change performance, on that note I watched a nice talk about how JSPerf-based conclusions are often not applicable :) https://www.youtube.com/watch?v=65-RbBwZQdU |
f446c9b
to
17a4a3d
Compare
17a4a3d
to
e905c13
Compare
Replace lodash's each/map with ES5 native forEach/map
And use some fat arrows because these are the good kind of fats.