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

Update events.md #316

Merged
merged 1 commit into from
Oct 3, 2020
Merged

Conversation

gabriel-curtino
Copy link
Contributor

Type of PR (feature, enhancement, bug fix, etc.)

Documentation

Description

Importing ... from 'lodash-es' .... is including all library (at least on RoR 6+) then you need to specify the module to avoid this behavior.

Why should this be added

The full import takes ~1.3Mb while debounce module only ~23Kb.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing

Importing ... from 'lodash-es' .... is including all library (at least on RoR 6+) then you need to specify the module to avoid this behavior. The full import takes ~1.3Mb while debounce module only ~23Kb.
@leastbad
Copy link
Contributor

Thanks for pointing this out. I freely admit that I'm no expert when it comes to tree shaking.

So for my edification, are you 100% sure that webpack won't actually take a lodash-es import and get rid of everything it doesn't need? Because I thought that was the whole point of 1. webpack 2. the existance of the lodash-es package.

Happy to be schooled, you're just rocking my world a little bit.

@gabriel-curtino
Copy link
Contributor Author

gabriel-curtino commented Sep 25, 2020

Hi!

I'm not an expert neither and I thought same as you... but following your examples to debounce a function in my app I saw how the bundle size jumped up from 953Kb to 2.26MB just for importing debounce as the example shows. At the end, as I only need one module, I'm importing it. I can confirm debounce works perfect importing only its module.

After a bit more of research today found this. It seems we don't even need to use lodash-es since lodash v4. I can confirm now that importing only one module from lodash is even better in RoR 6+ with default webpack setup.

Take a look:

ℹ webpack 4.44.1
ℹ lodash@^4.17.19
ℹ lodash-es@^4.17.15

Before import debounce:

js/application-afbf8cd74a7930fea48d.js 953 KiB application

After import as the docs recommends:

import { debounce } from 'lodash-es'
js/application-8cedc46cd9b8e2c3c2ff.js 2.26 MiB application

import { debounce, throttle } from 'lodash-es'
js/application-03a6646ac3550f548a6a.js 2.26 MiB application

Importing modules from lodash-es:

import debounce from 'lodash-es/debounce'
js/application-9b816b999fa42d724948.js 983 KiB application

import debounce from 'lodash-es/debounce'
import throttle from 'lodash-es/throttle'
js/application-7b68f2949ef2221f02b3.js 986 KiB application

Importing modules from lodash:

import debounce from 'lodash/debounce'
js/application-6b4997c6f0cf8cc55740.js 979 KiB application

import debounce from 'lodash/debounce'
import throttle from 'lodash/throttle'
js/application-80b9d725bec9572ac481.js 983 KiB application

Well, I think the changes I've proposed are not 100% correct. Should we edit the docs and advice imports directly from lodash v4+?

@hopsoft
Copy link
Contributor

hopsoft commented Sep 30, 2020

Note that we may want to raise awareness about the debounced JS lib in the docs to help people address this concern. https://github.com/hopsoft/debounced

@leastbad leastbad merged commit 9ed1b7a into stimulusreflex:master Oct 3, 2020
@leastbad
Copy link
Contributor

leastbad commented Oct 3, 2020

Generally, @gAHIA, we merge even with slight imperfections so that you get due contributor credit. Your research is good and I'm going to update the document further as per what the article suggests. Again, thank-you for pointing all of this out.

@hopsoft I will definitely work in a mention of debounced. That said, the lodash methods are extremely versatile.

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.

3 participants