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/Reduce lodash usage #41

Closed
SimonSchick opened this issue Oct 14, 2017 · 8 comments
Closed

Remove/Reduce lodash usage #41

SimonSchick opened this issue Oct 14, 2017 · 8 comments

Comments

@SimonSchick
Copy link

SimonSchick commented Oct 14, 2017

You are only using 2 of lodash's function, one can be replaced with a for in loop, the other can be replaced with https://www.npmjs.com/package/lodash.flattendeep.

@SimonSchick
Copy link
Author

Furthermore, since you are already dropped support for old node versions, you can also get rid of methods.

@mormahr
Copy link
Member

mormahr commented Oct 15, 2017

I know, unfortunately haven't had the time. I wanted to completely get rid of flattenDeep but then got distracted by other work. Maybe i'll do this as a intermediate solution. 🤔

@SimonSchick
Copy link
Author

I think dropping 2 deps for 1 micro would be enough for now.

@SimonSchick
Copy link
Author

Also if you wanna be a total bro adding TypeScript typings would be awesome!

@mormahr
Copy link
Member

mormahr commented Oct 16, 2017

Hah! As i‘m heavily relying on Flow for my own projects (i took over maintenance for this project, so it‘s not my „own“ codebase) i tried to add flowtypes to this aswell, but this code is mutating existing prototypes/classes that aren’t really typed to be modified. It should be possible to create a library definition, though.

As i’m not familiar with TypeScript i won’t create one for TypeScript but others are free to contribute one, but i’m not entirely sure what the TypeScript way to do is (include in npm module or contribute to DefinitelyTyped).

@mormahr
Copy link
Member

mormahr commented Oct 16, 2017

I just rechecked and methods is a dependency of express, so there is no benefit in removing it, as it will be installed anyway. It even has a drawback: Express uses methods to determine available methods, so if we use http.METHODS instead, they might mismatch.

mormahr added a commit that referenced this issue Oct 16, 2017
@mormahr
Copy link
Member

mormahr commented Oct 16, 2017

See v3.0.0-rc.1

@mormahr
Copy link
Member

mormahr commented Jan 19, 2018

This is now released and TypeScript definitions have been added as well (#47)

@mormahr mormahr closed this as completed Jan 19, 2018
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

No branches or pull requests

2 participants