Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
bkonkle wants to merge1 commit intoredux-utilities:masterfrombkonkle:master
Closed

Conversation

@bkonkle
Copy link

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 commentedJan 24, 2017
edited
Loading

@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] includingindex.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! :)

@JaKXzJaKXz closed thisJan 24, 2017
@dmytro-shchurov
Copy link

dmytro-shchurov commentedJan 24, 2017
edited
Loading

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 replacinglodash modular dependencies, would you consider switching dependencies tolodash main module and import from its entries? For example,

Before

importisPlainObjectfrom'lodash.isplainobject';importisStringfrom'lodash.isstring';importisSymbolfrom'lodash.issymbol';

After

importisPlainObjectfrom'lodash/isPlainObject';importisStringfrom'lodash/isString';importisSymbolfrom'lodash/isSymbol';

Thus, for those users who's also usinglodash 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 withbabel-plugin-lodash and thus preventing importing the wholelodash into their bundle.

@JaKXz
Copy link
Contributor

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 havelodash itself depend on all thelodash.* 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 commentedFeb 7, 2017
edited
Loading

Hi@JaKXz!

Thelodash.* packages are self-contained, zero dependency, packages. This means there'd likely be duplication. The preferred route would be to use something likebabel-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 upgradinglodash.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'slodash/isPlainObject as well into their package build.

JaKXz and tomchentw reacted with thumbs up emoji

@JaKXz
Copy link
Contributor

JaKXz commentedFeb 7, 2017
edited
Loading

@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

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

JaKXz reacted with thumbs up emoji

@tomchentwtomchentw mentioned this pull requestFeb 8, 2017
@tomchentw
Copy link
Contributor

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

JaKXz pushed a commit that referenced this pull requestFeb 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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bkonkle@JaKXz@dmytro-shchurov@tomchentw@jdalton

[8]ページ先頭

©2009-2025 Movatter.jp