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

We should quite possibly just use lodash-node #2

Closed
HenrikJoreteg opened this issue Sep 2, 2014 · 20 comments
Closed

We should quite possibly just use lodash-node #2

HenrikJoreteg opened this issue Sep 2, 2014 · 20 comments

Comments

@HenrikJoreteg
Copy link
Member

suggested by @legastero

https://www.npmjs.org/package/lodash-node

@HenrikJoreteg
Copy link
Member Author

Not sure if there's any issues with using it in the browser? The name makes me wonder if it's meant to be node specific, or just that it plays nice with browserify.

@jdalton
Copy link

jdalton commented Sep 2, 2014

It should work fine with browserify.
I had to tweak my system's ulimit but that was easy enough.

The lodash-node bundle includes builds so you can choose to support compat, modern, or the underscore build.

When we bump our packages to 3.0 the primary lodash package will have the lodash-node/modern build included so you'll be able to do require('lodash/array/compact') right from the lodash package and the other builds will be have to their own packages, require('lodash-underscore/array/compact'). We'll update lodash-node, lodash-amd, & lodash-es6 to 3.0 as well.

@latentflip
Copy link
Contributor

@jdalton with 3.0, if a submodule has require("lodash/array/compact") and the parent application just require("lodash"), will the lodash/array/compact references dedupe since they'll now point to the same file?

I tried testing it, but it sounds like 3.0.0-pre isn't there yet as require('lodash/array/compact') wasn't working installing from lodash/lodash on github?

@jdalton
Copy link

jdalton commented Sep 2, 2014

with 3.0, if a submodule has require("lodash/array/compact") and the parent application just require("lodash"), will the lodash/array/compact references dedupe since they'll now point to the same file?

Yes, dedupe would work (move the package higher up the folder structure) based on the package+version. However, Browserify would bundle it as 2 different modules because using require('lodash') will still load a monolithic build that is not composed of individual module references. This is to support _.runInContext, lazy evaluation, and ease up on load time. The individual modules are included to allow devs to go method by method if they want. This is based on the assumption that if you're using the individual modules you'll probably not be using the monolithic/full require('lodash') invocation.

I tried testing it, but it sounds like 3.0.0-pre isn't there yet as require('lodash/array/compact') wasn't working installing from lodash/lodash on github?

I'll be making the repo structure changes this week.

@phated
Copy link

phated commented Sep 2, 2014

I believe @jdalton is correct that npm dedupe would reduce size on disk, but node and browserify would load them as 2 separate modules because they are different files.

@lukekarrys
Copy link

I just tested something like this:

myModule.js

var extend = require("lodash-node/modern/objects/assign");
var bind = require("lodash-node/modern/functions/bind");
var keys = require("lodash-node/modern/objects/keys");

// A made up module that uses extend, bind and keys
module.exports = function () { /* ... */ };

app.js

var _ = require('lodash-node');
var myModule = require('./myModule.js');

// Uses _ and myModule

then...
browserify app.js > app.bundle.js

In app.bundle.js I only get one reference to lodash's extend, bind and keys functions.

@jdalton Are you saying that with v3, lodash will do this by default? And if all references to lodash-node were replaced with lodash in the example above, the result would be the same?

@HenrikJoreteg @latentflip If so, that sounds like a pretty good solution to me once v3 is released. If not, is it good enough to tell devs to use lodash-node?

@jdalton
Copy link

jdalton commented Sep 2, 2014

jdalton Are you saying that with v3, lodash will do this by default? And if all references to lodash-node were replaced with lodash in the example above, the result would be the same?

The paths would be different but ya the same general thing would apply as long as you aren't doing require('lodash') and stick with require('lodash/object/assign'). We'll still update the lodash-node package and friends for 3.0 too.

You all could use lodash-node now and when v3 is release transition to lodash if that's something you're interested in.

@lukekarrys
Copy link

So I got very curious about what happens when modules and applications use different util libraries and the effect on the overall bundle size after browserify-ing. I was writing up another comment with the output of some code, and well... https://github.com/lukekarrys/util-lib-compare#output

Basically, what I found is that using lodash-node for modules doesn't have as a big of an effect on bundle size as I thought it would.

@jdalton
Copy link

jdalton commented Sep 2, 2014

So awesome!! I noticed that initially the lodash-node is the smallest but when both contain lodash-node it's considerably larger. However, others where they share the same package more closely resemble the base size of the package. For example I would expect lodash-node--lodash-node to be around 2.35 kB min instead of 15.72 kB min.

Can you explain that a bit more.

@lukekarrys
Copy link

@jdalton I noticed that too, but I'm not 100% sure why. Investigating that a bit more.

@lukekarrys
Copy link

@jdalton The module that uses lodash-node is the smallest because it directly requires assign, bind and keys, so those are the only methods that end up in the bundle.

But to simulate what would happen when an application uses lodash-node I made it require the whole thing, so every method ends up in the browserified bundle. If you check out that bundle though, you'll see that browserify does dedupe properly since only one reference to assign ends up in the bundle.

