Skip to content

fix: don't include all of lodash#1847

Open
RobinClowers wants to merge 1 commit intoairbnb:masterfrom
RobinClowers:fix-lodash-dependency
Open

fix: don't include all of lodash#1847
RobinClowers wants to merge 1 commit intoairbnb:masterfrom
RobinClowers:fix-lodash-dependency

Conversation

@RobinClowers
Copy link
Copy Markdown

💥 Breaking Changes

  • None

🚀 Enhancements

  • Visx packages now only depend on specific lodash function packages, rather than all of lodash. This affects visx-responsive, visx-text, and visx-xychart.

📝 Documentation

  • None

🐛 Bug Fix

  • None

🏠 Internal

  • None

Copy link
Copy Markdown
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the improvement here @RobinClowers ! 🙌 had a quick comment on the non-root lock files.

Comment thread packages/visx-shape/yarn.lock Outdated
@RobinClowers RobinClowers force-pushed the fix-lodash-dependency branch from 8608590 to 95ac4ff Compare June 6, 2024 20:15
@RobinClowers
Copy link
Copy Markdown
Author

@williaster can we get this merged please? Let me know if there is anything else outstanding.

@VIKTORVAV99
Copy link
Copy Markdown

@RobinClowers have you considered replacing lodash entirely with https://es-toolkit.slash.page/?

It should be faster and smaller while being more or less a drop in replacement for lodash functions.

@RobinClowers
Copy link
Copy Markdown
Author

@VIKTORVAV99 I hadn't heard of es toolkit, looks like an awesome project!

Since many of these projects only depend on one or two functions, I'm not sure the savings would be significant (compared to all of lodash). Also, there might be subtle differences in the behavior between the libraries. That said, it sounds like a good improvement, but I'm not personally motivated to make the change. Especially since this low risk PR hasn't been merged, I worry that it would be wasted effort.

@RobinClowers
Copy link
Copy Markdown
Author

Oh, one other concern, I didn't know what the browser support policy is for visx, it might not be safe to depend on modern is for this project. For my project it would be, but again I haven't looked closely at the browser support guarantees visx provides.

@raidenmiro
Copy link
Copy Markdown

any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants