search

eiriklv / react-masonry-component

A React.js component for using @desandro's Masonry
MIT License
1.44k stars 145 forks source link

Use lodash/* ? #93

Closed mikelambert closed 6 years ago

mikelambert commented 7 years ago

While it may make sense to use lodash.* in isolation to minimize the code dependency sizes, in a large project where many other components may depend on lodash, it likely makes more sense to use lodash/* imports instead of lodash.* imports, to allow the various components to reuse their lodash dependencies.

Are you open to switching this project's dependencies?

mikelambert commented 7 years ago

In particular, react-masonry-components use of lodash.omit adds 36KB (or 12KB compressed) to my build, in a way that is not able to be de-duded against the main lodash in my build (that comes from my code as well as components I depend on)

afram commented 7 years ago

@mikelambert I'm open to it, but I think it's one of those situations where developers are going to be penalised if they use the alternative import.

Should people be using Webpack 2 (or other build tools that take advantage of tree shaking) then I think what you are saying makes sense.

Perhaps the thing to do is use the lodash/* style of imports...

mikelambert commented 7 years ago

I'm not talking about tree-shaking here (removing unnecessary code), etc. I agree that's a more advanced build-dependent feature, and may not apply to everyone. If you did import { omit } from 'lodash', then I agree it would require tree-shaking...but I am proposing import omit from lodash/omit, which should not require any tree shaking at all, and only pull in the necessary bits.

Given your response about tree-shaking, I'm not sure if my original proposal was clear, so I will explain in more detail, just in case it helps clarify more. I believe that every builder/packager (webpack et al) will detect that two modules depend on the same third module, and only include one instance of that third module. Since lodash has dozens of tiny modules under the hood, all of my dozens of lodash/* usages (in my code and modules I depend on) can be de-duped against each other, and do not bloat my library size more.

Unfortunately, lodash.omit pulls in its own copy of these internal mini-modules, and they cannot be de-duped against the lodash/* mini-modules. Which leads to needless duplication of 18KB/36KB.

I agree that if everyone else used lodash.*, there would be nothing to gain by using lodash/* (and in fact it might hurt!) But in my build I have dozens of different lodash/* imports, and only a couple lodash.* imports. So I think most module authors are designing for a case where they expect to be composed into a larger build, and want to play nicely with each other in that context.

afram commented 6 years ago

I've made this change and it is now live in v6.0.0

I'd be keen to hear get your feedback/update on how it goes.

Going to close the ticket now, but please reopen if needed.