SimpleBar with HTML API: 949 lines
SimpleBar without HTML API: 819 lines
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.
We should group DOM read and write (like what fastDOM is doing) to reduce layout trashing
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:
resize-observer-polyfill entirely from the dependencies.lodash-es instead of importing the lodash/function-name version since we were very likely importing twice the same helper functions (thanks @hc-codersatlas)@Grsmto Any idea when [email protected] and [email protected] will be published to npm?
@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.
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:resize-observer-polyfillentirely 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.
lodash-esinstead of importing thelodash/function-nameversion since we were very likely importing twice the same helper functions (thanks @hc-codersatlas)