fix bundlesize - Githubissues

adi518 commented 3 years ago

This PR fixes Rollup's external config. It was left an empty array for some strange reason, which means it didn't externalize all production dependencies from the bundle, resulting in a significantly large bundle (70.4KB, gzipped, see: https://bundlephobia.com/package/peaks.js@0.26.0). With this fix, the bundlesize is now 16.38KB (gzipped), a 76.732% difference. 🤯

Summary:

Fix Rollup external (impl from TSDX)
Add esm build to enable preliminary tree-shaking.
Add analyze script and size-limit (also from TSDX)
Move build to dist folder, which makes it easier to inspect (also from TSDX)
Fix some lint warnings
Remove unused dep watchify.

Btw, as you can see there's an issue with the waveform-data dependency, as it unnecessarily includes core-js, a set of polyfills that are NOT needed in today's browsers. That alone can be another 15-20KB reduction for the end-consumer. That should be handled in the dependency repository and later bump its version here.

Notes:

It's probably noteworthy that the library is maintained with npm 7, so anyone with npm <= 6 will regress package-lock.json into an old format ("lockfileVersion": 1).

chrisn commented 3 years ago

Thanks, I'll take a look at this. One thing I found is that the demo pages no longer work using the smaller peaks.js bundle.

Btw, as you can see there's an issue with the waveform-data dependency, as it unnecessarily includes core-js, a set of polyfills that are NOT needed in today's browsers.

core.js is part of the waveform-data library, not a separate dependency.

adi518 commented 3 years ago

I'll check the demo, thanks. Core.js is a bit extraneous, it can originate from the build-tool dependencies or appear as a standalone dependency that's used by the build-tool. It's usually removed by changing the build configuration and then removing it from the dependencies. In this case it's not an explicit dependency, so it only needs to be removed by configuration.

Edit: I confused core.js with the npm package core-js. 😌

adi518 commented 3 years ago

I fixed the demos. The reason they broke is because of externalising the dependencies obviously, so you have to treat each demo as an app that consumes the library, which means you have to add UMD scripts of the dependencies to each demo HTML page. However, that's tedious and repetitive, so I tricked it. I renamed build-demo to what it actually is, build-custom-markers-demo and added a new script build-demo, which builds the library, but this time, without externalising its dependencies, so the demos work as good as before. So now you have these scripts:

build - Builds the library.
build-demo- Builds the library demo.
build-custom-markers-demo - Builds the library Custom Markers demo.

chrisn commented 3 years ago

However, that's tedious and repetitive, so I tricked it.

The purpose of the demos is to show people how to use the library. So if we're using external dependencies, the demos should show how to do that.

chrisn commented 3 years ago

Thanks a lot for working on this. Reducing the bundle size is a good idea, and this is really helpful! Using external dependencies it's possible to do this, which overcomes the need to use a module bundler for the custom-markers demo:

<script src="https://unpkg.com/waveform-data@4.1.0/dist/waveform-data.js"></script>
<script src="https://unpkg.com/konva@8.1.0/konva.js"></script>
<script src="/peaks.js"></script>

Using externals for waveform-data and konva is good, but probably not worth it for eventemitter3.

adi518 commented 3 years ago

Yes, those are the dependencies that should be added to each demo, if you want to showcase usage properly. In a modern app though, it's not needed, so really, every dependency should be externalised.

Reducing the bundle size is not only helpful, but also very important. Consumers take bundle-size very seriously these days. You can build an app and reach megabytes if you are not careful. Then there's tree-shaking, which allows even more bundle saving, since it enables consumers to bundle only the code they consume. If a library exports ten methods and you end up using just two, then only these two methods will be in your app bundle.

chrisn commented 3 years ago

But this is why I'm a bit confused... Peaks.js is now using ES modules (although the last published release, 0.26.0 doesn't). So, from the next release onwards, anyone using Peaks.js with a modern module bundler will be using the module entry in package.json and producing their own bundle, which will de-duplicate any dependencies.

Using external reduces the size of the bundled peaks.js file, which only affects people who prefer to use a <script> tag. For those people, we actually increase the total amount of JavaScript they need to download, because they'll be using the full Konva library, and not just the specific component modules that Peaks.js uses.

adi518 commented 3 years ago

I explained ESM and the module entry above, but I'll go into further details here. While you are correct that it will deduplicate dependencies, consumers can't/shouldn't consume your source, because it may include stuff that only works within your development environment, namely you build it in order to provide production-ready code that is decoupled from your development code/environment. I'll provide an example. Many build tools provide you with an alias option to resolve dependencies through an alias instead of using relative paths. It usually looks like this:

import Foobar from "@components/Foobar"; // @components = <project-root>/src/components

// instead of

import Foobar from "../../components/Foobar"; // quite painful

That won't work if someone were to consume a dependency from its source code. Although it may not be relevant to this library, you get the idea. From the next release onwards, unless someone hacked their consumption of this library, the transition should be seamless, as if nothing changed. I would release it as a minor though, perhaps major even.

Using external reduces the size of the bundled peaks.js file, which only affects people who prefer to use a <script> tag. For those people, we actually increase the total amount of JavaScript they need to download, because they'll be using the full Konva library, and not just the specific component modules that Peaks.js uses.

Konva does have a tree-shakable build, which Peaks utilizes through Rollup, hence your assumption is correct, consumption through script tags will increase the amount of JavaScript they need to download. However, if they need Konva for something else, they'll have to import it and duplication will occur. Also, that's a very legacy method, which doesn't have build optimizations so it's expected. In the past, Peaks probably had the full Konva library bundled with it, because Konva wasn't tree-shakable or listed as a peer dependency only, so in retrospect, it doesn't make much difference when consuming through script tags.

adi518 commented 3 years ago

@chrisn Any updates?

chrisn commented 3 years ago

Please take a look at these recent commits on master: https://github.com/bbc/peaks.js/commit/0207a452dbba5df8e9edb7c9573e3a0363b181e4 and https://github.com/bbc/peaks.js/commit/1cef03d4e51185424b646b9a20619904ac6bb38d. I have added an ES module build, and added source maps to the ES and UMD modules.

Should we include a minified ES module build? Wouldn't minification be applied by a module build tool that consumes the Peaks.js ES module?

For UMD, I think that a build with all dependencies bundled is still useful, for people who want to simply add a Githubissues.

Githubissues is a development platform for aggregating issues.

bbc / peaks.js

fix bundlesize #410