Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to disable global plugins in typescript #11398

Closed
stockiNail opened this issue Jul 12, 2023 · 4 comments · Fixed by #11403
Closed

Unable to disable global plugins in typescript #11398

stockiNail opened this issue Jul 12, 2023 · 4 comments · Fixed by #11403
Labels
type: bug type: types Typescript type changes

Comments

@stockiNail
Copy link
Contributor

Expected behavior

As reported from the doc, how to disable plugins https://www.chartjs.org/docs/latest/developers/plugins.html#disable-plugins, you can disable the invocation of a plugin registered globally setting in the chart options as false.

Chart.register({
    id: 'p1',
    // ...
});

const chart = new Chart(ctx, {
    options: {
        plugins: {
            p1: false   // disable plugin 'p1' for this instance
        }
    }
});

The same when you want to disable all plugins, as reported to the doc:

const chart = new Chart(ctx, {
    options: {
        plugins: false // all plugins are disabled for this instance
    }
});

Current behavior

When you use TS, you can not disable the plugins as documented, getting the error:

index.ts:28:4 - error TS2559: Type 'false' has no properties in common with type '_DeepPartialObject<LegendOptions<"bar">>'.

28    legend: false,
      ~~~~~~

  node_modules/chart.js/dist/types/index.d.ts:2909:3
    2909   legend: LegendOptions<TType>;
           ~~~~~~
    The expected type comes from property 'legend' which is declared here on type '_DeepPartialObject<PluginOptionsByType<"bar">>'

Reproducible sample

https://github.com/stockiNail/chartjs-11288

Optional extra steps/info to reproduce

  1. edit index.ts
  2. set
options: {
   plugins: {
     legend: false,
   }
 }
  1. execute npm run test

Possible solution

Add to PluginOptionsByType interface items the options to be set to false.
Example:

export interface PluginOptionsByType<TType extends ChartType> {
  colors: ColorsPluginOptions | false;
  decimation: DecimationOptions | false;
  filler: FillerOptions | false;
  legend: LegendOptions<TType> | false;
  subtitle: TitleOptions | false;
  title: TitleOptions | false;
  tooltip: TooltipOptions<TType> | false;
}

Add to PluginChartOptions interface plugins items the options to be set to false.

export interface PluginChartOptions<TType extends ChartType> {
  plugins: PluginOptionsByType<TType> | false;
}

Context

No response

chart.js version

v4.3.0

Browser name and version

FF 115.0.1

Link to your project

https://github.com/stockiNail/chartjs-11288

@LeeLenaleee LeeLenaleee added the type: types Typescript type changes label Jul 12, 2023
@stockiNail
Copy link
Contributor Author

I have done a quick drill-down with the proposed solution but I see some issues.

Test definitions:

interface TitleOptions {
  display: boolean;
}

interface PluginOptionsByType {
  title: TitleOptions | false;
}
interface PluginChartOptions {
  plugins: PluginOptionsByType | false;
}

interface Defaults extends PluginChartOptions {
}

class Chart {
  static readonly defaults: Defaults;
}

Chart.defaults.plugins = false; // works
Chart.defaults.plugins.title = false; // issue: Property 'title' does not exist on type 'false | PluginOptionsByType'.
                                      //        Property 'title' does not exist on type 'false'.(2339)
Chart.defaults.plugins.title.display = false; // issue: Property 'title' does not exist on type 'false | PluginOptionsByType'.
                                              //        Property 'title' does not exist on type 'false'.(2339)

More time is needed to me (due to my lack of knowledge on TS) :(

@stockiNail
Copy link
Contributor Author

It seems a casting is needed...

(Chart.defaults.plugins as PluginOptionsByType).title = false;
((Chart.defaults.plugins as PluginOptionsByType).title as TitleOptions).display = false;

If so, it could be a breaking change to whoever is accessing to plugin options.

@stockiNail
Copy link
Contributor Author

It does not make sense to set false to defaults because otherwise we could loose the defaults of plugin.
Therefore we should duplicate the plugin interfaces, for defaults and for chart options.
This should be a breaking change I guess.
I’m going to submit a PR soon.

@stockiNail
Copy link
Contributor Author

Because of #11422 which reverted the changes, I would suggest to re-open the issue, in order to avoid to forget it and maybe we could tag it as breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug type: types Typescript type changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants