Avoid lodash where possible #1500
Conversation
Lodash is quite big package https://packagephobia.com/result?p=lodash and often used where modern javascript feature do a good job. In this diff I replaced lodash with native functions or for/of loop. The only left case is lodash/cloneDeepWith which does not have clear alternative. I will try to replace it with something in another PR and remove lodash dependency.
|
cc @fb55 |
There was a problem hiding this comment.
Great PR, thanks for looking into this! Left some comments, there are some patterns that can be simplified. Also, it would be great if we could avoid using iterators (both with for .. of loops and with Array.from, as some users are still using older JS engines. Note that this isn't forever & we will upgrade eventually.
|
I relied on travis.yaml and thought only node lts are supported. Also what about arrow functions? They are used a lot instead of "bind". |
|
@fb55 I converted all changes to es5 |
|
Though I left Object.assign as there is no alternative in es5 |
There was a problem hiding this comment.
A lot of great stuff here! There are still a few too many forEach here for my taste, that should either be replaced with reduce calls, or good old for loops. Otherwise this should be good to go soon.
Also agreed that Object.assign seems alright to keep around. Users can easily polyfill it if it isn't available.
.eslintrc.json
Outdated
| @@ -6,14 +6,6 @@ | |||
| "extends": ["eslint:recommended", "prettier"], | |||
| "globals": {}, | |||
| "rules": { | |||
| "indent": [ | |||
There was a problem hiding this comment.
Can we leave this rule enabled? This PR has quite a few whitespace changes & I'd prefer to keep those separate.
There was a problem hiding this comment.
You have prettier enabled. And its formatting is different from eslint one. I needed passed eslint to test properly.
https://github.com/cheeriojs/cheerio/blob/v1.0.0/package.json#L59-L62
There was a problem hiding this comment.
I see. Pre-commit package is missing.
I've just added a |
|
@fb55 Done |
|
Awesome work, thank you so much! |
|
Hi @fb55. I don't see cloneNode available in cheerio nodes. Am I missing something? |
Lodash is quite big package https://packagephobia.com/result?p=lodash
and often used where modern javascript feature do a good job.
In this diff I replaced lodash with native functions or for/of loop.
The only left case is lodash/cloneDeepWith which does not have clear
alternative. I will try to replace it with something in another PR and
remove lodash dependency.