v3: consider changing export logic for esm modules

[sgratzl]

Feature Proposal

the in the v3 version the enhanced Chart module is exported as the solely (default) export from the library.

https://github.com/chartjs/Chart.js/blob/master/src/index.js

As an alternative one could have the following structure:

import ChartChart from './core/core.controller';
export { default as Chart } from './core/core.controller';
import DatasetController from './core/core.datasetController';
export { default as DatasetController }  from './core/core.datasetController';

class Chart extends ChartChart {
}
Chart.DatasetController = DatasetController;

export default Chart;

the purpose around the Chart class is that only the default export has all the dependencies but not the class itself.

Feature Use Case

I want to prepare for tree shaking and thus only import things that I really need. e.g.,

import {DatasetController} from 'chart.js';

class MyController extends DatasetController {
..}

note: while this works already with some synthetic imports (by rollup, ...) it is not standard conform.

Possible Implementation

[benmccann]

I would love to make Chart.js tree-shakeable

We had investigated a bit earlier. Some notes:

• We'd probably want two separate entry points. One for the UMD bundle that we create that provides everything and one for the ES6 build consumed via npm that is tree-shakeable
• @kurkle and I had a different understanding of how the bundlers did tree shaking. We did some experimentation and although my memory is a little fuzzy, I think we came to the following conclusion: webpack does tree-shaking on a per-file basis unless sideEffects: false is set.
  • Chart.js does have side-effects right now because the scales get automatically registered
• The biggest debate we've had is what the API should be.
  • Right now you specify scales and controllers with strings. E.g. 'time' or 'line'. I believe @etimberg suggested we keep it this way
  • Assuming we continue with the string API, it means we need to continue registering scales and also introduce registration for controllers.
  • Should scales and controllers register themselves when imported (Have scales and plugins register themselves #6979) or should we make users explicitly register them? While it's probably easier to have them auto-registered, that would reduce the effectiveness of treeshaking since we couldn't set sideEffects: false. I think deciding on this last point is the only real thing we need to do before implementing tree shaking.
    • We were previously blocked by a custom rollup css plugin that has since been removed, so it should be relatively doable now

@sgratzl any chance you would want to implement this? :-) We're all pretty busy and haven't had much time for core development lately, but we could talk and decide that last point if someone has the time to implement it

[kurkle]

Some quick opinions:

• I'd most likely go with manual registration after importing (of scales and controllers)
• I'd call the it Core or ChartCore or CoreChart rather than ChartChart.

[etimberg]

I am in favour of the string API if possible. I think its a better default out of the box since it's backwards compatible with v2 code and also quite easy to understand. It also doesn't break any existing documentation that's out there on SO or equivalent.

If we can do an opt-in tree shaking API initially, I am on-board. That would let us experiment and see what works well and then consider how to migrate all users to it eventually.

[benmccann]

Ok, I think basically all we need to do is:

• Register controllers just like we do scales and use the registry here
  • It would be good to create a generic register method that can take scales and controllers so users can register everything they need in one line
• Create a new entry point for npm / esm users

[sgratzl]

I think the biggest side effect in the library are the defaults. e.g., while elements don't need to be registered their defaults need to be, Thus creating a side effect. One approach could be using the same idea as in the scale case ->static .defaults attribute.

in https://github.com/sgratzl/chartjs-chart-boxplot/tree/develop I adapted the plugin to support tree shaking as good as possible. Essential changes:

• controller, elements, scale all have .id, .defaults. and .register. The .register function registers it within chart.js creating side effects. Moreover, when registering a controller, it also registers all its default elements and scales.
• I created https://github.com/sgratzl/chartjs-chart-boxplot/blob/develop/src/chart.js which is a wrapper around the library with a flat export hierarchy and utility register functions. In the best case chart.js at some point looks like this.
• For each controller I created a class inheriting the Chart chart which injects the type into its config. For example, the BoxPlotChart can be used to create a boxplot with implicitly registered the BoxPlotController. see also https://github.com/sgratzl/chartjs-chart-boxplot/blob/develop/samples/default_esm.html

[sgratzl]

* It would be good to create a generic register method that can take scales and controllers so users can register everything they need in one line

I would prefer a function per type. it is easier to ensure that the right arguments are given and one can avoid conflicts in names. e.g. a Violin controller and a Violin element.

* Create a new entry point for npm / esm users

I have a bundle.js and index.js where the bundle.js is like the esm index.js but registers everything for the UMD case.

I am in favour of the string API if possible. I think its a better default out of the box since it's backwards compatible with v2 code and also quite easy to understand. It also doesn't break any existing documentation that's out there on SO or equivalent.

in the best case, chart.js supports two options everywhere where a .type is required:

1. the string id that will be looked up in the registry
2. a constructor / function that is used to create the instance directly.

relevant locations as far as I can see:

