Chart.js icon indicating copy to clipboard operation
Chart.js copied to clipboard

Tree shaking not working as expected?

Open kfancy opened this issue 3 years ago • 10 comments

Expected behavior

Using named imports to target what components are imported, the bundle size created with webpack by the host should be smaller than including everything in ChartJS, as described in the integration docs.

https://www.chartjs.org/docs/latest/getting-started/integration.html

Current behavior

Bundle size is the same for both the "kitchen sink" import and the named imports version.

Not really a reproducible issue that is code-based, it's for bundling/minifying for production use... please note this is why the required reproducible sample is just the react-chartjs-2 template.

Reproducible sample

https://codesandbox.io/s/react-chartjs-2-chart-js-issue-template-cg7b5?file=/src/App.tsx

Optional extra steps/info to reproduce

Using this in a react app, where the targeted component is lazy-loaded to try and isolate the ChartJS code into its own bundle. Both of these import strategies result in the same 97.5kb minified dynamic import bundle.

Strategy 1: named imports

import {
  CategoryScale,
  Chart,
  Legend,
  LinearScale,
  LineController,
  BarController,
  LineElement,
  PointElement,
  Tooltip,
  BarElement
} from 'chart.js';

Chart.register(
  LineController,
  PointElement,
  Legend,
  Tooltip,
  CategoryScale,
  LinearScale,
  LineElement,
  BarController,
  BarElement
);

Strategy 2: "kitchen sink" registrables import:

import { Chart, registerables } from 'chart.js';
Chart.register(...registerables);

Lazy-loading technique (for reference):

const ChartJsTreeShakeTester = React.lazy(() => import('./ChartJsTreeShakeTester'));

//// elsewhere in the code, used with Suspense as directed:
<Suspense fallback={<div>Loading chart...</div>}>
    <ChartJsTreeShakeTester />
</Suspense>

Possible solution

No response

Context

We are trying to create small production bundles using chartJS, and only use a subset of the components. This affects us by forcing the bundle to contain code we're not using, and affects our customers with longer download times.

chart.js version

3.7.1

Browser name and version

n/a

Link to your project

No response

kfancy avatar Feb 15 '22 02:02 kfancy

I can confirm this, webpack bundles all chart types, regardless of what is imported.

hovcharenko avatar Mar 08 '22 12:03 hovcharenko

Couple of things I recall about webpack:

  • it does not tree-shake unless doing a production build (just fyi)
  • it might require "sideEffects": false in package.json

So as you already have the setup, could you do a test? I think it should be sufficient to manually add the sideEffects entry in chart.js package.json in your node_modules.

kurkle avatar Mar 08 '22 12:03 kurkle

Hey @kurkle thank you for the tips. I tried adding sideEffects to the package.json but there is no difference in the bundle size. Webpack is bundling the whole library even though we are just using the Doughnut, Chart and ArcElement.

Any other suggestions?

Thanks in advance

carlosagsmendes avatar Mar 16 '22 10:03 carlosagsmendes

Hey @kurkle thank you for the tips. I tried adding sideEffects to the package.json but there is no difference in the bundle size.

Just making sure, you added the sideEffects in node_modules/chart.js/package.json ?

kurkle avatar Mar 16 '22 10:03 kurkle

Yes, I tried it out on my root package.json and on node_modules/chart.js/package.json and there's no difference.

Thank you!

carlosagsmendes avatar Mar 16 '22 11:03 carlosagsmendes

Couple of things I recall about webpack:

  • it does not tree-shake unless doing a production build (just fyi)
  • it might require "sideEffects": false in package.json

So as you already have the setup, could you do a test? I think it should be sufficient to manually add the sideEffects entry in chart.js package.json in your node_modules.

I tried all version since 3.0.0 and none of them has working tree shaking with webpack. I also tried to add "sideEffects": false to chartjs package json, didn't work. Do you have any working example, perhaps with any other bundler like rollup?

tungtrinh avatar Mar 17 '22 11:03 tungtrinh

I confirm that I did the same tests using NextJS with Webpack, production build and it doesn't work

carlosagsmendes avatar Mar 17 '22 13:03 carlosagsmendes

When I test this I do see a minimal change between importing the minimal things to show a bar chart or importing everything.

What I did was use create-react-app to create a new react app. Only show a canvas with the bar chart. Then I build the app and check the size of the build folder. When using the auto or ...registerables way of importing/registering everything I noticed they both had (almost) the same size, When I only imported the basics for a bar chart I noticed it was a bit smaller (around 6 mb and 40kb on disk)

import './App.css';
import { useEffect } from 'react';

// Size: 1.43 MB (1,504,072 bytes)
// Size on disk: 828 KB (847,872 bytes)
//import Chart from 'chart.js/auto';

// Size: 1.43 MB (1,503,745 bytes)
// Size on disk: 828 KB (847,872 bytes)
// import {Chart, registerables} from 'chart.js';
// Chart.register(...registerables);

// Size: 1.37 MB (1,441,057 bytes)
// Size on disk: 788 KB (806,912 bytes)
// import {Chart, BarController, BarElement, CategoryScale, LinearScale} from 'chart.js';
// Chart.register(BarController, BarElement, CategoryScale, LinearScale);

// Size: 1.40 MB (1,469,466 bytes)
// Size on disk: 800 KB (819,200 bytes)
import {Chart, BarController, BarElement, CategoryScale, LinearScale, Legend, Tooltip} from 'chart.js';
Chart.register(BarController, BarElement, CategoryScale, LinearScale, Legend, Tooltip);

function App() {
  useEffect(() => {
    if (Chart.getChart('zz')) Chart.getChart('zz').destroy()

    new Chart('zz', {
      type: 'bar',
      data: {
        labels: ['F', 'D', 'X'],
        datasets: [{
          data: [2,3,4],
          backgroundColor: 'pink'
        }]
      }
    })
  }, [])


  return (
    <div className="App">
      <header className="App-header">
        <canvas id="zz"></canvas>
      </header>
    </div>
  );
}

export default App;

So it seems to me the treeshaking is working, only thing I can think off why it might seem that it doesnt work is that under the hood a lot of things depend on the same base classes. So the dataset controllers for example all use the core dataset controller which is a pretty big part. So the small parts dont matter anymore then.

This can bee seen when I add the legend and tooltip to the imports.

LeeLenaleee avatar Mar 17 '22 16:03 LeeLenaleee

Hopefully someone can fix this 🥹. The bundles size include 67kbs gzip 🥲. Thanks image

harrytran998 avatar May 26 '22 10:05 harrytran998

Still like this in https://github.com/chartjs/Chart.js/releases/tag/v3.8.0

ennioVisco avatar Jul 05 '22 07:07 ennioVisco