-
Notifications
You must be signed in to change notification settings - Fork 305
Remove lodash dependencies #402
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Replace lodash.merge, lodash.castarray, and lodash.isplainobject with inline implementations to reduce bundle size and eliminate external dependencies. The merge function maintains full compatibility with lodash behavior, including recursive merging of nested objects within arrays. - Remove lodash.merge, lodash.castarray, lodash.isplainobject from dependencies - Add inline implementations in src/utils.js with identical behavior - Update src/index.js to import utilities from utils module - Maintain backward compatibility and all existing functionality Fixes bundle size concerns raised in community discussions about lodash dependencies.
df3d144
to
ff6a493
Compare
} | ||
|
||
return merge(target, ...sources) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code that uses merge
could probably be simplified to not need it at all tbh
} | ||
|
||
function castArray(value) { | ||
return Array.isArray(value) ? value : [value] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we simplified that code and dropped the entries -> fromEntries dance we could inline this too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments but don't actually need to simplify those things unless you feel like it 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this handles all the cases that lodash handled, but I also don't think we need to have support for all of that (old) JS stuff anyway.
This should be enough for our use cases. I would even argue that this is already too complex for what we need. But it's much simpler / smaller already.
@RobinMalfait @thecrypticace Yeah I was going for least changes possible to avoid any breakage. Can definitely be cleaned up but we'd want to invest time in the CSS-only branch if we do that instead. Let me add changelog and land it 👍 |
@RobinMalfait @thecrypticace thanks for whoever of you fixed the Vercel thing :D |
* Remove lodash dependencies Replace lodash.merge, lodash.castarray, and lodash.isplainobject with inline implementations to reduce bundle size and eliminate external dependencies. The merge function maintains full compatibility with lodash behavior, including recursive merging of nested objects within arrays. - Remove lodash.merge, lodash.castarray, lodash.isplainobject from dependencies - Add inline implementations in src/utils.js with identical behavior - Update src/index.js to import utilities from utils module - Maintain backward compatibility and all existing functionality Fixes bundle size concerns raised in community discussions about lodash dependencies. * Add changelog
Summary
Removes lodash dependencies (
lodash.merge
,lodash.castarray
,lodash.isplainobject
) and replaces them with inline implementations to reduce bundle size and eliminate external dependencies.This addresses concerns raised in the community about lodash dependencies bloating bundle sizes (referenced from this discussion).