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

Use lodash utils instead of including them in the package #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use lodash utils instead of including them in the package #47

wants to merge 1 commit into from

Conversation

greena13
Copy link

I know the issue of package size has previously been raised and the solution was to include a local utils in the package itself (with tests) - however this increases the code that must be read, understood and maintained as part of the ongoing development of the project.

There is a happy middle ground, however. It's possible to include individual functions offered by the lodash library like so:

Installation:

npm install lodash.compact --save

Usage:

import compact from 'lodash.compact';

This has a number of benefits:

  • It doesn't bloat the size of the package (adds < 1kB to the final bundle size)
  • May actually help to reduce the package size as node will be able to de-dup dependencies and share them with other libraries that use the same approach
  • It delegates these functions a trusted third party library that does not have to be maintained or tested

Let me know what you think. :)

@w33ble
Copy link
Contributor

w33ble commented Nov 3, 2017

This was proposed on #28, more or less. That PR used a babel plugin to do this, but the idea is the same. As pointed out in a comment there...

As a side note, what's funny is once you get a big enough application we basically have to manually revert this. Once you hit about 15 of the lodash individual pieces, you'll want to instruct webpack to force sub-modules to use the main lodash bundle rather than all the split ones, otherwise you end up with a bigger overall bundle because lodash split out ones have extra overhead.

The problem is that webpack can't de-dupe the build, not on its own, since these are all separate packages. You can go in an manually force it to with aliases or something like that, which can be tricky for large projects. By default the build now includes a bunch of duplicate code, some from the core lodash module, and some from these individual modules, and all of the overhead "glue code" for the individual modules too.

I'd love a real solution to this problem personally. A local utils file isn't it, neither is using the individual lodash modules. Maybe things will get better when they move everything over to esm? Right now, everyone that is bundling npm code for browsers has this issue, and not just with lodash. 😞

@greena13
Copy link
Author

greena13 commented Nov 7, 2017

Thanks for the additional insight, w33ble. I hadn't considered that webpack built the modules into the release bundle before publication, making it impossible for npm to de-dupe the lodash dependencies.

15 is a useful metric to keep in mind for when the size of the individual packages exceeds that of the whole lodash module.

I wonder if there are any useful blog posts or discussions that really run the numbers on this and enumerate the advantages and disadvantages of each approach.

@w33ble
Copy link
Contributor

w33ble commented Nov 7, 2017

I hadn't considered that webpack built the modules into the release bundle before publication

It doesn't, at least not module by module.

What I meant was that import { compact } from 'lodash' and import compact from 'lodash.compact' is importing two different modules. The code may be the same, but webpack doesn't know that.

If this was just stand-alone code, or code that would run in node, it wouldn't really matter. But once you bundle this in your application for use in the browser, it matters, because the size of the bundle affects how it runs. If react-shortcuts is using lodash.compact and some other module (or even the parent project) is just using lodash, now that compact code is in the bundle twice, along with any of lodash.compact's dependencies (this is a bad example, since there are none, but hopefully you get the idea).

You could argue that you're not actually worse off than the utils file that's not part of this package, since that code is also technically duplicate code. The only real advantage is its fixed size, you don't get all these "accidental" imports as lodash modules resolve, it's just one file that's a little over 1k uncompressed, and some 400 bytes minified. Like I said, I don't think it's a real solution to this problem, but at least you know what you're getting. ¯_(ツ)_/¯

@greena13
Copy link
Author

Thanks for taking the time to clarify. I did indeed misunderstand you the first time.

What about import compact from 'lodash/compact' as an alternative solution? I am not sure if lodash is set up to be consumed in this manner, but I have seen other packages (no specific names come to me right now) that allow being imported like this to "only include the code you are actually going to use". From what I understand, using a sub-directory as an entry point, only pulls in that part of the module into your bundle.

This would not avoid duplicate code if the user also imports lodash.compact into their project, but it would (I think) prevent duplicate code if they import the entire lodash or lodash/compact- which I think may be the more likely scenario.

This may be a moot point (as I said, I haven't checked if lodash can be consumed in this manner) but I am trying to get my head straight on the implications each way of including packages - so take none of the above as gospel, just untested knowledge.

@w33ble
Copy link
Contributor

w33ble commented Nov 13, 2017

That's a good question, and I don't know the answer. I think the code in the lodash module is still written as commonJS, which means that it still can't do tree shaking, so lodash/compact still imports all of lodash. But I haven't really been keeping up, so I could be wrong. There is another library, lodash-es, which exports all of the modules as es6 modules, and tree shaking would work there, so it would correctly combine code across your entire project and drop code not being used. But again, now you have multiple "lodashes" and you end up duplicating lodash code in your final build assets.

My understanding is that the next major version of lodash will address all of this, I believe by using es6 modules everywhere and using @std/esm to transparently support use in node. Again though, I haven't really been keeping up, so perhaps I'm misunderstanding the planned path forward.

@greena13
Copy link
Author

Ah, interesting. So is tree shaking only possible with es6 modules?

@w33ble
Copy link
Contributor

w33ble commented Nov 14, 2017

My understanding is yes, since it's the only module definition that is static. That is, import can't be called at runtime, so you can fully resolve the dependency graph before code is executed.

CommonJS and AMD can both have runtime requires so you can't tree shake that code, since the dependency graph can change as the application executes.

@greena13
Copy link
Author

That makes sense. Thank you so much for the time you have spent over several days clarifying things for me. It has been very informative.

I believe I have one final question: what environments actually support es6 modules and how does this interact with webpack and babel's configuration? I am never really sure whether it's babel or webpack that is injecting the polyfill for the module implementation when bundling for a web client - I believe CommonJS is supported natively by node.js and webpack wraps AMD modules so they will work in that environment.

Particularly, I want to know are there circumstances where even if a module is written as several smaller es6 modules and I compile it into a project using webpack and target web or nodejs, that tree shaking will still not work as intended and the whole module will be bundled in?

@w33ble
Copy link
Contributor

w33ble commented Dec 1, 2017

Sorry, I got pretty behind on emails. It looks like all recent major browsers now support es6 modules. Node 8 supports them behind a flag, and you can shim support into older versions using @std/esm.

Webpack uses the spec to do tree shaking, and I believe both tools can be used to convert module syntax from one form to another, but I could be wrong on that one. I'm no build tool expert by any means.

If your project is the one doing the "compiling" from es6 modules to some other format to target node and the web, then you get tree shaking, even if you're doing it against modules from npm. So if you can bundle up node modules yourself, you can trim down your "vendor" bundle in addition to your project bundle.

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

Successfully merging this pull request may close these issues.

2 participants