@jdalton
Copy link

jdalton commented Sep 3, 2014

Ah thanks for digging into that. If they are using lodash-node I don't think kitchen-sinking lodash-node would be common (as the point is to pick the bits you need & using 100% of the methods is rare). It would be nice if the dummy app used a subset (as most would) and then compare the numbers again, understanding that as more methods are used, and the less intersection between them, the larger the size would become.

I also get that you may be trying to see worse case scenario or simulate what the result would be if lodash referenced all modules but it'd be great to see the intersecting case for lodash-node, where I think it will shine.

@lukekarrys
Copy link

Not to get too much further down a rabbit hole with intersecting scenarios, I think using lodash-node for all the ampersand modules is a good choice.

If all modules used lodash-node then any combination of browserified modules wouldn't have any utility code duplication, and the individual modules would have the smallest bundle size. It also doesn't hurt the app size too much if a developer does require('lodash') or require('underscore') in their app. We could also recommend using lodash-node individual pieces (if they are into byte counting), in which case no code would end up duplicated.

@jdalton
Copy link

jdalton commented Dec 18, 2014

Ok, so I watched Henrik's Backbone conf video to see the issues raised against lodash and I wish they were raised beforehand so we could have mentioned that they're being addressed and we could have worked with Ampersand. It's a bummer to have put so much potentially unnecessary effort into a fork.

I've noticed that while many of the amp modules are copied directly from Underscore there's some issues that have slipped in. Some were copied from the edge version of Underscore which means their implementations weren't completely ironed out yet. Also, some of the amp specific changes introduced performance regressions as well as consistency/operational bugs. It's a bummer because amp doesn't offer much besides modules (something others have tackled and are continuing to refine) and further splinters utils with slower, incomplete, inconsistent alternatives. It would have been rad if the effort taken to fork would have been put into working with Underscore, lodash, or some of the other util libs.

@HenrikJoreteg
Copy link
Member Author

Perhaps so. I think goals were somewhat different and would have required changing your publishing strategy and versioning them independently as well as extending your scope to also include DOM-related functionality. There's no way Underscore would (or should have) gone along with that. Arguably we should have checked with you folks working on lodash on this but it seemed unlikely you'd want to make those rather drastic changes and I didn't see the harm in going this route.

Also the changes you all made introduced performance regressions as well as consistency/operational bugs.

how so?

Also, I wanted something like this to exist for the sake of us having a clear place and mechanism for adding new things to it as we needed them. It's obviously a bit of an idealistic dream, but the goal is for this to not be a huge on-going effort for us, but rather a place for us to categorize, test and document "done" utility code. I guess the thing is, does it matter? I don't see the harm in both existing.

@HenrikJoreteg
Copy link
Member Author

I guess also, if you obviate the need for amp, then... great! :)

@jdalton
Copy link

jdalton commented Dec 18, 2014

Perhaps so. I think goals were somewhat different and would have required changing your publishing strategy and versioning them independently as well as extending your scope to also include DOM-related functionality.

We've extended our scope in the past with the inclusion of string methods and lazy evaluation. I also don't think that would have prevented building on top of existing projects and supplementing for things like amp DOM utils. We're all into modules after all.

Arguably we should have checked with you folks working on lodash on this but it seemed unlikely you'd want to make those rather drastic changes and I didn't see the harm in going this route.

You totally should have 😸. We're pretty open to working with projects. That's why we're one of the most customizable and accommodating projects out there offering custom-builds, grunt plugins, individual modules and bundles for AMD, CommonJS, Node, ES6, & npm.

Also the changes you all made introduced performance regressions as well as consistency/operational bugs.

how so?

For example your amp-index-of module splits for native, unlike Underscore, introducing inconsistencies with sparse arrays and performance regressions. There's other issues in amp-keys and some of your amp-is-xyz modules too.

It's obviously a bit of an idealistic dream, but the goal is for this to not be a huge on-going effort for us, but rather a place for us to categorize, test and document "done" utility code

I had no idea when I started that 3 years later things wouldn't be "done". It goes on and on.

I guess the thing is, does it matter? I don't see the harm in both existing.

It would have been nice to avoid further splintering utils for something as trivial as modules/shallow-deps. Esp. since you'll likely continue to run into issues like those I mentioned earlier, issues others have tackled and are committed to tackling in the future.

I guess also, if you obviate the need for amp, then... great! :)

I wish you would have reached out :(

@fyockm
Copy link
Member

fyockm commented Dec 19, 2014

As a (not-so casual) observer, it still seems that amp is attempting to reinvent the wheel. I'm not completely convinced that another forking of underscore is necessary. To my understanding, there is really ONE major motivation behind this project:

  • modular distribution instead of a single wholesale library

Many of the other points made here are already handled by lodash. It does semver, has shared searchable docs, shared tests, etc... Is there anything else glaring that I missed?

@jdalton has already filed at least a dozen issues in this repo, which poke numerous holes in the '1.0 and done' theory. I just don't buy into the "solved problem" argument - no software is ever perfect.

What would be required to get lodash to distribute as separate tiny modules? It sure sounds like the lodash crew is more than willing to work with ampersand. Personally, I think we should push further down this path instead of introducing the fragmentation of &yet another competing library into the utils ecosystem. Breaking the utils into separate modules actually seems like a logical step from lodash-node.

@fyockm
Copy link
Member

fyockm commented Jan 8, 2015

While I may not have chosen the same route, I've come to understand why the Ampersand crew went down this path. I think this post explains it pretty well. Yes, it's very similar to post.md mentioned previously, but somehow makes the points a little clearer.

When you get right down to it, not "wanting to track lodash" was enough reason for this library to exist. Only time will tell how well Optimizing for “done” works out, but I give the Ampersand team some credit in making the bold choice. Hopefully many (if not most) of these modules will never require a major bump. I'm sure there will be changes under the hood, but the API should stay the same.

And...at this point, the ~4 month investment in amp is already done- at least for those utilities required by AmpersandJS. The build/test system is in place. The separate modules have been published. What remains is just the effort to update the ampersand-* modules to use amp over underscore.

@jdalton
Copy link

jdalton commented Feb 6, 2015

With lodash v3 out now I wanted to give an update on some of the lodash issues mentioned in the Modularizing Underscore.js post.

In v3 the dep graphs for modularized packages have been significantly reduced.

lodash.bind@3.0.0
├── lodash._baseslice@3.0.1
├─┬ lodash._createwrapper@3.0.1
│ ├── lodash._arraycopy@3.0.0
│ └── lodash._basecreate@3.0.0
└── lodash._replaceholders@3.0.0

Minor nit, the post links to lodash.bind which incorrectly suggests its dep graph is still large.

What's interesting is now the post could be tweaked to reference Amp itself:

But… it’s not perfect either. For example, below is the dependency tree for amp-compact:

amp-compact@1.0.0
└─┬ amp-filter@1.0.1
  ├─┬ amp-each@1.0.1
  │ ├── amp-create-callback@1.0.1
  │ └─┬ amp-keys@1.0.1
  │   ├── amp-has@1.0.1
  │   ├─┬ amp-index-of@1.1.0
  │   │ └── amp-is-number@1.0.1
  │   └── amp-is-object@1.0.1
  └─┬ amp-iteratee@1.0.1
    ├── amp-create-callback@1.0.1
    ├── amp-is-function@1.0.1
    ├── amp-is-object@1.0.1
    ├─┬ amp-matches@1.0.1
    │ └─┬ amp-pairs@1.0.1
    │   └─┬ amp-keys@1.0.1
    │     ├── amp-has@1.0.1
    │     └─┬ amp-index-of@1.1.0
    │       └── amp-is-number@1.0.1
    └── amp-property@1.0.2

It just seems a bit excessive for something simple.

Take the similar package lodash.compact:

lodash.compact@3.0.0
└── (empty)

Amp is largely ported from Underscore which is not designed to be modular. So they're getting unnecessary dependencies and less than stellar performance too.

There’s also lodash-node, which you then would require something in a nested path like: require('lodash/underscore/bind'). Which is pretty close, because now when we browserify it all, we’ll end up only including the code it uses. But those paths are a bit unsightly and hard to remember, in my opinion.

lodash v3 now has modules available in its primary package:

var clone = require('lodash/lang/clone')

Module paths are easy to remember because they map directly to their documented categories.

But there’s still some more subtlety there that’s a bit annoying. Specifically, that the Lo-Dash and Underscore codebases will march on. Which means we might be at 2.4.1 if we used Lo-Dash right now, for example. So we would have to pick a version range to march along with.

Devs can use ^3.0.0 for lodash, similar to using ^1.0.0 for Amp.

But when 3.x.x comes out, we’ll have to update dependencies in order to get proper de-duping. I don’t want to track Lo-Dash either, if all I want is a utility method, in most cases those seem like they should very rarely need any updating at all, right?

In lodash v3 modularized packages are updated independently of the stable version (from 3.0 up to around the current stable version). Some are 3.0.0, others 3.0.1, and others 3.1.0. Bumps are handled automatically by lodash-cli which makes managing 150+ modules a breeze and helps avoid oversights.

Take for example the lodash.noop method. First, I’m not convinced an empty function deserves to be its own module, but that aside, to me it feels odd that a module like this should ever have reached a SemVer version of 2.4.1.

In v3 several smaller functions like noop and identity are inlined in much the same way that Amp does so there's no need to worry about it. If one was to depend on it though they could use the * version range.

So what about this concept of “done” code? Is there such a thing?

It's complicated. Amp appears to have v2.0 work lined-up and has to walk a precarious path to keep from bumping while following semver. With Amp porting Underscore much of the history of fixes, implementation rationale, & direction is lost. Even in this short time since Amp's release, Underscore has optimized and fixed bugs that are missing in Amp. Keeping up with fixes, features, & optimizations is time consuming so the gap will likely widen.

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

No branches or pull requests

7 participants