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

DecimationAlgorithm enum not found #8993

Closed
Nico-DF opened this issue Apr 28, 2021 · 17 comments · Fixed by #9010 or #9046
Closed

DecimationAlgorithm enum not found #8993

Nico-DF opened this issue Apr 28, 2021 · 17 comments · Fixed by #9010 or #9046
Labels
type: bug type: types Typescript type changes
Milestone

Comments

@Nico-DF
Copy link
Contributor

Nico-DF commented Apr 28, 2021

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

@kurkle kurkle added the type: types Typescript type changes label Apr 28, 2021
@etimberg
Copy link
Member

etimberg commented May 1, 2021

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,
      }
    }
  }
});

@etimberg etimberg added this to the Version 3.2.1 milestone May 1, 2021
@etimberg etimberg linked a pull request May 1, 2021 that will close this issue
@Nico-DF
Copy link
Contributor Author

Nico-DF commented May 4, 2021

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
Copy link
Contributor Author

Nico-DF commented May 4, 2021

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

@kurkle
Copy link
Member

kurkle commented May 4, 2021

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 kurkle reopened this May 4, 2021
@kurkle
Copy link
Member

kurkle commented May 4, 2021

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

@Nico-DF
Copy link
Contributor Author

Nico-DF commented May 4, 2021

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
Copy link
Member

kurkle commented May 4, 2021

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
Copy link
Contributor Author

Nico-DF commented May 4, 2021

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
Copy link
Member

etimberg commented May 4, 2021

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
Copy link
Contributor Author

Nico-DF commented May 4, 2021

Indeed, the type will now match.

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

@etimberg
Copy link
Member

etimberg commented May 7, 2021

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

@kurkle
Copy link
Member

kurkle commented May 7, 2021

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
Copy link
Member

kurkle commented May 7, 2021

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

image

@etimberg
Copy link
Member

etimberg commented May 7, 2021

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
Copy link
Member

kurkle commented May 7, 2021

@etimberg, you probably have some old types in the build folder, dating before #8720 :)

@etimberg
Copy link
Member

etimberg commented May 7, 2021

that could be it

@lucianosantana
Copy link
Contributor

@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!

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
4 participants