tommoor / emojione-picker

A react emoji picker for use with emojione
http://tommoor.github.io/emojione-picker
MIT License
261 stars 61 forks source link

[Proposal] change underscore to lodash #22

Closed sugarshin closed 8 years ago

sugarshin commented 8 years ago

Thank you merged prev PR !

lodash has underscore compatible, it will also be small fill size after build by require for each function. This, I think that problem is solved in the case of dependent on the large library, such as the underscore.

tleunen commented 8 years ago

Can we please remove all lodash functions except throttle? We can use all native functions instead.

And remove compact. Might need a bit of refactoring.

sugarshin commented 8 years ago

pls check it out.

tleunen commented 8 years ago

In general, I think it looks ok, but please remove all .bind() in render and I'll review once again after :)

sugarshin commented 8 years ago

How's this?

Object.keys(this.props.modifiers).forEach((function(_this){
  return function(index){
    var hex = _this.props.modifiers[index];
    ...
  };
})(this));
tleunen commented 8 years ago

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach

More like this:

Object.keys(this.props.modifiers).forEach(function(index) {
    var hex = this.props.modifiers[index];
}, this);
// or
var modifiers = this.props.modifiers;
Object.keys(modifiers).forEach(function(index) {
    var hex = modifiers[index];
});
sugarshin commented 8 years ago

How about?

tleunen commented 8 years ago

looks good

sugarshin commented 8 years ago

@tommoor pls review :)

tommoor commented 8 years ago

Hey both, just had a quick skim and this looks good - I'm going to do my best to test and merge with a new release in next 24hours, bear with me :)

tommoor commented 8 years ago

Thanks for your work here! I implemented this in 457f5aad7a6a3fcfed7d8194da96a448b010e2f5 as the extra code for native methods is not worth the loss of readability in my mind.