• scale type, e.g. scales.x.type = 'category'
• controller type, e.g., .type = 'bar'
• element type, e.g., BarController.prototype.dataElementType = Rectangle
• interpolator type, e.,g., `animation.numbers.type = 'number'

[benmccann]

I would prefer a function per type. it is easier to ensure that the right arguments are given and one can avoid conflicts in names. e.g. a Violin controller and a Violin element.

If we had one method it could check the type (e.g. is it a subclass of DatasetController or Scale) and still store it in a separate registry based on type

I have a bundle.js and index.js where the bundle.js is like the esm index.js but registers everything for the UMD case.

That sounds good to me

in the best case, chart.js supports two options everywhere where a .type is required:
the string id that will be looked up in the registry
a constructor / function that is used to create the instance directly.

We actually only need the constructor. At least that's how scale registration works now. The id is a static property on the constructor

in the best case, chart.js supports two options everywhere where a .type is required

We had discussed this as an option earlier, but I think the consensus was to only support the string option via registration in v3 because we were worried it would be too confusing to have two ways to do things and would confuse people when they were looking at samples on StackOverflow, etc.

[sgratzl]

I played a bit with tree shaking by manually commenting out things from the build that are not needed for a bar chart and then use rollup to remove all the things that fall of. see https://github.com/sgratzl/chartjs-treeshake-test.

in summary:

• ESM build of charts.js full: 348 kB
• ESM build of chart.js for barchart only: 249 kB

it could be a bit less if the helpers object is not used within the code but just the methods directly. Then, some parts of the helpers object could be shaken out, e.g., in this example splineCurve and so on.

besides removing all the wrappers (elements, plugins, scales, ...) I mostly had to comment out access of the prototypes (e.g., LineController.prototype.dataElementOptions = ...) and manipulating the defaults object, (e..g, defaults.set('line', ....). The former prevents the LineController from being shaken since it is actually used and the later since its config is not needed.

[benmccann]

Wow, that's great! I'm a little surprised that not more was able to be shaken out, but it's a good start. No one will complain about a 30% reduction in build size! If you want to send us any PRs to make these changes we'd be happy to review. (btw, I didn't understand what you meant about wrappers)

Code tends to shake out better when it's functions. We use a lot of classes and those don't shake out well, so I guess that's the core issue. I think there are some additional wins we can get by updating things a bit.

E.g. one I noticed is that the whole color library is getting pulled in and we don't need most of it. We really just use mix in core.animation. It's class-based like the rest of our code and that class depends on most of the other files in the project. However, if we updated it to be just a collection of functions like date-fns then we could shake most of it out. That might require some changes to our API. E.g. we have code to parse any type of color that a user gives to us right now. It might be better to take only rgba and then if a user wants to provide something like hsl, they can import the color library and do the parsing themselves. That would save it from being imported into the build for every user regardless of whether they use that functionality.

Another thing I noticed is that your build had the ResizeObserver polyfill in it. That shouldn't be present in our ES6 builds (we'd expect users to provide their own polyfill if they want to support older browsers), so hopefully we could find a way to remove it as well

[sgratzl]

Wow, that's great! I'm a little surprised that not more was able to be shaken out, but it's a good start. No one will complain about a 30% reduction in build size! If you want to send us any PRs to make these changes we'd be happy to review. (btw, I didn't understand what you meant about wrappers)

Code tends to shake out better when it's functions. We use a lot of classes and those don't shake out well, so I guess that's the core issue. I think there are some additional wins we can get by updating things a bit.

the main question I wonder is how to deal with detault values. e.g. all the defaults.set(...) calls that are not bound to the class and thus not shaken out. Moreover, in two or three locations (legend, title, ...) the defaults.elements.line are used, even tho I don't know how those are related to the context.

E.g. one I noticed is that the whole color library is getting pulled in and we don't need most of it. We really just use mix in core.animation. It's class-based like the rest of our code and that class depends on most of the other files in the project. However, if we updated it to be just a collection of functions like date-fns then we could shake most of it out. That might require some changes to our API. E.g. we have code to parse any type of color that a user gives to us right now. It might be better to take only rgba and then if a user wants to provide something like hsl, they can import the color library and do the parsing themselves. That would save it from being imported into the build for every user regardless of whether they use that functionality.

one could consider switching to a different library such as d3-color or tinycolor. regarding custom parsing, it would require that uses can manipulate the interpolators or the automatic hover color.

Another thing I noticed is that your build had the ResizeObserver polyfill in it. That shouldn't be present in our ES6 builds (we'd expect users to provide their own polyfill if they want to support older browsers), so hopefully we could find a way to remove it as well

if you take a look at https://unpkg.com/chart.js@3.0.0-alpha/dist/Chart.esm.js?module it is part of your esm build. maybe babel injects it because of your browserlist settings.

[kurkle]

if you take a look at https://unpkg.com/chart.js@3.0.0-alpha/dist/Chart.esm.js?module it is part of your esm build. maybe babel injects it because of your browserlist settings.

https://www.chartjs.org/dist/master/Chart.esm.js does not contain it anymore.