chartjs / Chart.js

Simple HTML5 Charts using the <canvas> tag
https://www.chartjs.org/
MIT License
64.69k stars 11.92k forks source link

DecimationAlgorithm enum not found #8993

Closed Nico-DF closed 3 years ago

Nico-DF commented 3 years ago

Expected Behavior

I am no pro at Angular/JS, but by seeing the d.ts file and how the decimationOptions are defined, I expect the DecimationAlgorithm enum to be exported

export declare enum DecimationAlgorithm {
  lttb = 'lttb',
  minmax = 'min-max',
}

And that, when declaring options, I can use the following:

plugins: {
  decimation: {
    enabled: true,
    algorithm: DecimationAlgorithm.lttb
  }
}

Current Behavior

https://codepen.io/Nico-DF/pen/WNRWJQr

"export 'DecimationAlgorithm' was not found in 'chart.js' error Command failed with exit code 1.

Maybe it is a misunderstanding on my side, but currently, I'm forced to do the following:

plugins: {
  decimation: {
    enabled: true,
    // @ts-ignore
    algorithm: 'lttb'
  }
}

The ts-ignore is needed due to the definition of the option

etimberg commented 3 years ago

The DecimationAlgorithm type is exported. https://github.com/chartjs/Chart.js/blob/master/types/index.esm.d.ts#L1955-L1958

Do you have a reproduce that uses TS? I created this test case and it worked as expected

import { Chart, DecimationAlgorithm } from '../../../index.esm';

const chart = new Chart('id', {
  type: 'bubble',
  data: {
    labels: [],
    datasets: [{
      data: []
    }]
  },
  options: {
    plugins: {
      decimation: {
        algorithm: DecimationAlgorithm.lttb,
      }
    }
  }
});
Nico-DF commented 3 years ago

Sorry for the delay, took a few days off, the codepen exhibits the not found exception, and for the TS, something like this does not work (which I agree is weird as I also saw the export on d.ts file)

import { ChartData, ChartDataset, ChartOptions, DecimationAlgorithm } from 'chart.js';

public static buildChartOptions(
    xLabel: string,
    yLabel: string,
    minX: number,
    maxX: number,
    zoomCallback: () => void
  ): ChartOptions {
    return {
      // We are using objects like {x: 10, y: 20}, disabling parsing for performance
      parsing: false,
      // Data are already ordered
      normalized: true,
      animation: false,
      responsive: false,
      scales: {
        y: {
          title: {
            display: true,
            text: yLabel
          },
          type: 'linear',
          ticks: {
            minRotation: 50,
            maxRotation: 50
          }
        },
        x: {
          title: {
            display: true,
            text: xLabel
          },
          type: 'linear',
          min: minX,
          max: maxX,
          ticks: {
            minRotation: 50,
            maxRotation: 50
          }
        }
      },
      plugins: {
        decimation: {
          enabled: true,
          algorithm: DecimationAlgorithm.lttb
        },
        legend: {
          align: 'start'
        },
        zoom: {
          limits: {
            x: { min: minX, max: maxX }
          },
          pan: {
            enabled: true,
            mode: 'x'
          },
          zoom: {
            enabled: true,
            mode: 'x',
            onZoom: zoomCallback
          }
        },
        tooltip: {
          mode: 'index',
          intersect: false
        }
      }
    };
  }

This functions creates for me the ChartOptions and I use it to build several different graphs. Using the enum will fail with "export 'DecimationAlgorithm' was not found in 'chart.js' (On IDE chart.js refers correctly to index.esm.d.ts)

Nico-DF commented 3 years ago

I could try to setup a mock project and post the link here if you want (probably not before tomorrow though)

kurkle commented 3 years ago

Here's a repro: https://codesandbox.io/s/competent-hooks-bi2i7?file=/src/index.ts

The issues is, chart.js is written in javascript, so the exported typescript enum never gets compiled and transpiled to an object, as it would in a typescript project. So it really is undefined.

The type checking works fine though, so you can just use 'lttb'.

~I think we should stop exporting the enums, because those are not really usable.~

Edit: The enums are usable for augmentation

I think we should do the same as with UpdateMode:

export declare enum UpdateModeEnum {
  resize = 'resize',
  reset = 'reset',
  none = 'none',
  hide = 'hide',
  show = 'show',
  normal = 'normal',
  active = 'active'
}

export type UpdateMode = keyof typeof UpdateModeEnum;

const blah = UpdateModeEnum.hide; still does not work (for the same reason, the enum is never compiled to the lib), but const blah: UpdateMode = 'active' does, as its just type checking.

kurkle commented 3 years ago

And another note, seems like there is a better solution, by using const enum: https://stackoverflow.com/a/50568865/10359775

Nico-DF commented 3 years ago

Thanks for the update @kurkle, I agree that using 'lttb' works (it is what I'm doing right not), but if using a strict check with tslint, it will fails as string != DecimationAlgorithm, that was basically my point

kurkle commented 3 years ago

Sorry for missing the actual point, but I think the const enum could be an answer to that too. Unless you then really need to use something called module isolation and that is not really possible in all cases? (still really new to TS)

Nico-DF commented 3 years ago

Didn't saw the last part of comment, I think that indeed it might do the trick, thanks for the help, didn't think of it (still pretty new too)

But nonetheless, I either missed a thing or the generation is effectively clunky as I agree with @etimberg that I also see the export declare enum DecimationAlgorithm in d.ts file, so I let you guys confirm whether or not there is an issue with how enums are transcompiled

etimberg commented 3 years ago

I agree that using 'lttb' works (it is what I'm doing right not), but if using a strict check with tslint, it will fails as string != DecimationAlgorithm, that was basically my point

I believe the string was fixed in #9010.

Nico-DF commented 3 years ago

Indeed, the type will now match.

Concerning the enum export, I'll trust you on what to do with this Issue

etimberg commented 3 years ago

I tried out the export const enum solution locally, but the output .d.ts files appear identical.

kurkle commented 3 years ago

What do you mean by "output .d.ts"? We are shipping the definitions as is now, if I'm not mistaken. The result should be only visible in a typescript project using the types, where the typescript compiler would convert the enum values to strings in the const case. At least this is my understanding, but I'll verify it.

kurkle commented 3 years ago

Diff of the .js output of npx tsc decimation_algorithm.ts by changing declare to const:

image

etimberg commented 3 years ago

I meant the built files that we generate as a result of npm run build. I didn't see any changes related to the DecimationAlgorithm enum when it was changed to export const enum.

kurkle commented 3 years ago

@etimberg, you probably have some old types in the build folder, dating before https://github.com/chartjs/Chart.js/pull/8720 :)

etimberg commented 3 years ago

that could be it

lucianosantana commented 2 years ago

@etimberg sorry for the ping, but since you were the one approving the other PR and this follows the same pattern, would you mind reviewing the PR I've just prepared. Thanks!