Chart.js
Chart.js copied to clipboard
fix: treeshaking
https://github.com/chartjs/Chart.js/issues/10163 fix
Before (build of each export):
378 kB Chart.js
378 kB BarController.js
378 kB BubbleController.js
378 kB DoughnutController.js
378 kB LineController.js
378 kB PolarAreaController.js
378 kB PieController.js
378 kB RadarController.js
378 kB ScatterController.js
378 kB ArcElement.js
378 kB LineElement.js
378 kB PointElement.js
378 kB BarElement.js
385 kB Decimation.js
394 kB Filler.js
397 kB Legend.js
382 kB SubTitle.js
382 kB Title.js
384 kB Tooltip.js
378 kB CategoryScale.js
378 kB LinearScale.js
378 kB LogarithmicScale.js
378 kB RadialLinearScale.js
378 kB TimeScale.js
378 kB TimeSeriesScale.js
After:
210 kB Chart.js
83.7 kB BarController.js
72.5 kB BubbleController.js
80.2 kB DoughnutController.js
74.1 kB LineController.js
75.2 kB PolarAreaController.js
80.4 kB PieController.js
71.1 kB RadarController.js
74.8 kB ScatterController.js
40.4 kB ArcElement.js
53.5 kB LineElement.js
36 kB PointElement.js
37 kB BarElement.js
36.4 kB Decimation.js
69.6 kB Filler.js
71.1 kB Legend.js
49.5 kB SubTitle.js
49.6 kB Title.js
85.8 kB Tooltip.js
84.2 kB CategoryScale.js
89.6 kB LinearScale.js
92.6 kB LogarithmicScale.js
106 kB RadialLinearScale.js
95.4 kB TimeScale.js
98.5 kB TimeSeriesScale.js
Assign properties to classes is a side effect, so that's why tree-shaking didn't work. Changed it to static properties.
Also, some calls (class constructors) are marked as pure.
Also added size-limit to control sizes of specific exports. Action is failing cause it should be in the base branch to compare sizes.
Using esbuild? I think most others can shake assigning class properties (it can have side effects, but usually does not). I'm not against changing to static properties, but its a breaking change.
@kurkle webpack with terser (default minifier, actually terser make tree-shake). Why it's braking change ? Public API stays the same.
Changing es version can break things.
@kurkle we can add, for example, swc to transpilation
my test bench: https://github.com/dangreen/chartjs-treeshaking-test
@kurkle we can add, for example, swc to transpilation
swc and even babel transpile it to side-effected code 😕
Using esbuild? I think most others can shake assigning class properties (it can have side effects, but usually does not)
https://github.com/terser/terser/issues/724
https://github.com/babel/babel/issues/5902
@kurkle Ok, it's breaking change, but it's a desirable and valuable fix. So what is the release plan?
I think this is one big thing to drive towards v4. I think we need to do 3.8.1 and 3.9 first. There has been some talk on moving to typescript in v4 (does not need to be full conversion IMO, just the transpilation stuff and some part as POC so things can be converted in steps). But this ts thing could pushed forward to v5 too.
@kurkle Got it. Thank you!
I'd be open to getting through 3.8.1 and 3.9 quickly so that we can start on v4 dev.
Will review this evening my time.
Personally, I have no objection to using static properties. They look to be fairly well supported and users can use something like Babel if they need to support older browsers. So I'd be in favor of this change
We need to merge these in some sane order to avoid tough rebases
@benmccann
users can use something like Babel if they need to support older browsers
I have the article about that 🙂
https://dev.to/cubejs/why-and-how-to-transpile-dependencies-of-your-javascript-application-3phf
@kurkle I think it will better to merge before https://github.com/chartjs/Chart.js/pull/10525
Going to merge this first, then #10525 can be rebased