Skip to content

Conversation

@lars-sh
Copy link

@lars-sh lars-sh commented Jul 13, 2021

Chart.js seems to use Chart.register, while v2 uses Chart.plugins.register.
This fixes registering the Color Schemes Plugin in Chart.js v3.

Chart.js seems to use `Chart.register`, while v2 uses `Chart.plugins.register`.
This fixes registering the Color Schemes Plugin in Chart.js v3.
@s4m0r4m4
Copy link

s4m0r4m4 commented Oct 5, 2021

I think this is already covered by this one? #26

@lars-sh
Copy link
Author

lars-sh commented Oct 5, 2021

@s4m0r4m4 #26 does not solve the problem addressed by this Pull Request.

While #26 fixes Chart.JS 3 compatibility of the beginUpdate methode, this Pull Request fixes the plugin registration automatism.

@s4m0r4m4
Copy link

s4m0r4m4 commented Oct 5, 2021

Oh I see, you're right. @nagix any chance we can get eyes on this?

@harrytalbot
Copy link

Any update on this @nagix ? We are looking to get on Chart.js v3 but right now this lib is preventing us from upgrading

@choweiyuan
Copy link

Hi @nagix any news on this? would be good to get this out so that we can get colorscheme to work with v3 :)

Comment on lines +184 to 187
// Chart.js v3 uses "Chart.register", while v2 uses "Chart.plugins.register"
const registerPlugin = Chart.register || Chart.plugins.register;
registerPlugin(ColorSchemesPlugin);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a better solution would be to remove the automatic registration or to do it only for v2 users

Suggested change
// Chart.js v3 uses "Chart.register", while v2 uses "Chart.plugins.register"
const registerPlugin = Chart.register || Chart.plugins.register;
registerPlugin(ColorSchemesPlugin);

Other plugins no longer do automatic registration because it registers the plugin globally. It's preferred to be able to register it on a per-chart basis. For example, see https://chartjs-plugin-datalabels.netlify.app/guide/getting-started.html#registration

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable from a developers perspective and I'd be fine with that, too. Though here are some reasons, why it might make sense to stay with the global registration:

  • Pattern of the Zoom Plugin
    I took the Zoom Plugin as it's quite a basic plugin and it's hosted inside the chartjs GitHub organization. They're still registering the plugin globally as of https://github.com/chartjs/chartjs-plugin-zoom/blob/master/src/index.js#L4 .
  • Simple Usage
    Similar to the Zoom Plugin this one is likely to be used by developers with less knowledge. Therefore it might make sense to simplify the getting started process.
  • Backwards Compatibility
    Keeping global registrations allows for in-place upgrades from Chart.js 2.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zoom plugin has two entry points. The UMD entry point auto-registers. The ESM one does not: https://github.com/chartjs/chartjs-plugin-zoom/blob/master/src/index.esm.js

If we want to follow that plugin, then we should have two entry points here as well rather than forcing all users to auto-register

@LeeLenaleee
Copy link

looks like there is some more that needs to be fixed before you can use this with V3, in V3 the way of import Chart.js has changed with Treeshaking so import Chart from 'chart.js' wont work. You need to use treeshaking to import it like so: import { Chart } from 'chart.js'.

Also Chart.helpers does not exist anymore. You need to import the specific helpers with treeshaking from 'chart.js/helpers'

LeeLenaleee added a commit to LeeLenaleee/awesome that referenced this pull request Dec 5, 2021
Colorschemes plugin is not compattible with V3.

Issue about it: nagix/chartjs-plugin-colorschemes#28
Open PR for improving V3 compatability: nagix/chartjs-plugin-colorschemes#30
kurkle pushed a commit to chartjs/awesome that referenced this pull request Dec 6, 2021
Colorschemes plugin is not compattible with V3.

Issue about it: nagix/chartjs-plugin-colorschemes#28
Open PR for improving V3 compatability: nagix/chartjs-plugin-colorschemes#30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants