NerdWalletOSS / apollo-cache-policies

An extension of the Apollo 3 cache with support for advanced cache policies.
Apache License 2.0
152 stars 22 forks source link

replace lodash with lodash-es #27

Closed raysuelzer closed 2 years ago

raysuelzer commented 3 years ago

First let me say that this is a great library you have created, I am going to try it out with our Angular app which we are upgrading to use Apollo 3. But, there are two issues relating to the bundle size that I hope can be resolved.

The first, which is in this PR, is to replace the usage of import _ from "loadash" with importing the specific functions needed from the [lodash-es package].(https://www.npmjs.com/package/lodash-es). The current import from the "lodash" library results in the entire lodash library being included with your package, adding a significant amount of overhead.

The second, I will open as a new issue and has to do with this library being built using commonjs modules instead of es2015+

raysuelzer commented 3 years ago

Prefixing the lodash functions with a _ on the import is purely a personal preference as a developer used to using _. If you want to import them with renaming, I am happy to make that change.

abdul-hamid-achik commented 3 years ago

great pr dude i hope this gets merged

raysuelzer commented 2 years ago

great pr dude i hope this gets merged

It seems they aren't reviewing or merging outside PRs at this point? This dramatically reduces the bundle size...

FoodProduct commented 2 years ago

👋 Hi Ray, thanks for opening this PR. I'm a bit hesitant on this because I don't have full understanding of the implications of using lodash-es.

However, we have not had an issue with bundle sizing with this package (or lodash in general) due to usage of babel-plugin-lodash. Is this something that you would consider pursuing?

To be clear, I am not necessarily opposed to this change; I just want to explore and understand our options here 🙂.

mogelbrod commented 2 years ago

@FoodProduct Unfortunately babel-plugin-lodash only works when using babel, making other bundlers import the full library. I'd love to use this library as it's the only option I've found to actually make using apollo3-cache-persist viable, but the current increase in bundle size is less than ideal 🙁 Bundlephobia reports that lodash makes up 80% of the bundle size as it stands today.

Using lodash-es or explicitly importing each lodash function to be used can be done manually or automated using something described in this article: https://webuild.envato.com/blog/automating-the-migration-of-lodash-to-lodash-es-in-a-large-codebase-with-jscodeshift/

abdul-hamid-achik commented 2 years ago

guys i believe it is also posible that you in your webpack configuration(if you have one else other bundles may also provide the same feature), can specifically resolve lodash into lodash-es,

raysuelzer commented 2 years ago

Unfortunately babel-plugin-lodash only works when using babel, making other bundlers import the full library

Yes, this seems to be the case. I try not to get too involved in the Angular bundler and build process as most of the changes and overrides I make end up causing problems down the road.

There might be a way to do this other than using lodash-es, but since it's maintained and built by the core lodash team, it seemed like the easiest way to get the bundle size down and speed up compile time.

From their website:

Module Formats
Lodash is available in a variety of builds & module formats.

lodash & per method packages
lodash-es, babel-plugin-lodash, & lodash-webpack-plugin
lodash/fp
lodash-amd
danReynolds commented 2 years ago

I made a PR that replaces lodash with lodash-es here: https://github.com/NerdWalletOSS/apollo-cache-policies/pull/42. Broke it out from this one mostly because of the drift and merge conflicts. Thanks @raysuelzer for first identifying this and feel free to try it out in that PR folks. If it reduces your bundle size let us know here or on the PR and we can merge it in.