-
Notifications
You must be signed in to change notification settings - Fork 14
Do we really need _ or lodash? #134
Comments
This is a (related) good read: https://plus.google.com/u/1/114198465647271367011/posts/AowKp99k496 |
Not sure there is any real justification in that post. I am totally bought into the substack pattern and will always code my modules in that way. But I'm not fussed about refactoring out underscore because I trust underscore/lodash from a author, maintance, performance and major bug perspective. I don't want to do the duediligance on every module replacement for every function I use in underscore. Even if I do the duediligance, it would take some time to have the same confidence of these modules in production. When every utility function is a module it introduces more and more authors that we become reliant on, if one of them become lazy or lets a critical module fall into disrepair we are then forced to pick up it up and maintain it going forward. I accept this risk when there is no alternative, but do I really want to exacerbate this by forcing out utilitybelts like underscore out my projects for purist ideals when it does cause any problems. Modulatity is so so important, but there becomes a point when it is like what in OOD we call Object Oriented Masterbation http://robert.accettura.com/blog/2008/12/16/object-oriented-masturbation/ i give an example here https://speakerdeck.com/serby/building-for-clients-with-nodejs?slide=29 maybe it is a little contrived but you get the point. In ctrl we've striped out all features so very little of underscore it used, as bundles are added more functions will be required. It is a complete regression from my purist ways as a SOLID developer, underscore is exemply locical cohesion and at odds with the S, but I think there are practical considerations beyond SOLID when there is a pragmatic motive is to 'get shit done' and sometime being a purist is as more costly than making a few consessions. I suspect that in time we will be able reach the highest of standards, but for now I'm not sure the benefit is justified. |
+1 If we're willing to drop support for v0.6, we really should adopt https://github.com/joyent/node/blob/master/lib/util.js#L563-573 |
Many of the dependent modules that i control have already had that change Sent from a telephone On 14 Nov 2012, at 22:29, Erik Lundin [email protected] wrote: +1 If we're willing to drop support for v0.6, we really should adopt https://github.com/joyent/node/blob/master/lib/util.js#L563-573 — |
The justification is that we are depending a module that is 4279 lines of code (1MB npm package), of which we are using 11 lines (0.002%).
I completely agree with that notion, but I don't think it applies to this instance. No abstraction is happening. In fact less abstraction is happening. You require() a function that you intend to use, rather than a grab-bag (object) containing functions that you probably won't use.
This doesn't have to introduce any new code, we can just use the same function that's in underscore anyway. Plus, a smaller module is more easy to test and isolate issues. We can reuse the underscore/lodash source and tests.
We can't leave things in based on the premise that future bundles might need them, otherwise we'd include the whole of npm! Dependencies can be added as and when they are needed (and to each bundle's own package.json).
To echo what Paul is saying, I thought about using util.extend, but it is private API stuff (denoted with the '') and undocumented. It's not future proof. |
So the justification is for purism sake not any actual real world problem I in fact created a package to do just extend right from the start, see Using underscore meant I spent less time wanking about and more time being To be clear, I would like a no headache pure fix to this. Maybe ever adding Sent from a telephone On 14 Nov 2012, at 23:18, Ben Gourley [email protected] wrote: Not sure there is any real justification in that post The justification is that we are depending a module that is 4279 lines of but there becomes a point when it is like what in OOD we call Object I completely agree with that notion, but I don't think it applies to this Even if I do the duediligance, it would take some time to have the same This doesn't have to introduce any new code, we can just use the same In ctrl we've striped out all features so very little of underscore it We can't leave things in based on the premise that future bundles might If we're willing to drop support for v0.6, we really should adopt To echo what Paul is saying, I thought about using util.extend, but it is — |
Perhaps naively, I think controls biggest problem is currently that it isn't finished. If lodash is offering a means to speed up development my vote is to leave it in, for now. In the long run, I can see how it may be underutilised in the core and personally like the idea of removing it. I would suggest finishing the core and then look at refactoring it out after release. |
At this moment in time I am agreeing with @lukewilde on this one on leaving it in at the moment. We do not know what will be needed in newer iterations of the codebase, and by leaving Underscore is the most popular module, and I would like to think one which has a lot of eyes on the codebase. This means us and the community can trust the functionality of this library. Maintaing our own versions of these functions just so we do not have to depend on a monolithic collection of functions does sound nice, but in practise most likely will be much more of a ball-ach than adding a measly 1MB module. In all, I think we should stick with |
Dammit. My trackpad freaked out and I hit close and comment before I'd finished typing… If the consensus is that it should stay, then obviously it should stay. I just can't see the reason for keeping dependencies 'just in case'. It takes seconds to do I'm not trying to hate on underscore/lodash here, I think they do a great job. And I am pretty certain that we will keep it around for front-end stuff, as it fills a lot of useful functionality that's missing in browsers. However, the things that make it good for the browser render a bit pointless in Node – you know exactly the environment you're in and what native functions are available, yet a big portion of the library is defining In the end, I'm not particularly fussed if it stays. There are bigger fish to fry, and it doesn't have a tangible negative impact on the application (except some extra waiting around at install/deployment time). I think, though, that it would have been an oversight for us not to ratify its existence when such a tiny portion of it is being used. |
'except some extra waiting around at install/deployment time' If each of the 9 functions I listed was a module, the combined weight of the code, the package.json, the tests, readme and the fact that each would be over a new HTTP connection is likely to be equal if not greater than the size/speed of underscore. |
9 functions? We use 2? Anyway, 9 functions would not be a meg and npm does http reqs in parallel. |
That's only because everything is striped out of ctrl. There are ~9 in Also _.pluck uses _.map. _.min and _.max use _.each. _.shuffle uses If you can find drop-in replacements for at least: Should we also replace async because we only use 'parallel' in ctrl? On 16 November 2012 17:24, Ben Gourley [email protected] wrote:
[email protected] Direct: +44 2030 516115__Mobile: +44 7881 550999 *
|
I can see these ways in which it could be decoupled:
Not in the browser, because they all tend to rely on the ES5 polyfills, but in Node I think that would definitely make sense.
If the goal was to be 'pure', yes. But using 1/20 functions in a much smaller library is more acceptable (for me). |
Project wide search for
_.
returns 17 matches._.extend()
_.pluck()
I think it would be better to use an extend module (it probably exists, if not we can create it):
We could do the same for pluck, though as it's only used in on place it might make sense to just do an array map (which is what _.pluck does anyway). From the _ source:
_ makes sense on the frontend, as it fills a lot of missing functionality from old browsers, but I think it's an unnecessary grab bag for node code.
The text was updated successfully, but these errors were encountered: