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

Importing lodash methods directly in order to reduce bundle sizes #269

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KonkretneKosteczki
Copy link

@KonkretneKosteczki KonkretneKosteczki commented Mar 29, 2023

Addressing issue #266 It's a pretty small change, so should not be difficult to review it

  1. Changed misc arrayHelpers and translate-from-firestore lodash imports
  2. Changed FireClient database provider lodash imports
  3. Updated package.json and yarn.lock to reflect the dependency changes

@LorenzoCruccu
Copy link

@benwinding this could be a nice improvement to the actual bundle size of the project!

@benwinding
Copy link
Owner

Just as I suspected, this seems to have 0 effect on the bundle size, which is already tiny at 10kb

image

I think the bundler already helps with this, due to tree-shaking... Unless anyone can prove otherwise?

Maintaining dependencies for each helper function of lodash seems a bit insane to me...

@LorenzoCruccu
Copy link

unfortunatly, i don't know how to analyze the bundle of this project (I will glad if you tell me how can i visualize it like webpack-bundle-analyzer), but as this commit of react-admin -> marmelab/react-admin#9385 , cherry picking lodash is prefered due to dependencies resolution. In this way, you are 100% sure that you will inject only the lodash modules that are actually utilized.

As regards of maintaning each lodash dependency in the package.json, I agree with you that is not sustainable and importing just lodash is fine due treeshaking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants