Replace lodash.throttle with es-toolkit's throttle#1534
Conversation
|
Hi @dayongkr! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Hey! Thanks for your contribution. This would have too little benefits / impact for us to replace the well tested and highly reliable and secure lodash. |
|
Thank you for your thoughtful response. I completely understand your reasoning about stability and avoiding unnecessary churn. However, I'd like to respectfully present a few additional considerations that might change the cost-benefit analysis: 1. Bundle impact for usersEven with just 1 function, the size difference is significant: lodash: ~1.45 kB That's a 66% reduction affecting all projects using metro. 2. Zero-risk migrationThe migration is literally a drop-in replacement
3. Maintenance realityWhile lodash's throttle is stable, the library itself hasn't seen meaningful updates in 2+ years. es-toolkit is actively maintained and already adopted by major projects like Storybook and Recharts for these exact reasons. I understand if this still feels like unnecessary change, but given the minimal effort required and the collective benefit to your users, would you be open to reconsidering? Thanks for maintaining such a useful bundler! 🙏 |
Metro is a Node.js application, so install size would be the only relevant size consideration (though not a particularly important one). Besides that, we do tend to be cautious about accepting PRs from folks promoting their own packages, especially when they don't mention their affiliation. |
|
Thank you for the clear feedback and explaining the reasoning! ❤️ |
Summary
This pull request replaces the
lodash.throttledependency withes-toolkit/compat/throttleacross the Metro codebase. The motivation for this change is to address several key areas:es-toolkit'sthrottleis significantly more lightweight (115 lines vs. 440 lines forlodash.throttle). This is achieved by removing unnecessary self-implemented helper functions (likeisObjectandtoNumber) and simplifying complex internal state management present in thelodashversion. This directly leads to a smaller overall bundle.lodash.throttlehas not been updated in the past four years, indicating a lack of active maintenance. In contrast,es-toolkitis actively maintained, ensuring ongoing support, bug fixes, and performance optimizations.es-toolkit/compat/throttlehas been rigorously tested, passing alllodash.throttletests to ensure 100% compatibility in terms of interface and behavior. It has also been successfully adopted by other prominent libraries like Storybook and Recharts, demonstrating its stability and reliability in real-world scenarios.This migration allows us to leverage a more modern, performant, and actively maintained throttling utility without introducing any breaking changes or significant migration effort.
Changelog: [Performance] Replace
lodash.throttlewithes-toolkit/compat/throttlefor reduced bundle size and improved maintainability.Test plan
To verify the successful replacement and continued functionality, the following step was taken:
yarn test(or equivalent command to run all Metro unit/integration tests).throttleimplementation maintains the expected behavior and does not introduce regressions.