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

Lodash dependency - required? #428

Closed
Daveawb opened this issue Apr 21, 2020 · 13 comments
Closed

Lodash dependency - required? #428

Daveawb opened this issue Apr 21, 2020 · 13 comments

Comments

@Daveawb
Copy link

Daveawb commented Apr 21, 2020

Is this a question? Yes and... no? Go figure.

I noticed that I had the full blown version of lodash in my dependencies once I had installed this package (which is excellent btw). I started to look through the code base to see if it really was required and I have only found these usages:

context-matcher.ts
_.isFunction
_.isString

config-factory.ts
_.assign
_.isString
_.isPlainObject

handlers.ts
_.camelCase
_.get
_.isFunction

http-proxy-middleware.ts
_.assign
_.isFunction

path-rewriter.ts
_.isFunction
_.forEach
_.isEmpty
_.isPlainObject
_.isUndefined
_.isNull
_.isEqual
_.isPlainObject
_.forIn

router.ts
_.isPlainObject
_.isFunction
_.forIn

Each one of these dependencies can be achieved easily with ES6 without the need for lodash, would you be willing to receive a PR to remove this dependancy?

Most of my projects are built by CI many times / day and anything I can do to reduce the time needed to bootstrap each run is always appeciated.

Is this a bug report?

No

Is this a feature request?

Sort of. This request is to remove the lodash dependancy from the project.

Motivation:

  • Faster installs. Better performance.

Setup

Not applicable

@Daveawb Daveawb changed the title Lodash depencency - required? Lodash dependancy - required? Apr 21, 2020
@Daveawb Daveawb changed the title Lodash dependancy - required? Lodash dependency - required? Apr 21, 2020
@chimurai
Copy link
Owner

Ah, that's a trace of the node 0.10 era. Before we had transpilers like Babel or TypeScript.
Only recently converted to TypeScript to start modernising the codebase.
It's on the todo list to remove it.
Just didn't had the chance to work on it.

@Daveawb
Copy link
Author

Daveawb commented Apr 21, 2020

@chimurai happy to put in a PR to remove this.

The only ones I can see that'll need maybe more than a simple change are:
_.camelCase
_.get * depending on how it's used

@chimurai
Copy link
Owner

@Daveawb , would be really nice to have a helping hand.

Not always pretty to face the facts...

source: bundlephobia.com
image

@Daveawb
Copy link
Author

Daveawb commented May 1, 2020

Says it all doesn't it 💯

Sure, I've just had to do this for a number of my own repos so happy to take a look this weekend.

@Daveawb
Copy link
Author

Daveawb commented May 12, 2020

Made some headway over the weekend. A few tests still failing, will get to them this week at some point.

@chimurai I've also added the strict: true flag in tsconfig.json and adding types. If you'd prefer I didn't do this in this PR let me know.

@chimurai
Copy link
Owner

Nice. Think it is better to have strict: true in a separate PR.

@lattam
Copy link

lattam commented May 18, 2020

You can also install only the functions you really use. That should reduce the size significantly.

npm i lodash.isfunction

etc.

@Daveawb
Copy link
Author

Daveawb commented May 18, 2020

@lattam I did consider this but really they still bring in a host of lodash functions that just aren't needed and still inflate the package size considerably. Also, they're removing the individual function imports in v5, see Jdaltons reply in lodash/lodash#3838 (comment)

@s100
Copy link

s100 commented Jun 25, 2020

The priority of this task may be about to increase due to this prototype pollution vulnerability in lodash, which lodash's maintainers seem not to be acting on.

@Daveawb
Copy link
Author

Daveawb commented Jun 26, 2020

Yep ok, I'll crack on at the weekend, apologies, been a bit inactive on this for a while.

@Daveawb
Copy link
Author

Daveawb commented Jun 30, 2020

@chimurai nearly there now. I have 4 failing e2e tests that I need to sort out.

@chimurai
Copy link
Owner

let me know if you need any help

@Daveawb
Copy link
Author

Daveawb commented Jul 7, 2020

@chimurai nearly there now. I have 4 failing e2e tests that I need to sort out.

Actaully a second set of eyes on this might be useful. Debugging the websocket test is a pain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants