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
Import per-function lodash modules #11789
Changes from all commits
ae5feee
3f65485
be7a4ad
040654c
1453bec
98edd64
e64348a
d0067a5
e50e41c
814a632
46d741b
4b5d099
539f2cd
d943cfa
35a0768
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,6 @@ | |
"dependencies": { | ||
"@babel/helper-function-name": "workspace:^7.10.4", | ||
"@babel/types": "workspace:^7.10.5", | ||
"lodash": "^4.17.19" | ||
"lodash.has": "^4.5.2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That said, the YDNLU native JavaScript replacement is relatively complex (9 lines of non-trivial code including regular expressions) and generates an error when key paths do not exist. On balance it may make sense to maintain readability for now, although that's just my two cents and I'd welcome feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep it, since we are using it with a deep path. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,6 @@ | |
}, | ||
"main": "lib/index.js", | ||
"dependencies": { | ||
"lodash": "^4.17.19" | ||
"lodash.pull": "^4.1.0" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be possible to replace
lodash/escapeRegExp
with a function provided by the MDN docs, although I don't know whether it'd make sense to replicate that per-babel-package or whether it should go into a utility/common package if so.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we instead consider replacing it with escape-string-regexp. That module is well tested, not being deprecated and gets 19 million weekly downloads (it's not below scrutiny).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind it requires major bump
https://github.com/sindresorhus/escape-string-regexp/blob/master/.travis.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry for missing that, I guess this suggestion would only apply to babel 8 then.