amcharts4 icon indicating copy to clipboard operation
amcharts4 copied to clipboard

tree shaking does not work - bundle size is huge

Open oocx opened this issue 6 years ago • 48 comments

steps to reproduce: build a simple Angular cli project that uses amcharts4, for exapmle the sample from node_modules@amcharts\amcharts4\examples\angular\simple-line-chart

The output contains 2 MB pdfmake.js and 1,2 MB xlsx.js, and main.js is way too large.

I started migrating to amcharts4 because the version 4 overview page advertises tree shaking, and I hoped to reduce my bundle size. Instead, bundle size is much larger now.

oocx avatar Jun 18 '18 14:06 oocx

@oocx I tried building our simple-line-chart example, here are the file sizes I got:

main.js           12 KB
polyfills.js     222 KB
runtime.js         8 KB
styles.js         16 KB
vendor.js      5,787 KB
             = 6,045 KB

I then tried removing all of the amCharts-related stuff, and tried building it again, and these are the file sizes I got:

main.js           11 KB
polyfills.js     222 KB
runtime.js         6 KB
styles.js         16 KB
vendor.js      2,672 KB
             = 2,927 KB

That means that the amCharts code is 3,118 KB. That's quite a lot, but it's not unreasonable compared to many modern JS libraries (Angular by itself is 2,927 KB!) And of course we are looking into ways to reduce the filesizes.

As for the extra files (such as fabric.js, pdfmake.js, responsivedefaults.js, and xlsx.js), they are not loaded by default, they are loaded on-demand if you use the Export functionality, so if you don't use Export then they won't be loaded at all.

Pauan avatar Jun 19 '18 13:06 Pauan

Here I followed the amCharts 3 Vue.js with Vue CLI 3 tutorial.

Since it is not using the Export functionality, why it loads using @vue/cli 3 (tested on 3.0.0-rc.3) and it also getting the extra files (pdfmake.js , xlsx.js, etc) even on production mode or build?

If it needs to be disabled, how I can do that?

I got this files after build it:

File                                      Size             Gzipped

dist\js\pdfmake-legacy.2c5519d5.js        1670.31 kb       820.01 k
dist\js\xlsx.5692312e.js                  642.42 kb        313.54 k
dist\js\fabric.cc5cb61d.js                282.76 kb        78.68 kb
dist\js\responsivedefaults.84b7f978.js    10.07 kb         2.83 kb

image

I may have missed something here. Please let me know what I have to config here and also include the step in your tutorial page.

peluprvi avatar Jun 19 '18 18:06 peluprvi

Adding a simple line chart to my app should not add 3 MB to the bundle size. When you analyze the sample with webpack-bundle-analyzer, you can see that the bundle contains the code of all chart types and all of amcharts features. If tree shaking was working correctly, I would expect the bundle to only contain the code for the chart types I used. So for the sample, it should contain only the LineSeries chart.

I see that https://www.amcharts.com/v4/ does not mention tree shaking any more, so I guess it now works as advertised :). However, it would be great if AmCharts would consider adding real tree shaking support. I will probably just use another, less feature rich but smaller library like chart.js (only 37kb),, as I don't need 90% of the features and code that amcharts would add to my bundle.

oocx avatar Jun 20 '18 09:06 oocx

@oocx Tree shaking of classes is a very tricky subject. Because of the dynamic nature of JavaScript (and the output of transpilers like Babel and TypeScript), it is very difficult for tree shaking to work correctly.

This problem isn't specific to amCharts, it happens with all uses of classes.

Here are some examples of that:

https://github.com/webpack/webpack/issues/2899

https://github.com/webpack/webpack/issues/4080

https://github.com/mishoo/UglifyJS2/issues/1261

https://stackoverflow.com/questions/39065038/how-to-use-tree-shaking-for-classes-with-typescript-and-webpack

It's not really something we can "fix", since there's nothing wrong with our code, it's caused by limitations in the other tools (like UglifyJS and Webpack).

We would love to have great tree shaking support, but we cannot change the JavaScript language, and we cannot improve the tree shaking algorithms used by UglifyJS or Webpack.

Having said that, we completely agree that 3 MB is quite large for simple charts, and we will continue to investigate ways to improve the file size (through tree-shaking improvements, or other approaches).

Pauan avatar Jun 20 '18 14:06 Pauan

@peluprvi You're right, I just tested our Vue tutorial and it does load those files, which is incorrect. With our Angular tutorial it correctly does not load the files.

I don't have any experience with Vue, I'll ask the rest of our team to take a look.

Pauan avatar Jun 20 '18 15:06 Pauan

@oocx Aha! I figured out the reason for the large file sizes. The Angular CLI does not use minification when doing ng build. Instead you have to use ng build --prod.

Here are the new file sizes when using ng build --prod:

main.js        819 KB
polyfills.js    59 KB
runtime.js       2 KB
             = 880 KB

And here are the file sizes when not using amCharts:

main.js        153 KB
polyfills.js    59 KB
runtime.js       2 KB
             = 214 KB

As you can see, amCharts only adds a 666 KB overhead. That's still a lot, but certainly much better than 3 MB!

Pauan avatar Jun 20 '18 15:06 Pauan

@peluprvi we are still investigating a proper way for webpack/vue not to use those files by default (unless needed) but as a quick workaround you can tap into the Vue CLI generated webpack config and tell it not to prefetch these files.

To do that, create a vue.config.js file in the root of your project and add this to it:

// vue.config.js
module.exports = {
  chainWebpack: config => {
    config
      .plugin('prefetch')
      .tap(args => {
        return [
          {
            rel: 'prefetch',
            include: 'asyncChunks',
            fileBlacklist: [
              /\.map$/,
              /pdfmake\.[^.]+\.js$/,
              /xlsx\.[^.]+\.js$/,
              /fabric[^.]*\.[^.]+\.js$/,
              /responsivedefaults\.[^.]+\.js$/,
            ]
          }
        ]
      })
  }
}

The files will still be generated to the output directory but they will not be loaded, which is the most important part, imo.

ailon avatar Jun 21 '18 14:06 ailon

@Pauan thanks for checking!

@ailon thanks for your effort! Your workarround only works when production mode (vue-cli-service serve --mode=production). It also broken the build process when using --modern flag (vue-cli-service build --modern).

To make it work with the --modern flag, I changed it to:

// vue.config.js
module.exports = {
  chainWebpack: config => {
    if (config.plugins.store.get('prefetch')) {
      config
        .plugin('prefetch')
        .tap(args => {
          args[0].fileBlacklist = [
            /\.map$/,
            /pdfmake\.[^.]+\.js$/,
            /xlsx\.[^.]+\.js$/,
            /fabric[^.]*\.[^.]+\.js$/,
            /responsivedefaults\.[^.]+\.js$/
          ]
          return args
        })
    }
  }
}

I agree that in my case the most important is to not load the files, but including unused files to the build process also increases its time (~85% of my building time right now is for that files).

I'll keep waiting for a definitive and automatic solution.

Edit: Also remembering if the product uses a column chart, the app doesn't need to load other chart scripts.

peluprvi avatar Jun 21 '18 15:06 peluprvi

Although it's not a substitute for tree shaking, gzip compression makes a huge difference:

Angular with amCharts    = 235.09 KB
Angular without amCharts =  86.02 KB

As you can see, with gzip compression enabled, amCharts V4 only adds 149.07 KB.

Pauan avatar Jul 01 '18 09:07 Pauan

I created a base that you can download and edit to test it ~~https://codesandbox.io/s/828rxoq2lj~~ https://github.com/peluprvi/vue-amcharts4-teste Maybe there is a way to use babel-plugin-transform-imports to avoid full import the lib.

peluprvi avatar Jul 03 '18 14:07 peluprvi

For Vue, it is related to https://github.com/vuejs/vue-cli/issues/979 and documented there on https://cli.vuejs.org/guide/html-and-static-assets.html#prefetch

peluprvi avatar Jul 03 '18 15:07 peluprvi

Is there any optimization for Angular 2+?

I see in the docs that the way to import it on Angular is like this:

import * as am4core from "@amcharts/amcharts4/core";
import * as am4charts from "@amcharts/amcharts4/charts";

This would not allow tree shaking to work as expected I believe.

eliashdezr avatar Sep 18 '18 06:09 eliashdezr

- import * as am4core from '@amcharts/amcharts4/core';
- import * as am4charts from '@amcharts/amcharts4/charts';
+ import { useTheme, create, Scrollbar } from '@amcharts/amcharts4/core';
+ import { XYChart, DateAxis, ValueAxis, LineSeries, XYCursor } from '@amcharts/amcharts4/charts';

Shyam-Chen avatar Dec 24 '18 02:12 Shyam-Chen

Even by importing this way, it is including pdfmake, xlsx and others in the bundle process (Vue environment), increasing the bundle process time:

image

peluprvi avatar Jan 21 '19 12:01 peluprvi

Hi guys,

Any updates on optmising the bundle size?

When you say that js chunks like 'pdfmake' are loaded on demand, do you mean when the end user requests them or if we request it in a script?

fonziemedia avatar Feb 26 '19 14:02 fonziemedia

For anybody checking this ticket, I also recommend checking #569.

darlesson avatar Feb 28 '19 05:02 darlesson

@fonziemedia When the user exports the chart (using the export menu in the upper-right), then it will download the files. Until then, the files aren't downloaded.

You shouldn't include them in <script> tags, since they'll be loaded automatically on-demand.

Pauan avatar Mar 04 '19 01:03 Pauan

That's great @Pauan / @darlesson

Thanks for clarifying!

fonziemedia avatar Mar 08 '19 14:03 fonziemedia

Bundle sizes are also huge using create-react-app. Why not restructure the project on a "one file per chart type" basis in order to make tree shaking work?

delaaxe avatar Mar 20 '19 17:03 delaaxe

@delaaxe We already do that (you can see the source code here).

The issue is that Webpack will always include all files (even files that aren't used). This is mandated by the ES6 spec.

Tree shaking just doesn't work as well in JavaScript as it does in other languages, and there isn't much we can do about it.

Pauan avatar Mar 23 '19 16:03 Pauan

I'm evaluating a few options for charting packages and have really enjoyed testing amcharts! It is very efficient and easy to jump into and having pdfmake load on demand is a great idea 😄

I do agree bundle size can be issue though. I'm seeing ~178kb gzipped after removing all of the extras like pdfmake and xlsx, which is still bigger than I was hoping for.

I think chart.js has been working on treeshaking in the same way some people have suggested here (import SomChartType from 'package/path' and having only that chart type and its dependencies loaded). It might be worth checking out a few of the issues / PRs they've created to see if anything might be useful for the amcharts team.

References note that they have had this issue being tracked since May 5, 2016 https://github.com/chartjs/Chart.js/issues/4478 https://github.com/chartjs/Chart.js/issues/2466

A clever or perhaps unintentionally effective approach to reduce bundle size that I've noticed Highcharts has done is to split out Highcharts from Highstock. Idk if this type of splitting of the amcharts package is possible but as you can see in the image below, if I didn't need the highstock functionality it significantly reduces bundle size.

Together they are the same size as amcharts, but individually are about half with Highstocks being about 100kb gzipped and 78kb gzipped for highcharts.

The difference between the two, at a quick glance, seems to be the time series advanced navigation tools like scrolling, zooming and date picker etc... Maybe amcharts could have a diet / lite amcharts version without these features? 😆

Highcharts and Highstock -- v7.1.1 image

amcharts without accessories -- v4.3.14 image

onx2 avatar Apr 15 '19 22:04 onx2

Related: https://github.com/amcharts/amcharts4/issues/1267

justinhelmer avatar May 04 '19 17:05 justinhelmer

@onx2 We originally split each chart into a separate file (so you'd load @amcharts/amcharts4/charts/serial for example).

However, after looking at the compiled output, core.js is 733 KB, and all of the charts put together is 245 KB.

The core.js is shared by all the charts, so it can't really be tree shaken.

So it didn't make any sense to split the charts, since it wouldn't save much KB.

Pauan avatar May 10 '19 15:05 Pauan

@Pauan - It depends on what you define as "much kB".

The core is massive, but I understand that is a larger effort to tackle. @martynasma mentioned the interwoven nature of dependencies in core in #1267.

However, all charts combined at 245kB is not something that should be shrugged at.

Here is a great article by @addyosmani regarding the cost of JS, and what your JS budget (in kB) should be: https://medium.com/@addyosmani/the-cost-of-javascript-in-2018-7d8950fbb5d4

In today's web world, customers expect things to be immediate. In most projects, I limit the overall bundle size of all vendor dependencies combined to just 30kB (gzipped). amCharts is nearly 1MB combined, and can only be reduced to 187kB with minification/obfuscation/compression. This is huge.

I would suggest taking a phased approach to addressing this problem. 187kB with all of the most modern compression/obfuscation algorithms should not be considered acceptable. If splitting the charts out into individual consumable files is low-hanging fruit, then I think that would be a great place to start. It wouldn't solve the bigger issue, but it would be a positive step in the right direction. As an example, it would be possible to shave off the majority of the 245kB you cited if a consumer just needed a pie chart. More interestingly, it would also inherently introduce the ability to load specific charts on-demand, using techniques such as lazy-loading and code splitting.

I think it is important for your team to continue to elevate this as an essential benchmark for success of amCharts, as it is with any web-based project. Performance is a metric for success in today's web world, and heavily correlates to customer engagement and conversion. This metric especially holds weight for websites that thrive on SEO, as performance results have a direct correlation to their page rank. These are very real-world problems with metrics to support this need.

As an example, I once worked for a company that had tools in place to track performance and its impact on the conversion funnel. At one particular point in time we did an evaluation that resulted in understanding that a quarter of a second improvement in key performance metrics resulted in a 1.3% increase in revenue. Due to the volume of the B2C, such a change would yield a six figure improvement in revenue per day. These are not small numbers.

I appreciate the consideration.

justinhelmer avatar May 10 '19 21:05 justinhelmer

Any update? This is very critical.

My project avoids as many dependencies to be as small as possible, but after adding amCharts it went down the drain, extra 2MB and 300% larger bundle.

I understand that tree shacking in JS doesn't work very well, but combined with Code Spliting and Lazy load this is fine, but everything goes down the drain when amCharts comes in.

marlonmleite avatar Aug 02 '19 13:08 marlonmleite

Absolutely, at this point highcharts seems better option if performance is a must. Been trying to integrate amcharts but it is not feasible due the exceed of bundle size.

eliashdezr avatar Aug 02 '19 14:08 eliashdezr

Hi @Pauan and @peluprvi,

I am seeing a very huge bundle size because of pdfmake and xlsx, which I am not using in my project. I am using vue-cli version3.10 and amcharts4.6.4 and I have tried this workaround as well:

// vue.config.js module.exports = { chainWebpack: config => { if (config.plugins.store.get('prefetch')) { config .plugin('prefetch') .tap(args => { args[0].fileBlacklist = [ /.map$/, /pdfmake.[^.]+.js$/, /xlsx.[^.]+.js$/, /fabric[^.]*.[^.]+.js$/, /responsivedefaults.[^.]+.js$/ ] return args }) } } } in vue.config.js but it seems this is not working for me. here is the snapshot: amcharts_log

Is anybody working on this issue? Can someone point me what am I doing wrong?

b49015 avatar Sep 30 '19 15:09 b49015

I would love to get some traction on this issue. The impact is debilitating.

As you can see in @b49015 's post above, using webpack-bundle-analyzer illustrates the issue well. In the above image, the orange, green, purple and cyan blocks are all dependencies of amcharts. It's 80% of the entire size of the project for a single library. As you can see, all other third party dependencies combined (node_modules) are smaller than the single core charts.js file.

When using webpack-bundle-analyzer with our project, we get very similar results.

At the price point of amcharts, there should be an expectation of a higher standard for performance.

Please revisit this as a priority.

justinhelmer avatar Oct 22 '19 15:10 justinhelmer

@b49015 @justinhelmer When doing a production build, pdfmake.js and xlsx.js are compiled into their own bundles which never get downloaded by the browser if you don't use the PDF or Excel export features.

The core amcharts code is still huge and should be made more modular. ES modules anyone?

delaaxe avatar Nov 10 '19 13:11 delaaxe

@justinhelmer 30KB is an awfully low limit. Angular is 6MB, jQuery is 86KB, React is 128KB, PixiJS is 359KB, Highcharts is 233KB. All are widely used and accepted by organizations.

Obviously we want smaller code size, but that has to be balanced with providing value to our customers, since our customers expect a lot of functionality. We provide more features than any other charting library.

The idea of splitting each chart type into a separate file is something we want to try, but it's tricky to get the build process right.


@marlonmleite You shouldn't be seeing a 2MB increase. Are you excluding pdfmake and xlsx from the size calculations?


@delaaxe We already use ES6 modules. The problem is that dead code elimination does not work very well for JS classes. We've considered various ways to workaround that, but it's a very tricky topic, which is why other projects (such as Angular) also struggle with large file sizes, it's definitely not unique to amCharts.

Pauan avatar Nov 23 '19 02:11 Pauan