-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Merge configs using lodash merge #101
Conversation
9e3ea00
to
20359d8
Compare
Codecov Report
@@ Coverage Diff @@
## master #101 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 96 99 +3
Branches 16 16
=====================================
+ Hits 96 99 +3
Continue to review full report at Codecov.
|
20359d8
to
24a422e
Compare
24a422e
to
f73186e
Compare
Thank you so much for the PR! Sorry but could we not use lodash? It adds to the bundle size quite a bit... |
It imports the lodash submodule for that function only, is that still large? |
Unfortunately yes... we had same problem at Vuex ORM Core. Lodash uses many common functions, so even if you import only specific features, it still loads unnecessary codes. I really hate this too, but it's how JS libraries have to do things I guess... |
I guess the code will have to be ripped out of the lodash library.. |
Yeah that's what I did when implementing |
Yea, lodash is a simple fix but adds considerable weight and a significant performance impact. The clone example that @kiaking is referring to may point you in the right direction – it's quite verbose because performance is crucial for Vuex ORM – so it could easily be half the code. Note, the signature for type-safe merging is considerably more complex than cloning. If you wish to preserve types. Sometimes I abhor TS. |
Closing due to inactivity. I don't believe there's been much steer on this topic to warrant this PR progressing. |
Fixes #97