Simplebar: Opportunity to save kb and improve performances

Created on 24 Sep 2019  路  12Comments  路  Source: Grsmto/simplebar

  1. The framework plugins don't need the HTML API, so we could provide a second bundle that gets rid of the global mutation observer and everything that comes with it.
    From my tests we could save about 5kb of script with this, plus one less mutation observer that could improve performances especially in a JS app scenario.

SimpleBar with HTML API: 949 lines
SimpleBar without HTML API: 819 lines

  1. resizeObserver is now supported on most modern browser so we should probably not include the polyfill by default anymore https://caniuse.com/#feat=resizeobserver
    Edit: actually iOS Safari still doesn't include it so we might keep it anyway.

  2. We should group DOM read and write (like what fastDOM is doing) to reduce layout trashing

enhancement

Most helpful comment

There is!

I released a new beta version: [email protected]. I didn't advertise it cause I need to write changelog and everything but I'll do a sum up here, as most suggestions in this issue were applied:

  • Remove resize-observer-polyfill entirely from the dependencies.
    I didn't find an easy solution to "have it by default" and let advanced users remove it if needed, so let's just get rid of it. I totally agree that it should be lightweight by default.
  • Move to lodash-es instead of importing the lodash/function-name version since we were very likely importing twice the same helper functions (thanks @hc-codersatlas)
  • As suggested in my initial comment, the HTML API is now separated. That saves us a few kb.

All 12 comments

Following up on discussion in #391, should we remove resize-observer-polyfill from the bundle and let users install it themselves?

@Grsmto I'd be 100% in favor. My app already conditionally polyfills ResizeObserver (so Chrome and Firefox users don't have to download it for example), so by including simplebar I'm undoing that optimization.

It makes sense and I agree as well with that statement.
Will definitely work on that for the next release. It's a shame this came up right after the breaking 5.0 release! x)

Especially since lazy/conditional modules loading with Webpack is a breeze, polyfills shouldn't be included by default.

Still regarding core-js I'm not too sure we could do anything.

So I really quickly tried to remove resize-observer-polyfill, the problem is that it's not anymore a polyfill but a ponyfill, so I'm not sure how that would work for the users as I suppose they would have to apply the ponyfill to window.resizeObserver before SimpleBar init:

import ResizeObserver from 'resize-observer-polyfill';
import 'simplebar';
import 'simplebar/dist/simplebar.css';

window.ResizeObserver = ResizeObserver;

This definitely makes usage more complex. I also had in mind to just have 2 bundles in the main package: one with (that would basically do what I just wrote above) and one without the polyfill. It would keep the default import with polyfill and also we would avoid the major release breaking change.

Something like:

import 'simplebar/dist/simplebar-no-polyfills.esm';

So for advanced users they can optionally get rid of the polyfills if necessary. Not sure it's something that would make sense...What do you think?

Having two bundles would solve the issue for me. The only thing is that it complicates your build and release process a bit...

For saving kb why not optimize lodash too? See: https://github.com/fkhadra/react-on-screen/issues/55 for example

Has there any progress on this?

There is!

I released a new beta version: [email protected]. I didn't advertise it cause I need to write changelog and everything but I'll do a sum up here, as most suggestions in this issue were applied:

  • Remove resize-observer-polyfill entirely from the dependencies.
    I didn't find an easy solution to "have it by default" and let advanced users remove it if needed, so let's just get rid of it. I totally agree that it should be lightweight by default.
  • Move to lodash-es instead of importing the lodash/function-name version since we were very likely importing twice the same helper functions (thanks @hc-codersatlas)
  • As suggested in my initial comment, the HTML API is now separated. That saves us a few kb.

@Grsmto Any idea when [email protected] and [email protected] will be published to npm?

@Grsmto Is this an accurate size raport?

This also shows that it's ~13Kb with Broccoli compression.

@gitcatrat yes I think so.

It seems like the Lodash functions are taking a lot of kb. Especially memoize I need to do some test, maybe we don't need it.

Yup, don't even need memoize. It seems to be only used for caching RTL helper result. I suggest to cache it manually.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

JoshClose picture JoshClose  路  7Comments

juliandreas picture juliandreas  路  3Comments

barnu5 picture barnu5  路  5Comments

luky1984 picture luky1984  路  5Comments

TheeOhioState picture TheeOhioState  路  6Comments