BREAKING Tree shakable standalone library

[PowerKiKi]

The library is standalone and tree-shakable, so it introduces the following breaking changes:

• All usage of NgChartsModule must be replaced by provideNgCharts() (and withDefaultRegisterables() and optionally withColorGenerator())

@santam85, this is a draft to transform the library into a pure standalone lib. My goal was to be standalone and tree shakable, hence the introduction of withColorGenerator() (which I'd like to avoid in my own projects).

It is still WIP, because I didn't do much for schematics. I see they are a lot of them to add each possible chart. IMHO that is a bit overkill, and I'd drop support for adding chart entirely. And instead, I'd only keep adding the library to an existing standalone project (not a module project). It is a severe breaking change, but would reduce maintenance drastically. WDYT ?

tl;dr: new usage is like:

provideCharts(withDefaultRegisterables(), withColorGenerator())

[santam85]

Thanks @PowerKiKi , I agree all the schematics are overkill, I am not opposed to remove the generation ones. As a compromise, we could probably keep a single one with a type parameter to generate a new chart?

IIRC the color generation feature is now something directly supported from Chart.js (https://www.chartjs.org/docs/latest/general/colors.html#default-color-palette) so we could potentially rework that option to include/exclude the extra registrable.

[PowerKiKi]

I am not opposed to remove the generation ones

I'll remove all of them then. Because generating hardcoded values does not bring much value, and we already have well documented usages that can be copy-pasted on https://valor-software.com/ng2-charts/

the color generation

I assumed that the hardcoded colors from this lib (which differ from chart.js values), and the generated colors, were something that should be kept. But if that is not the case, and we agree that colors will change in this release, then I agree it would make more sense to drop it entirely and instead recommend something like https://github.com/kurkle/chartjs-plugin-autocolors. Would that be ok with you ?

[santam85]

Rather than suggesting en external library ('autocolor' from https://github.com/kurkle/chartjs-plugin-autocolors) I would prefer using the built-in one already present in Chart.js.

It should be possible to test it in our lib by adding these defaults:

import { Colors } from 'chart.js';

Chart.register(Colors);

and for the charts:

  plugins: {
    colors: {
      enabled: true
    }
  }

[PowerKiKi]

Actually the built-in Colors plugin will "will cycle through a palette of seven Chart.js brand colors". To double check that statement I made a StackBlitz with pure Chart.js with 14 datasets, and clearly colors are not auto-generated but instead cycled:

So what should we do ? drop and recommend that [autocolors](https://github.com/kurkle/chartjs-plugin-autocolors) plugin ? drop and don't recommend anything ? keep something in our lib ?

[santam85]

My opinion is we should:

• drop color generation (we start with a palette of 12 before generating too)
• default to registering and enabling Colors from Chart.js as it does not require any extra dependency, state in the release notes that the palette only has 7 colors
• Point people in need of more colors to the autocolor plugin if they need color generation, but avoid adding it to our dependencies, add a sample in the docs app on how to configure it

This can be part of a separate PR too though...

[PowerKiKi]

This is ready for review (and merge). I'll address the color thing in a separate PR.

[santam85]

Nice work, thanks for the help!
Can you take care of updating the changelog as well for the library please? That way we can start tracking the breaking changes for 6.0.0

[santam85]

It's not mandatory to use the directive in a standalone component right? Should we make that explicit?

[PowerKiKi]

Documented in 7b0bfa8

[santam85]

Isn't there a more restrictive type we could use for defaults in Chart.js?

[PowerKiKi]

I wish, but unfortunately Chart.js typing is quite loose for that method:

https://github.com/chartjs/Chart.js/blob/767d64e7a90dbfe94f9c5d159a890054818c1680/src/types/index.d.ts#L673

[santam85]

Can you please also add a config example with a custom registerables list as you did in the docs app?

[PowerKiKi]

Done in 0d894e5

[santam85]

fixes #1771 , #1360

[PowerKiKi]

I did everything you asked, except updating CHANGELOG.md, because that file was not kept up to for 4.0.1, 4.0.2, 4.1.0, 4.1.1, 5.0.0, 5.0.1, 5.0.2, 5.0.3 and 5.0.4. I'd rather not do that 😅. Instead I would suggest to delete CHANGELOG.md and only use GitHub releases, which are up-to-date and easily generated. I know it's slightly less convenient for users, but CHANGELOG.md maintenance is a pain. And i'd rather have a systematically up-to-date non-file changelog in GitHub, rather than incomplete CHANGELOG.md file.

[PowerKiKi]

IMHO this is ready to be merged. Would you like me to rebase/squash ?