-
Notifications
You must be signed in to change notification settings - Fork 208
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
Remove (or Replace) lodash dependency to reduce bundle size? #1870
Comments
Great points! Do you think you would be able to create a PR with these replacements? |
Sure, I'd volunteer a PR. I think having a minimum ES version to target (preferably larger than 2015) would be the main thing I need to get going. |
Lodash was recently intentionally introduced as a replacement for most of the utility functions. At the time I saw a 15-20% increase in bundle size which I considered an acceptable tradeoff as |
In regards to the compilation target. That turned out a lot bigger than initially expected and is currently shelved until I have more time to invest in it. |
Do we need to switch to ESM to support ES2015? I don't think so? @joliss what lodash functions in use could be replaced by native 2015 methods? That would help understand the size of the change. It might be that only one or two utility methods would need to be re-introduced. @bd82 do you have a minimum ES language target? 2015 is 7 years ago, surprisingly! |
Whatever works on the oldest still supported nodejs version |
I think the switch to lodash is actually really useful groundwork for this change, as now I can just grep for lodash and change everything systematically.
I wasn't looking to change the compile target, but rather just to bump the advertised compatible ES version.
Ok, Node 14 (the oldest supported LTS) seems to broadly support ES2020. (If there's anything in ES2020 that isn't supported, the CI on Node 14 should catch it.) All the greenfield browsers seem to support ES2020 as well. So if you're happy with ES2020, I'll use that as the target and prepare a PR. @mattbishop wrote:
I think most all of them can be replaced by a single native function call or a one-liner. So "one or two utility methods" is what I'd expect as well. |
What is the value proposition in that? this would be "officially" a breaking change... Hmm, which built-in functions do you need? what versions were they added? For such a PR to be accepted the new code (including tests) would have to be minimal and so would the overall complexity. |
* refactor: replace lodash with native ES2015 functions fixes #1870 * chore: reformat and update pnpm-lock.yaml Co-authored-by: Shahar Soel <[email protected]>
@bd82 seems like @joliss's PR was merged into https://github.com/Chevrotain/chevrotain/tree/remove_lodash, but not released. We're trying to switch MermaidJS' parser to Langium, which uses chevrotain. Bundle size is a big concern for us and would really appreciate it if @joliss' changes make it to a release. Perhaps you were waiting for NodeJS 14 EOL (April 2023)? |
Hello @sidharthv96 I am a-lot less active on Chevrotain recently... |
@bd82 thanks for creating such a wonderful tool and also for the prompt response. I've created a PR #1942 which you can review. I tried out almost all available grammar tools and none checked all the boxes like Chevrotain did. I genuinely understand shifting priorities and that we don't have any obligations for maintaining previous work. Moreover, things like CJS vs ESM and export maps are very frustrating things to do, outside our core product (I've personally dealt with it in Mermaid). But as a potential user of chevrotain, we were wondering what the redundancy plans are for chevrotain, do you have other trusted maintainers who could help with the maintenance? If not, could we discuss something for this please? |
@sidharthv96 you are welcome 😄
msujew from langium has write permissions to this repo and helps mainly by answering questions on the discussions tab and maintaining the chevrotain-allstar plugin used in langium. I would definitively not mind having additional maintainers, but I am not sure how to achieve this...
I have a little more free time recently and will try to push some of these items... |
All-right @sidharthv96 @joliss I have status update: Removing
|
Replacing lodash with RemdaRemda is:
So it seems to match all the requirements and may reduce bundle size by ~15% (I hope...) while keeping The one blocker is that it requires ES6 (or polyfills). |
In #1952 there is a POC to replace lodash with Perhaps the best approach would be to re-implement these utilities inside chevrotain
Normally I would not consider it worth-while just to save 20-30Kb of bundle size. |
The following steps reduced the
These will be released in version 11.0.0 as they are breaking changes. |
I know this is closed, but still persists as an issue folks would like to solve. What about replacing lodash functions individually rather than a large all-in-one PR? For instance, replace |
Hello @mattbishop For me this is past the point of diminishing returns. |
I wonder what you thought about the idea of using native JavaScript functions instead of lodash?
In my minified bundle, lodash takes up 41 KB in addition to Chevrotain's 136 KB, so it seems that this would probably be the biggest opportunity for reducing bundle size (#1697).
JavaScript has gained a lot of the utility functions provided by lodash in the past 10 years, so this change might be a lot more doable than it was a few years ago.
It might also be worth bumping the minimum JavaScript version to something like ES2020 in the same go, so that we get access to most of the new functions. ES2020 is currently compatible with ~95% of global web users' browsers, down from 98% for ES2015.
The text was updated successfully, but these errors were encountered: