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

Remove lodash dependency to decrease global package size #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kraynel
Copy link

@kraynel kraynel commented Jan 6, 2020

Hi!

I am not sure if this repo is still active, but here is my contribution anyway.
I noticed that installing request + request-promise-native adds a whole 10Mb to my project.

The full Lodash is bundled and needed for only 4 basic functions (+1 for build).
I switch from lodash.isArray to Array.isArray, please tell me if this is a problem.

@kraynel kraynel force-pushed the remove-full-lodash-dependency branch from 6c854e6 to 3c1de62 Compare January 6, 2020 12:22
@coveralls
Copy link

coveralls commented Jan 6, 2020

Coverage Status

Coverage decreased (-0.003%) to 98.87% when pulling 38d3507 on kraynel:remove-full-lodash-dependency into 45c01b7 on request:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 100.0% when pulling 6c854e6 on kraynel:remove-full-lodash-dependency into 45c01b7 on request:master.

@kraynel
Copy link
Author

kraynel commented Jan 7, 2020

I've just seen #7, so I guess you can close that one if you did not change your opinion on the subject.

@2PacIsAlive
Copy link

+1, already discussed in #7 but this size reduction would still be beneficial for us :)

@kraynel kraynel changed the title Require only part of lodash to decrease global package size Remove lodash dependency to decrease global package size Jan 11, 2020
@kraynel kraynel force-pushed the remove-full-lodash-dependency branch from 770cb04 to 38d3507 Compare January 11, 2020 14:51
@kraynel
Copy link
Author

kraynel commented Jan 11, 2020

Ok I tried something new and extracted the lodash function into the lib. It has the advantage of adding no overhead and being self-contained, with the drawback of not profitting from any of the future lodash fixes.

As the copied code is quite short, I guess this could be acceptable.

@odinho
Copy link

odinho commented Feb 17, 2020

Yes please!

This is the only thing dragging in the full lodash anymore, and we could save space in our build artifacts.

Was going to write a similar PR, but this is already better.

@sseide
Copy link

sseide commented Jul 2, 2020

I created a patch too before seeing this and have thrown out all of the lodash functions used in production dependencies except isObjectLike() with one more generic typeOf() function. But is easily possible to inline this single function too.

Afterwards lodash.flatten is only used as devDependency...

Any thoughts as this patch here has not been merged by now?
https://github.com/sseide/promise-core/tree/reduce_lodash_dep

And hoping for lodash v5 is probably like waiting for Godot...

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.

None yet

5 participants