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

chore: replace lodash with lodash.isequalwith #634

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wojtekmaj
Copy link
Contributor

What:

Continues effort from #593; further improves on #592

Why:

Since we're only using one method from the entire lodash package, we can dramatically reduce @testing-library/jest-dom's install footprint (by roughly 1.3 MB) by replacing lodash (https://packagephobia.com/result?p=lodash) with lodash.isequalwith (https://packagephobia.com/result?p=lodash.isequalwith).

How:

Checklist:

  • Documentation N/A
  • Tests N/A
  • Updated Type Definitions N/A
  • Ready to be merged

Since we're only using one method from the entire lodash package, we can dramatically reduce @testing-library/jest-dom's install footprint (by roughly 1.3 MB) by replacing lodash (https://packagephobia.com/result?p=lodash) with lodash.isequalwith (https://packagephobia.com/result?p=lodash.isequalwith).
@chyzwar
Copy link

chyzwar commented Sep 26, 2024

https://lodash.com/per-method-packages

These per method packages are deprecated. Most projects already have lodash installed via transient dependencies anyway.

@wojtekmaj
Copy link
Contributor Author

This is a vicious circle. lodash is being installed everywhere because it's already installed everywhere.

@JReinhold
Copy link

Yes someone has to start this, we can't all go around pointing fingers at the other packages saying "we won't optimize because the other's aren't".

The per-function modules are indeed deprecated, but I think it's the best we can do right now with lodash.

@chyzwar
Copy link

chyzwar commented Oct 2, 2024

The problem with per function modules is that they bloat the runtime of the program.
For example, this is source code for module in question
https://www.npmjs.com/package/lodash.isequalwith?activeTab=code
and this is when imported from lodash via modular import
https://github.com/lodash/lodash/blob/main/src/isEqualWith.ts

If we look how lodash is used across this organization:
https://github.com/search?q=org%3Atesting-library%20lodash&type=code

There are multiple lodash packages installed, both full version of lodash and per module.
Some of per module packages were not updated in 8 years and might be outdated.
https://www.npmjs.com/package/lodash.groupby?activeTab=readme

I would prefer that my dependencies of my projects have the small number of well vetted transient dependencies.
Install size is secondary, because savings of 80kB is nothing compared to esbuild/swc/bione/prisma 50MB+ packages sizes. Solution for large node_module is caching, not million of smaller packages.

If we want to optimize, we should use lodash/ everywhere. This would make runtime cost small and be compatible with future lodash v5.

Saying all that, runtime cost would be both influenced by how often module is required (lodash/isEqualWith have higher require cost). If lodash function is loaded once during test run vs loaded on every test file.

@wojtekmaj
Copy link
Contributor Author

Then maybe we should take a different turn and migrate to es-toolkit across all the repos?

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.

3 participants