Skip to content
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

Remove underscore/lodash dependency #121

Closed
pstoica opened this issue Jun 17, 2013 · 30 comments
Closed

Remove underscore/lodash dependency #121

pstoica opened this issue Jun 17, 2013 · 30 comments
Labels

Comments

@pstoica
Copy link

pstoica commented Jun 17, 2013

This might just be a personal preference, but I don't feel like it makes sense to require underscore/lodash for this library, especially when a lot of the functions already have Angular or jQuery alternatives. The fewer external libraries needed to get up and running, the better, and I just don't have a pressing need for those libraries in my apps.

I'd be glad to help rewrite the necessary bits if you're okay with moving away from this dependency.

@kwhitley
Copy link

I wondered the same thing, but to be fair, he's using more than a handful of underscore's functions...

@mgonto
Copy link
Owner

mgonto commented Jun 18, 2013

Hey,

I think it's a really fair and good question.

Some of the methods that are implemented in Underscore/Lodash, are indeed implemented in AngualrJS, but not all. AngularJS doesn't have all of the inmutable methods that Underscore does. AngualrJS has a forEach but doesn't have a map.

Also, you're saying that jQuery has some of this methods. It's true. But jQuery isn't a dependency of AngularJS nor of Restangular, so using jQuery methods would be just changing one dependency to another, as it's not really needed for AngularJS. It'll otherwise use the "minimal" version that comes with Angular.

So, the question is why not using AngularJS methods, or coding them here.

  1. I dont' want to reinvent the wheel. I always use map, omit, keys, values, extend, etc. All of this are not available on AngularJS and I'd use them here. If I didn't include Undersocre, I'd have to implement it my self. My implementation wouldn't actually rely on ECMA5/6 already implemented methods if available and also might not be as performant as the ones in Underscore / Lodash

  2. I want to favor inmutability always. I think that using Inmutability is the best way to code, as with no Side effect, it's much easier to code, and much easier to not get many bugs. Underscore makes it easy for me to do this.

  3. Most of Underscore / Lodash methods are in ECMA5, and almost all of them are in ECMA6. As soon as ECMA6 is out there, I'll be able to remove this dependency and just use the default ones. I think that using this methods is the right way to code JavaScript and it's how I'm used to coding. I've built the library how I code JS every day and how I know about it. I know I'm adding dependencies, but once you start using Underscore / Lodash, there's no getting back :). It's simply awesome!

I know there're some libraries out there like SugarJS which actually do some of the same stuff and including this 2 is really a pain. I know that and it sucks. But I thought this was the easiest, quicker, clearer, DRYer way of implementing Restangular.

I've actually talked to @jdalton to make sure that Underscore and Lodash share the same compatibility stuff for this to make this library compatible with any of them.

I hope this answers your questions.

In your cases, why don't you want / use underscore or lodash? Do you use other libraries or you just don't use them?

@mgonto
Copy link
Owner

mgonto commented Jun 18, 2013

Some libraries also include other libraries "inline", using the minified version. For example, take a look at https://github.com/flot/flot/blob/master/jquery.flot.resize.js which actually includes in there the dependency to a jQuery plugin inline.

I'd rather say OK, this is what I use, this is what this needs to work and be clear about this. And yes, as @kwhitley says I do use a lot of Underscore methods. And if that's the case, actually reimplementing them here so as not to include other library, wouldn't that be the same (or worse) than actually adding the dependency inline? To what point we want no dependencies? I know it's just a philosophical point of view.

@pstoica
Copy link
Author

pstoica commented Jun 18, 2013

Sorry, didn't mean to sound hostile!

I honestly didn't look at the source very thoroughly, and no, I definitely wouldn't want to depend on jQuery over lodash. I went ahead and added lodash to my project, and Restangular's been working really great so far. I guess at first glance I was concerned about needing one more dependency just to get up and running, but I'm sure it'll be a beneficial addition and it's a small library anyway. I missed using utilities like these in Ruby anyway.

Thanks for making Restangular! It's definitely better than ngResource and I hope they use this for further improvement.

@pstoica pstoica closed this as completed Jun 18, 2013
@mgonto
Copy link
Owner

mgonto commented Jun 18, 2013

Hey,

I didn't mean to be hostile either. Sorry if that's how it seemed :(.

Thanks for the help!!!

@wesleytodd
Copy link

Sorry to resurrect a dead issue, but I really think this library would be great if it didn't have the added dependancy. We are having some issues with the $resource implementation with angular 1.7 and so I am exploring other options. This looks like a great project, but adding underscore to our project is not going to happen.

From what it looks like you have only a few uses of underscore:

Method Number of Uses Angular Version
_.contains 2 No
_.isUndefined 7 Yes
_.defaults 4 Yes (Just like using extend)
_.isArray 2 Yes
_.each 7 Yes
_.extend 9 Yes
_.find 1 No
_.has 1 No
_.initial 1 No
_.last 1 No
_.isNull 2 No
_. isBoolean 1 No
_.keys 1 No
_.isEmpty 1 No
_.reduce 1 No
_.bind 48 Yes

So most of these that are not replicated in Angular are just a matter of doing a type check:

  • _.isNull: typeof val === null
  • _isBoolean: typeof val === 'boolean'
  • _.isEmpty: typeof val === 'undefined' || val == '' || val === null
  • etc...

The rest are just used once or twice and could be written more efficiently inline. With this information, does it seem feasible to remove the underscore dep?

@jdalton
Copy link

jdalton commented Sep 10, 2013

@wesleytodd

but adding underscore to our project

Why? If it's file size you could use a custom build of lodash.

So most of these that are not replicated in Angular are just a matter of doing a type check:

Not really. The _.defaults method is different than _.extend, _.bind is different as it more closely follows the ES5 spec, and even _.isFunction is different as it avoids issues that Angular doesn't address.

@mgonto
Copy link
Owner

mgonto commented Sep 10, 2013

@jdalton thanks for jumping in with the comments :). I agree that using either Lodash or Underscore is absolutely awesome and all projects should use it.

That said, @wesleytodd I'm not against having a version that doesn't have them and doesn't have Lodash or Underscore. If that's the case, you can fork this project and remove all Lodash and Underscore uses and replace them with Angular's one or implement some of your own functions for those and then I'd point to your repo is somebody wants a version that doesn't depend on another external library. Does that make sense?

Thanks!!

@wesleytodd
Copy link

@jdalton You are absolutely right that a custom build would be a great solution for any file-size considerations. Which is one of the great features of lodash. Implementation details aside, I don't think that all projects benefit from including en external lib. This especially applies to open source projects that are meant for re-use. The extra dependency is a barrier to entry regardless of it's file-size.

@mgonto Thanks for the response and what looks like great work on this project. I think we are going to go with a more custom tailored solution for our project. Probably one that implements an interface more closely related to the existing $resource which we are already using. It will also probably implement HAL helpers, since that is our API standard.

Thanks for your guys quick response, and good luck on your project. Sorry if I wasted your time.

@jdalton
Copy link

jdalton commented Sep 10, 2013

@wesleytodd

This especially applies to open source projects that are meant for re-use. The extra dependency is a barrier to entry regardless of it's file-size.

I disagree. Deps are trivial and allow devs to offload parts of their codebase to projects that are more specialized for the given task. Often, when devs try to avoid deps they end up recreating the wheel, except with less experience than the specialized libs it can easily lead to more bugs/edge-case issues.

@mgonto
Copy link
Owner

mgonto commented Sep 11, 2013

@mgonto Thanks for the response and what looks like great work on this project. I think we are going to go with a more custom tailored solution for our project. Probably one that implements an interface more closely related to the existing $resource which we are already using. It will also probably implement HAL helpers, since that is our API standard.

That's great. You can support some of the stuff of HAL with restangular using the selfLink feature :).

@Thinkscape
Copy link

Quick note: later versions of Angular added a lot of those functions to angular.*... maybe this can be revisited some time in the future.

@mgonto
Copy link
Owner

mgonto commented Dec 5, 2013

I'll take a look in the future then :)

@Thinkscape
Copy link

👍

@jbedard
Copy link

jbedard commented Dec 20, 2013

I recently started looking at this as well. I took a quick look at restangular and I really don't see any substantial use of _. Most of the use cases are just helper functions that are either a) in angular as well, b) have es5 equivalents, or c) can be nicely written using angular/es5. And of course d) there are a few cases where there is no helper in angular/es5. Personally I don't think that d) case is worth depending on _, but that's just me.

I can understand supporting non-es5 browsers still, although I sure wish more people would take the shim approach with libraries such as es5-shim instead of _/util libraries. This way es5+ projects don't even need an extra library, without any code changes (to restangular in this case).

But anyway, I tried replacing _ with angular/es3 methods. There were a few ugly spots (no Object.keys, Array reduce/map, _.isEmpty/pluck, ...), many of those could be improved with es5 but I thought I'd stick with es3 for now. I have not fully tested anything and I'm relying on unit tests to verify I didn't break anything, so this might mainly just be an example for now.

The branch is at https://github.com/jbedard/restangular/tree/remove_lodash

If you're interested it would be great if at least some of the direct es3/angular replacements of _ could be considered so that custom _ builds can be minimized more.

Also note that:

  • isFunction in angular/_ is the exact same (in angular 1.2.5 and lodash 2.4.1 at least)
  • _.defaults/extend are close enough and can be used for the same purposes (unless I'm missing something? but your tests all pass :)
  • and the best es5 spec bind would be a spec following shim, not a bind-like function in a util library (neither _ or angular follow the spec very well)

@jdalton
Copy link

jdalton commented Dec 20, 2013

and the best es5 spec bind would be a spec following shim, not a bind-like function in a util library (neither _ or angular follow the spec very well)

Using a shim for bind will give you all kinds of inconsistencies. I've seen it cause problems for code that assumes a functions will have specific .length. This is fine for native Function#bind, as it will produce bound functions with the same .length as the original, however it's a problem for shims as produce functions with a .length of 0. Lo-Dash's implementation follows spec as close as other shims and a lot closer than angular.bind, but avoids inconsistencies by not using native bind so environments with or without get the same result. To top it off Lo-Dash has optimizations to avoid unnecessary binding/wrapping in its methods too.

Lo-Dash methods are more feature rich than ES5 methods &, in many cases, faster too. Also, like jQuery, Lo-Dash ships with modern first, so you're not lugging around bug fixes for older browsers.

If you need older browser support Lo-Dash offers more consistency than es5shim and avoids inconsistencies with array iteration too:

I have not fully tested anything

Speaking of testing, Lo-Dash has 100% code coverage and automates testing with Sauce Labs, something you won't find for many shims 😄

Lo-Dash offers more consistency, features, & performance over ES5/Shims.

@jbedard
Copy link

jbedard commented Dec 21, 2013

Any non native bind-like function will have the .length problem (although if it's not a shim then it's not as important obviously...).

That's great that lo-dash has 100% code coverage and has more features, but this issue/request was about removing lo-dash since the small set of lodash methods used can easily be replaced. I mentioned es5/shims as an alternative to many lodash helpers since it would allow the library users to decide if they need the shim or not. I don't mean to say the code is better or has more test coverage, just that it has the advantage of being optional.

BTW, thanks for making lodash so customizable! I assume I'll be making a custom lodash build for now so that will really help!

I still hope (some of) my changes to restangular could be considered though. At the very least I don't think restangular users need another forEach method... (native, angular and jQuery is already 2 more then I want...).

@mgonto
Copy link
Owner

mgonto commented Dec 21, 2013

Hey,

I'll take a look at your branch and consider replacing some of the functions you mention. Can you point exactly to which and what you're replacing it with?

I'm hoping to remove actually bind directly and use prototypes for adding Restangular functionality to the models, but that refactor is taking longer than expected :).

I'll reopen and take a look.

Thanks!

@mgonto mgonto reopened this Dec 21, 2013
@jbedard
Copy link

jbedard commented Dec 21, 2013

If you look at the history of my branch each commit usually replaces 1 lodash method with es3/angular methods. I did the easier ones first (mainly direct replacements) so I suggest looking at the older commits first. If you look at my branch commits (https://github.com/jbedard/restangular/commits/remove_lodash/) you can see the list.

But for example, starting at the older commits:

2d4a6b1: replaces _.isUndefined with angular.isUndefined (which have identical implementations)
3f991b7: replaces _.isNull(foo) with null === foo (identical implementation)
f9e02bc: replaces _.contains(a,b) with the es3 a.indexOf(b) !== -1 (which lodash will indirectly do)
9d55514: replaces _.isArray with angular.isArray
fe9f503: replaces _.each with angular.forEach

The most recent few commits aren't as nice/direct replacements, but I'd think a few of these are worth removing the dependency. For example...

dbb05af: replacing _.values/omit with plain old for loops

@jdalton
Copy link

jdalton commented Dec 21, 2013

Any non native bind-like function will have the .length problem (although if it's not a shim then it's not as important obviously...).

Right, the issue is with shims devs will observe the native behavior first, write code for it, then when testing older environments they'll run into issues as the shim behaves differently. By behaving consistently in new and old environments devs will have already worked their code around its behavior.

That's great that lo-dash has 100% code coverage and has more features, but this issue/request was about removing lo-dash since the small set of lodash methods used can easily be replaced. I mentioned es5/shims as an alternative to many lodash helpers since it would allow the library users to decide if they need the shim or not. I don't mean to say the code is better or has more test coverage, just that it has the advantage of being optional.

I don't really understand the drive to remove Lo-Dash as a dependency. I think it's worth its weight. Restangular gets methods that are tested and optimized and devs can then still use and incorporate Lo-Dash methods into their code. Seems like all win. The whole point is Lo-Dash does these utility methods better than most so devs can avoid using less robust alternatives or hand rolling their own code.

Hand rolling lib alternative code more times than not introduces bugs/side effects that could have been avoided by using the battle tested lib. For example by pulling lodash out and using things like Array#indexOf instead of _.contains you're forcing devs to use a shim if they want compat and capping its perf. By using delete in your hand rolled alternative instead of _.omit you're de-optimizing the object causing potential perf issues in engines like V8/Chakra and missing opportunities like using native Object.keys, when available, instead of using a for-in+hasOwnProperty checks. By replacing _.isArray with angular.isArray you're losing benefits of using Array.isArray when available as Lo-Dash does use it while angular.isArray doesn't.

Lo-Dash puts utilities first (its primary focus) so using alternatives that don't seems like a step back for little benefit.

@jbedard
Copy link

jbedard commented Dec 21, 2013

I actually thought IE8 had indexOf, my mistake there :(. As you can see with the rest of the commits I was trying to only use es3 methods, otherwise a lot of those changes would be much nicer with es5.

And yes, you've pointed out all the issues in my last couple commits that don't have nice angular/es3 equivalents. I know those are ugly and I pointed that out already. I honestly think a couple ugly ones like that would be worth it, but I didn't expect everyone to agree with that.

I also noticed the angular.isArray doesn't use the es5 version. They should fix that! (Or, again, if they used a shim...).

Sure lodash puts utilities first, but maybe I already have all the utilities I need? In fact, I already have 3x the utilities I need :|. That is why I wouldn't mind avoiding a 4th set of utilities (even if it is better & faster then the first 3). So if it is easy to remove or reduce the number of utilities that a library depends on I think that is great for everyone!

@jdalton
Copy link

jdalton commented Dec 21, 2013

Sure lodash puts utilities first, but maybe I already have all the utilities I need? In fact, I already have 3x the utilities I need :|. That is why I wouldn't mind avoiding a 4th set of utilities (even if it is better & faster then the first 3).

You could always make a custom build or a dummy _ with the methods mapped to angular, jQuery, or your fallbacks. That's what's great about using a lib instead of the bare-bones lang, it allows wiggle room for things just like this.

So if it is easy to remove or reduce the number of utilities that a library depends on I think that is great for everyone!

Great for everyone by introducing bugs & compat issues?
You're just exchanging one issue, api overlap, for a host of others here.

@jbedard
Copy link

jbedard commented Dec 21, 2013

I assume I will end up doing the dummy _ with methods from angular/es5. Restangular using less _ methods will make that easier.

I don't think this is a specialized concern though. This applies to every restangular user who doesn't already depend on lodash, especially those who already have a utility library (or 3).

@jdalton
Copy link

jdalton commented Dec 21, 2013

I don't think this is a specialized concern though. This applies to every restangular user who doesn't already depend on lodash, especially those who already have a utility library (or 3).

Those who already have a different utility lib should really look in to making the switch to lodash anyways and those that don't already depend on lodash should give it a look. I'm sure there's something helpful in it for devs to write less and do more 😄

@mgonto
Copy link
Owner

mgonto commented Jan 29, 2014

This is not going to happen for now guys, sorry but I do use it for lots of stuff. Really sorry!

@mgonto mgonto closed this as completed Jan 29, 2014
@niemyjski
Copy link

+1

@niemyjski
Copy link

Will you be using lodash for 2.0?

@bernardolm
Copy link

I do not like the idea of having an implicit dependence on Loadsh in Restagnular too.

This brings me problems when I want to use other libraries as Lazy.js, which for some functionality have better performance. And if in my project I just need these features that has better performance, I need to be required to have the problems of conflicts? Why that? Why force the use of extra library is the objective of the main library is not the same dependency library? Open source is not freedom? Restangular is not open source?

PS: I use Restangular in almost all my projects with angularjs and HTTP requests and I love to do this to be easy with Restangular! Thanks @jdalton.

@jdalton
Copy link

jdalton commented Oct 14, 2015

lodash v3 supports lazy evaluation btw.

@evanjmg
Copy link

evanjmg commented Jan 7, 2017

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants