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 individual lodash modules #9

Closed
wants to merge 1 commit into from
Closed

Conversation

wraithgar
Copy link
Contributor

Was doing an audit of one of my apps this week and noticed this module was bringing in ALL of lodash with it, that's > 200k of code I don't need, personally.

Not sure if there was a specific reason or bringing it in but this PR takes it back to the single-repo-per-method we were doing w/ lodash 3.

@wraithgar
Copy link
Contributor Author

Build is failing on the push due to a failed call to localtunnel.me, which if I recall correctly had been phased out of the other ampersand modules for this reason.

@cdaringe
Copy link
Member

Gar your bundler shouldn't bundle all of lodash the way it was. In fact, the bundle should be smaller the way it was vs what you're reverting to, per JDD

@wraithgar
Copy link
Contributor Author

Hmm ok that's odd. I'm using https://www.npmjs.com/package/disc to analyze the bundle straight from browserify using:

browserify --full-paths -t [ jadeify --doctype html ] client/app.js | discify --open

I'll see if there's any more documentation before pushing back on this, thanks

@wraithgar wraithgar closed this Aug 23, 2016
@wraithgar wraithgar deleted the specific_lodash branch August 23, 2016 15:34
@cdaringe
Copy link
Member

i used browserify to gen the bundles i mention over here

@wraithgar
Copy link
Contributor Author

Yeah thanks for the follow up, after looking at the built file on disk and the reports from the disc app, disc is overreporting on size, including the entire module even if it doesn't end up in the bundle. False positive, thanks.

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