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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintenance: use fine-grained lodash-imports #21

Merged
merged 1 commit into from May 19, 2022
Merged

Conversation

fbartho
Copy link
Collaborator

@fbartho fbartho commented May 18, 2022

Lodash is a huge and chunky module. If we directly specify which methods we need from it, we can avoid a bit of the always-keep-it-up-to-date treadmill.

Further investigation could be done to see if we really even need the clone/isEqual methods from lodash, but at least with this PR change we put a soft fence around it to keep from panicking if lodash has security incidents. And to keep it from spreading 馃槈.

Not a big deal if you don't want to drop it, since the version of whatwg-url that is used by jsdom used by jest still needs it (even if newer versions don't), but I thought I'd provide a PR.

I'm aware that in lodash v5 they claim they will stop deploying the individual module packages, but that argument is based on "runtime package bundling" arguments, rather than on the maintenance concerns.

Lodash is a huge and chunky module. If we directly specify which methods we need from it, we can avoid a bit of the always-keep-it-up-to-date treadmill.

Further investigation could be done to see if we really even need the clone/isEqual methods from lodash, but at least with this PR change we put a soft fence around it to keep from panicking if lodash has security incidents.

Not a big deal if you don't want to drop it, since jest#core still needs it, but I thought I'd provide a PR
@blutorange
Copy link
Collaborator

blutorange commented May 19, 2022

That's definitely a good change. As we've seen in the recent years, more dependencies are not always better. isEqual(nodes[0].loc, node.loc) could probably be replaced with an explicit comparison between the properties of the location (which shouldn't be many), which would only leave the clone import. If we replace that with JSON.parse(JSON.stringify(...)), we could get rid of lodash completely.

@fbartho fbartho merged commit 3b72a60 into main May 19, 2022
@fbartho
Copy link
Collaborator Author

fbartho commented May 19, 2022

Okay, merging this PR for now, and will propose a future PR to strip the lodash imports entirely.

@fbartho fbartho deleted the fb/lodash-diet branch May 19, 2022 15:07
@fbartho fbartho mentioned this pull request May 19, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants