refactor: remove the lodash dependency
#808
Conversation
This significantly reduces the package install size.
| @@ -164,7 +165,7 @@ export default iterateJsdoc(({ | |||
|
|
|||
| const abbreviationsRegex = abbreviations.length ? | |||
| new RegExp('\\b' + abbreviations.map((abbreviation) => { | |||
| return _.escapeRegExp(abbreviation.replace(/\.$/ug, '') + '.'); | |||
| return escapeStringRegexp(abbreviation.replace(/\.$/ug, '') + '.'); | |||
There was a problem hiding this comment.
Maybe look into if that extra replacing is necessary.
|
Thanks! Looks good. Hope to have energy to review in the next few days... Re: scope creep, I don't mind a few small things--better to get things right, imo. |
Co-authored-by: Jayden Seric <me@jaydenseric.com>
|
I pushed a couple minor tweaks and also fixed testing and linting. Could you handle the removal of the remaining |
|
Actually, I think I've handled that last file all right--maybe just review all of my changes to see if you feel all right to move forward |
| if (context.options[0] && option in context.options[0] && | ||
| // Todo: boolean shouldn't be returning property, but tests currently require | ||
| (typeof context.options[0][option] === 'boolean' || | ||
| key in context.options[0][option]) | ||
| ) { | ||
| return context.options[0][option][key]; | ||
| } | ||
|
|
||
| return context.options[0][option][key]; | ||
| return baseObject.properties[key].default; |
There was a problem hiding this comment.
I'm not aware of the context for these changes, but trust it makes sense.
There was a problem hiding this comment.
Yeah, although it is likely a bug (some issues reported which may suggest it), just using the boolean value of context.options[0][option] when found to be a boolean didn't work either, so would need investigation and a separate PR.
What this fix does is just restore the lodash behavior of avoiding erring when trying in checking on context.options[0][option] (as it will do when it is a boolean).
brettz9
force-pushed
the
jaydenseric/remove-lodash-dependency
branch
from
e9011c6
to
aecf6cd
Compare
|
I'm excited to see this change has landed. Is there any chance that a new release will be published soon? I'm asking because we're waiting for this change to make it into a release over in Node.js core. |
|
🎉 This PR is included in version 37.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@Trott : As per the automated release, this is now released in v37.1.0. And thanks for the heads up on its use on the Node.js codebase! Hope it works well for y'all... |
This significantly reduces the package install size, as
lodashhas a 1.35 MB install size:https://packagephobia.com/result?p=lodash@4.17.21
While the current total install size for
eslint-plugin-jsdocis 4.75 MB:https://packagephobia.com/result?p=eslint-plugin-jsdoc@37.0.3
Also, presumably the native methods (
Array.filter,Array.flatMap,Array.some, etc.) perform better thanlodashfunctions.Some of the more complicated helpers have been refactored to minimal dependencies, most or which were able to be dev dependencies to correctly ensure a minimal production installation for consumers.