Export plugin as an ESM compatible module

[santam85]

Avoids getting errors like:
Uncaught TypeError: helpers$1.merge is not a function
when loading the plugin as a module.

[santam85]

@simonbrunel if you like this PR, and decide to merge it, could you consider releasing the target branch under the next label, in order to enable users of ng2-charts to start testing a v3.0 compatible beta?

[simonbrunel]

@santam85 Thanks you for your help in migrating this plugin.

I can't guarantee that I will release a beta right after but I will try to do my best to have it released soon. The documentation will also need to be updated to reflect Chart.js 3 changes about registering plugins.

[santam85]

@simonbrunel latest beta broke several tests, any clue what the cause might be? Can't link these errors to any of the listed breaking changes...

[kurkle]

@santam85 its mostly the proxied options that default to scriptable being true, and thus tries to resolve the event handlers etc.

Adding something like that to the plugin should help (from #221):

  descriptors: {
    _scriptable: function(name) {
      return !['formatter', 'enter', 'leave', 'click'].includes(name);
    },
  },

[simonbrunel]

@santam85 This plugin doesn't rely on proxied options (and I'm not sure I would like to) so it's probably better to fully disable this feature for now (_scriptable: false if supported) and keep the explicit resolve calls.

@kurkle Shouldn't proxied options for plugins default to false? The current behavior seems quite error prone and hard to debug and will probably break a lot of existing plugins.

[kurkle]

@simonbrunel yes, it should default to false for plugins.

[kurkle]

So, to make this work with beta.11 you'll need to add this:

  descriptors: {
    _scriptable: false,
    listeners: {
      _scriptable: false
    }
  },

And this should not be needed after next release.

Some docs samples are broken, this needs to be investigated before merging.

[simonbrunel]

@santam85 A few samples are broken compared to the chartjs-next branch:

• Charts/Line: chartjs-next | PR
• Charts/Bar: chartjs-next | PR
• Events/Highlight: chartjs-next | PR
• Events/Highlight: chartjs-next | PR
• Maybe more...

[kurkle]

Looks like the samples issue persists, I'll take a closer look at it.

[kurkle]

This is probably an issue with the types in beta.12

[simonbrunel]

If the generic base type is still needed, maybe the core could export a nice alias:

// chart.js
export type AnyArray = unknown[];

// chart-plugin-datalabels
import { AnyArray } from 'chart.js';

declare module 'chart.js' {
  interface ChartDatasetProperties<TType extends ChartType, TData extends AnyArray> {
    //...
}

[simonbrunel]

Could even be AnyData instead of AnyArray.

[santam85]

I am just matching the definition here to prevent errors. If only array data-types are needed, would TData extends Array<any> work? Or even ChartDatasetProperties<TType extends ChartType, Array<any>>?

[simonbrunel]

I think it needs to be the exact same definition as in the core repo, so it would not work. Also, I disabled the use of any and enforce array type consistency in this project, so it would need to be unknown[].

[simonbrunel]

@santam85 I think I misunderstood your previous comment. Are you suggesting to use Array<any> in the core repo?

If so, I would not go that way because any isn't good for typing, unknown is better because it forces a check of the actual type before using it. Using an alias such as AnyData would allow to extend this data type later (if needed) without breaking existing integrations (e.g. type AnyData = AnyArray | AnyObject) or type AnyData = (number | string | DataObject)[], etc.).

[santam85]

Yeah, I meant in the core repo. Sounds good, will adjust accordingly once decided.

[simonbrunel]

Thanks @santam85.

Samples are still broken: legend and tooltip should not be visible. It looks like an issue in the core lib where registering a plugin (and probably other types) doesn't preserve existing defaults:

import {Chart, registrables} from 'chart.js';

Chart.defaults.set({
  plugin: {
    legend: {
      display: false;
    }
  }
})

Chart.register(...registrables);

// Chart.defaults.plugin.legend.display === true

I will see how to fix the docs after merging this PR.