refactor: switch to smaller dependencies to reduce bundle size #525
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Refactoring
PR Summary: This pull request focuses on optimizing the bundle size of the vue3-lottie package by replacing lodash-es with fast-deep-equal and klona. These changes aim to maintain functionality while significantly reducing the library's footprint, as demonstrated by the provided bundle size comparisons before and after the refactor.
Decision: Comment
📝 Type: 'Refactoring' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Ensure comprehensive testing around the new dependencies to cover all scenarios previously handled by
lodash-es. This includes unit and integration tests that specifically address deep equality checks and deep cloning functionalities. - Consider adding performance benchmarks or updating existing ones to evaluate the impact of the new dependencies on the library's performance. This will help in identifying any potential bottlenecks or performance regressions introduced by the change.
- Enhance documentation to guide users through the transition, especially those migrating from older versions. Highlighting the rationale behind the switch, any notable differences in behavior, and steps for migration could greatly assist users in adapting to the changes.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| @@ -13,7 +13,8 @@ | |||
| "prettier": "npx prettier --write ." | |||
| }, | |||
| "dependencies": { | |||
| "lodash-es": "^4.17.21", | |||
| "fast-deep-equal": "^3.1.3", | |||
There was a problem hiding this comment.
suggestion (llm): Given the switch from lodash-es to fast-deep-equal and klona, it's crucial to ensure that the existing unit tests cover scenarios where deep equality checks and deep cloning are performed. If such tests are not present, I recommend adding them to verify that the new dependencies behave as expected in all use cases previously covered by lodash-es.
| @@ -13,7 +13,8 @@ | |||
| "prettier": "npx prettier --write ." | |||
| }, | |||
| "dependencies": { | |||
| "lodash-es": "^4.17.21", | |||
| "fast-deep-equal": "^3.1.3", | |||
| "klona": "^2.0.6", | |||
There was a problem hiding this comment.
suggestion (llm): Considering the replacement of lodash-es's cloneDeep with klona, it's important to validate the cloning functionality, especially in edge cases such as circular references, or cloning complex objects like functions, Dates, RegExps, etc. If tests covering these scenarios are missing, adding them would ensure klona meets the requirements.
| @@ -13,7 +13,8 @@ | |||
| "prettier": "npx prettier --write ." | |||
| }, | |||
| "dependencies": { | |||
| "lodash-es": "^4.17.21", | |||
| "fast-deep-equal": "^3.1.3", | |||
There was a problem hiding this comment.
suggestion (llm): Please ensure that the integration tests also reflect the change in dependencies. It's important to verify that the integration points where isEqual and cloneDeep were used still work as expected with fast-deep-equal and klona. This might involve updating mock data or test scenarios to cover new edge cases introduced by the dependency change.
| @@ -13,7 +13,8 @@ | |||
| "prettier": "npx prettier --write ." | |||
| }, | |||
| "dependencies": { | |||
| "lodash-es": "^4.17.21", | |||
| "fast-deep-equal": "^3.1.3", | |||
There was a problem hiding this comment.
suggestion (llm): It's commendable that the bundle size has been significantly reduced by switching dependencies. However, it's equally important to ensure that performance benchmarks, if any, are updated or added to reflect the impact of these changes. Performance tests can help in understanding if the new dependencies introduce any latency or execution bottlenecks compared to lodash-es.
| @@ -13,7 +13,8 @@ | |||
| "prettier": "npx prettier --write ." | |||
| }, | |||
| "dependencies": { | |||
| "lodash-es": "^4.17.21", | |||
| "fast-deep-equal": "^3.1.3", | |||
There was a problem hiding this comment.
suggestion (llm): The switch to fast-deep-equal and klona is a significant change that could affect the functionality of the library. It would be beneficial to add a section in the documentation or README explaining this change for users migrating from older versions. This documentation should ideally include any steps users might need to take or any notable differences in behavior.
|
Thank you for the PR! Will take a look |
* fix: ensure `loadLottie` is called after the component mounted (#527) * fix: ensure `loadLottie` invoke after `lottieAnimationContainer` is mounted * refactor: remove unnecessary `nextTick` * perf: educe the unnecessary use of `ref` (#526) Co-authored-by: Sanjay Soundarajan <[email protected]> * refactor: switch to smaller dependencies to reduce bundle size (#525) * 🐛 fix: performance improvements and suspense bug fixes * 🔖 chore: prep for releasing v3.3 * 🔖 chore: prep for releasing v3.3 beta --------- Co-authored-by: Alex Liu <[email protected]>
fast-deep-equalandklonaserve as alternatives tolodash-es'sisEqualandcloneDeep, respectively. They offer more lightweight options that do not fall short in performance compared tolodash-es.Here are the differences in bundle sizes before and after:
Before
After
All examples tested in the
playgroundhave been working normally. I am very open to understanding if there are any other considerations.