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

refactor: switch to smaller dependencies to reduce bundle size #525

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/vue3-lottie/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"prettier": "npx prettier --write ."
},
"dependencies": {
"lodash-es": "^4.17.21",
"fast-deep-equal": "^3.1.3",
Copy link

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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.

"klona": "^2.0.6",
Copy link

Choose a reason for hiding this comment

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

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.

"lottie-web": "5.12.2"
},
"peerDependencies": {
Expand Down
3 changes: 2 additions & 1 deletion packages/vue3-lottie/src/vue3-lottie.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
nextTick,
} from 'vue'
import Lottie from 'lottie-web'
import { cloneDeep, isEqual } from 'lodash-es'
import isEqual from 'fast-deep-equal/es6';
import { klona as cloneDeep } from 'klona/json';

import type {
AnimationDirection,
Expand Down
1 change: 1 addition & 0 deletions packages/vue3-lottie/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"module": "ESNext",
"strict": true,
"declaration": true,
"allowSyntheticDefaultImports": true,
"lib": ["es2017", "ESNext", "DOM", "DOM.Iterable"],
"jsx": "preserve",
"outDir": "dist",
Expand Down
15 changes: 6 additions & 9 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.