search

request / promise-core

Core Promise support implementation for the simplified HTTP request client 'request'.
ISC License
20 stars 43 forks source link

Remove lodash dependency to decrease global package size #23

Open kraynel opened 4 years ago

kraynel commented 4 years ago

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.

coveralls commented 4 years ago

Coverage Status

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

coveralls commented 4 years ago

Coverage Status

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

kraynel commented 4 years ago

I've just seen https://github.com/request/promise-core/pull/7, so I guess you can close that one if you did not change your opinion on the subject.

2PacIsAlive commented 4 years ago

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

kraynel commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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...