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

Get rid of lodash to achieve a smaller file size #22

Closed
wants to merge 1 commit into from
Closed

Get rid of lodash to achieve a smaller file size #22

wants to merge 1 commit into from

Conversation

bkonkle
Copy link

@bkonkle bkonkle commented Dec 3, 2015

When creating a browser bundle for my unirouter library, I noticed that redux-actions was pulling in nearly 10k from Lodash. I traced it to flux-standard-action, and the isPlainObject call. To eliminate 10k from flux-standard-action's file size, I wrote a standalone isPlainObject function.

This is mostly extracted from Lodash 4, as are the tests. I didn't include the IE6/7 host object check, but that's easy to add in if you'd like. Let me know what you think!

@JaKXz
Copy link
Contributor

JaKXz commented Jan 24, 2017

@bkonkle thanks for the contribution - I think in the time that this PR was made and now the lodash file sizes have improved, and I just tested v1.1.0 and found it to be < 10k [uncompressed and not uglified] including index.js and all dependencies. Let me know if you see anything wildly different from this.

Improvements like this are welcome though, so if you find something else, feel free to crack open another PR! :)

@JaKXz JaKXz closed this Jan 24, 2017
@dmytro-shchurov
Copy link

dmytro-shchurov commented Jan 24, 2017

It's very sad it is closed, because it's a pain to use redux-promise -> flux-standard-actions with SystemJS. Because when I agreed to configure lodash.isplainobject, it required yet more 4 modules: _basefor, isarguments, isarray, keysin. My config.js is getting bigger and bigger, and soon exceeds a size of lodash

@tomchentw
Copy link
Contributor

@JaKXz instead of replacing lodash modular dependencies, would you consider switching dependencies to lodash main module and import from its entries? For example,

Before

import isPlainObject from 'lodash.isplainobject';
import isString from 'lodash.isstring';
import isSymbol from 'lodash.issymbol';

After

import isPlainObject from 'lodash/isPlainObject';
import isString from 'lodash/isString';
import isSymbol from 'lodash/isSymbol';

Thus, for those users who's also using lodash in their user space, the bundler (rollup/webpack) could resolve these modules and create a smaller output. What do you think?

@tomchentw
Copy link
Contributor

One benefit of this approach is user can combine with babel-plugin-lodash and thus preventing importing the whole lodash into their bundle.

@JaKXz
Copy link
Contributor

JaKXz commented Feb 7, 2017

Hey @tomchentw I think that's a reasonable suggestion, want to make a PR? :)

I have a broader, naive question though, probably for @jdalton:

Would it be possible to have lodash itself depend on all the lodash.* modules and just export them under the one var _? There's probably a maintenance issue I'm not thinking of, but perhaps lerna can help with that?

@jdalton
Copy link

jdalton commented Feb 7, 2017

Hi @JaKXz!

The lodash.* packages are self-contained, zero dependency, packages. This means there'd likely be duplication. The preferred route would be to use something like babel-plugin-lodash. In v5 we're dropping the monolithic file altogether opting for folks to use the babel plugin if they want the monolithic-like syntax.

In the case of this PR though simply upgrading lodash.isplainobject to ^4.0.0 would get you reduced deps as it now has no deps. You can look to others like redux which cherry-pick Lodash's lodash/isPlainObject as well into their package build.

@JaKXz
Copy link
Contributor

JaKXz commented Feb 7, 2017

@jdalton thanks for the reply :)

I believe we were at ^4.0.0: https://github.com/acdlite/flux-standard-action/blob/01e8fc3cb9fa80fe49b816a9d65d1ad9c85c3180/package.json#L56

but yeah that's a good idea, I'll take a look at redux.

@jdalton
Copy link

jdalton commented Feb 7, 2017

If you go the redux route you can combo with lodash-webpack-plugin for an even smaller bundle.

@tomchentw
Copy link
Contributor

I've created the PR #51 to address it.

JaKXz pushed a commit that referenced this pull request Feb 9, 2017
* chore(package): add "babel-plugin-lodash" to devDependencies

* feat(package): add "lodash" to dependencies

* feat(index): switch to lodash modules under different entry points

* Ref #22

* feat(package): remove "lodash.isplainobject", "lodash.isstring", "lodash.issymbol" from dependencies

* Ref #22
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.

5 participants