TypeScript typings

[sgratzl]

Feature Proposal

follow up from #6598 (comment)_

Feature Use Case

provide typescript typings directly as part of chart.js (3)

Possible Implementation

starting points are:

• https://github.com/sgratzl/chartjs-esm-facade/blob/master/src/index.d.ts
• https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/chart.js/index.d.ts

open questions:

• which directory structure should the typings follow: one huge file, per existing source file, in a separate directory, next to the source, ...
• where is a documentation which options are available for different elements/controllers?

[benmccann]

Thanks for leading the charge on this!

which directory structure should the typings follow

I personally don't have a strong opinion on this except to say that I'd like it if we could also add tests for the typings and consider the directory structure of where the tests will go

where is a documentation which options are available for different elements/controllers?

The code might be the best source for this. The only other thing we have is https://www.chartjs.org/docs/master and https://www.chartjs.org/docs/master/typedoc. I believe @stockiNail has been going through and trying to identify places where the docs do not match the code

[sgratzl]

honestly, now sure how to test the typings. There is

• https://github.com/Microsoft/dtslint which tests whether the typings will make sense.
• https://github.com/SamVerschueren/tsd does a similar thing

However, I don't know how to test whether JavaScript matches the TypeScript typings

[benmccann]

I'm not an expert in this area, but here's what I've seen done in one of the plugins: https://github.com/chartjs/chartjs-plugin-datalabels/tree/master/types/test

[jonrimmer]

Awesome that this is happening!

This is my sketch of how the types could potentially be structured: https://github.com/jonrimmer/Chart.js/tree/types-experiment

• Add an index.d.ts to each level of the src hierarchy defining and exporting the equivalent types from that directory's index.js.
• Using rollup-plugin-dts to bundle those .d.ts files into a single index.d.ts file in the dist folder and the package.json's files.

For this I more or less copied-and-pasted and the types from @types/chart.js into src/core/index.d.ts.

[etimberg]

One thing that would be nice in the typings would be to allow templating the data object type. If I know my data is x/y points of numbers, I wouldn't want to have to guard that everywhere. We'd want to provide a good default, but it'd be pretty verbose since we allow a lot as evidenced by data?: Array<number | null | undefined | number[]> | ChartPoint[]; in the existing type definitions. I'm sure this complicates things, but would really help with the usage.

type DataPoint = {
  x: number;
  y: number;
};
type MyDataset = {
  label: string;
  data: DataPoint[];
};
type MyChartData = {
  datasets: MyDataset[];
};

...

const chart = new Chart<MyChartData>(ctx, {
  data,
});

// Now chart.data is of type MyChartData

Another problem I ran into with the existing types is that a lot of things are marked as potentially undefined (e.g. ChartTooltipItem.dataIndex) but I'm pretty sure a number of those can never be undefined given how the code is written.

[etimberg]

In terms of getting types into master, rollup-plugin-dts looks pretty good. My only concern would be with how the options type definition is composed. src/plugins/plugin.tooltip.d.ts would need to define all of the config options for tooltips, but something else would compose them together into the overall chart options by importing the types. I suppose the plugin handles that case, but would be good to confirm

[jonrimmer]

That's a good point. I see three options:

1. Just use a single definition file and no bundling. Simple but a bit unwieldy.
2. Compose the types as suggested. I don't see any reason why this wouldn't work, although it would require breaking things up beyond just the directory-level in order to avoid circular dependencies. I.e. core/Chart.options.legend using plugins/ChartLegendOptions, which in turn imports types like ChartColor from core.
3. Use Declaration merging. So, different files would declare the same interface:

// core/index.d.ts
interface ChartOptions {
  responsive?: boolean;
  // ...
}

// plugins/index.d.ts
interface ChartOptions {
  legend?: ChartLegendOptions;
}

And since these are both bundled into the same file, declaration merging would create a unified interface. Unfortunately, rollup detects the duplicate names and mangles one of them, because it's really intended for bundling JS, where this kind of a collision is a problem.

A workaround is to use module augmentation:

// plugins/index.d.ts
declare module "./chart.js" {
  interface ChartOptions {
    legend?: ChartLegendOptions;
  }
}

The augmentation is emitted as-is into the defs bundle. It's a bit weird for a module to augment itself like this, but it does seem to work.

Another possibility would be to use a non-rollup based tool such as dts-bundle-generator, as I believe it leaves duplicates unchanged.

[sgratzl]

my current state: https://github.com/sgratzl/Chart.js/tree/sgratzl/types/types went through the code and now looking at the documentation for different options

[stockiNail]

@sgratzl I'm working on animation options for another project.

If I may provide a first feedack, having a look to the https://github.com/sgratzl/Chart.js/blob/sgratzl/types/types/core/index.d.ts, I see that the onProgress and onComplete callbacks are defined intoIAnimationPropertySpec but as far as I have tested, you can set them only into IAnimationOptions.
Maybe I'm wrong.

[stockiNail]

@sgratzl another feedback, always about animation.

debug, loop and delay can be set at IAnimationOptions as well, not only at IAnimationPropertySpec.

[sgratzl]

thx, it is work in progress and I appreciate any hints

[kurkle]

Noticed a typo there: (data vs date)

export const _adapters: {
    _data: DateAdapter;
};

[etimberg]

@sgratzl I like the type structure. What do you think the best way to collaborate on the types? We could split the initial types into separate modules + add the build steps then PR that followed by PRs for improvements. Or we could work and get the types all ready in one branch and merge at the end

[sgratzl]

I should be able to finish going through the documentation by the end of today. Unfortunately, I don't get supported financially by anyone for doing things like that. Then I should have a list of all the properties. However, there are still open questions regarding how to mix the options and so on together. Like in the best case, as soon as I say it is a 'bar' controller, it should just suggest me the options that are valid.

Another big questions is how to handle new plugins such that they can contribute to the configuration / options, ...

[etimberg]

Sounds good; I do this in my free time as well, so no rush 😄

Agree that once we say it's a bar controller, we would show the correct options. In my experience with typescript, that means we're going to need a bunch of derived types (not sure how we flow that down to the dataset options either).

That's a great point on plugins. In theory, plugin options could be anywhere which really complicates the types

[sgratzl]

so https://github.com/sgratzl/Chart.js/blob/sgratzl/types/types now contains all possible options I could find.

However, what is missing is how to merge them to create one configuration

[sgratzl]

https://github.com/chartjs/Chart.js/pull/7668/files#diff-7a581ab159bd53d91fc2a62a45759557 is a working version how the typing could work

[Cristy94]

Isn't Chart.js v3 written in TypeScript? Is there any way to currently get typings for v3?

[benmccann]

No, it's not written in TypeScript, but some types were recently added: #7668

[Cristy94]

Oh, I'm a bit disappointed to hear it was not written in TS, I thought that was a big advantage of the rewrite.

Can I use those typings through npm or do I have to manually copy the d.ts files?

[sgratzl]

when you install the upcoming version of chart.js via npm, the typings should just work.

[Cristy94]

when you install the upcoming version of chart.js via npm, the typings should just work.

You are referring to the v3-alpha-2? Because that doesn't have typings. Or do I have to wait for the next alpha release?

[etimberg]

No, the typings were merged after v3.0.0-alpha.2 released. I don't have a specific date targeted for the next release, but my gut says early September for a beta.1.

To clarify as well, v2 -> v3 is not really a rewrite. While bits and pieces were rewritten (e.g. animations), by and large the v3 code is the v2 code. The major version bump comes from making a number of backwards incompatible changes to the config options.

[sgratzl]

do overcome some issue in the current alpha version I created https://github.com/sgratzl/chartjs-esm-facade which also has the same Typescript typings

[Cristy94]

I tried again upgrading now that 3.0.0-beta was released and typings are included, but after spending 1 hour trying to convert a single time-series chart I gave up. It keeps giving strange TypeScript errors that are hard to understand, and even while looking at the migration guide it is so hard to understand the correct format of the new chart options.
Also, the typings feel very poor as VSCode doesn't show any autocomplete even for the chart type (it's defined as string, not as a the string literals union linear | bar | doughnut ...).

One example that makes migration really hard is for example that chart.options no longer exists and is now only accessible via chart.config.options (chart.options still exists, but has different data?). This change doesn't seem to be documented in the migration guide. Maybe only the typings are wrong?

Anyway, I do not recommend trying to update to v3.0-beta as the migration is really painful (I reverted my changes, couldn't make it work in 1 hour).

[benmccann]

the typings feel very poor as VSCode doesn't show any autocomplete even for the chart type (it's defined as string, not as a the string literals union linear | bar | doughnut ...)

Chart.js is made to work with plugins. I'm not sure we could define it as a union of literals because then you probably couldn't use a plugin chart like https://github.com/chartjs/chartjs-chart-financial

[etimberg]

@Cristy94 see #7767 for improvements around the chart type.

Essentially its now an enum and you can extend the enum as shown from the discussion in the PR.

[benmccann]

Cool! I'll be excited to try these out! We should probably add some tests for these at some point as well

[santam85]

Can someone give me some pointers on what is the correct typing to use for Chart.js v3 configuration options and data? The flurry of generics is really confusing me...

[etimberg]

With the latest beta release (v3.0.0-beta.3) this is the typing I am using in my typescript projects with regards to datasets.

import { IChartDataset } from 'chart.js';

const dataset: IChartDataset<"line"> = {
  // Validates that properties here match those expected for a line dataset.
};

@sgratzl @kurkle do you think we can close this issue now?

[joshkel]

Our application has been using Chart.js 3.0.0 alpha 1 with some hacked-together type definitions based on @types/chart.js, so I was excited to see that TypeScript type definitions will be built in. I've started looking at upgrading to the latest beta and switching to the build-in definitions. I'm really impressed with how thorough the definitions are and how well they support the various plot types and support extensibility. I have some questions and feedback.

.d.ts organization: I'm in the habit of using WebStorm's "Go To Declaration or Usages" hotkey with third-party TypeScript definitions as a quick way of getting more information about an API. With the beta's definitions, that works very poorly; WebStorm goes to the imported-and-re-exported symbol within the barely-readable machine-generated dist/chart.esm.d.ts file, and I have to navigate two more hops to find the actual definitions.

Does Chart.js need to bundle its .d.ts files? In other words, instead of rolling up the types directory into several files under dist and adding "types": "dist/chart.esm.d.ts" to package.json, it seems simpler to include the types directory as is within the release and add "types": "types/index.esm.d.ts" to the package.json. (I don't have much experience in packaging libraries for others' use, so I may be missing something, but this seems to work from a brief test I did, and it seems to match other projects' approach.)

Type names: I noticed that some of the type names are different than the third-party @types/chart.js. This seems fine to me - if the project is going to break any compatibility with type names, now seems like the time to do it, but I wanted to double-check that everyone's okay with this.

(As long as I'm nitpicking, there is one place where I prefer @types/chart.js's names - it uses ChartPoint, while the new type definitions use IScatterDataPoint. Something more generic like ChartPoint or DataPoint seems preferable, since it's used for line charts as well as scatter charts.)

Type naming conventions: The beta's type definitions use the convention of I to mark interfaces. I realize that naming conventions are subjective and there's enormous room here for bikeshedding, but I really think that libraries for TypeScript should avoid this convention. A strong distinction between classes and interfaces seems less useful in TS, given its reliance on structural typing and object literals. And the TypeScript community seems to strongly prefer avoiding it (for example, TypeScript's internal coding standards, the excellent third-party TypeScript Deep Dive, major projects like Angular and Material-UI, (formerly) typescript-eslint's defaults, online discussions, etc.). There's a lot of value in following community conventions.

And, now that I've stated my (strong) preference and reasoning, I'll defer to whatever you decide, since it's a subjective topic. 🙂

Specific questions: As part of updating my project, I've run into several problems or questions with the new type definitions. What's the best way to handle these (post a comment here, open a new issue listing all of them, open a separate issue for each, open a PR with proposed fixes for discussion, ...)?

• Defaults.maintainAspectRatio (and other Defaults fields) are readonly. I thought that the purpose of these fields was to allow developers to override them?
• Several properties of the Defaults object are undefined: animation, elements, layout, line, showLines
• Something about ITooltipCallbacks prevents type inference from working for callback arguments. I'm not sure why. There's an easy workaround (add an explicit type to the callback's parameters), so this doesn't seem important.
• The tooltip callbacks are documented as taking a second parameter, data, but the type definitions don't allow that. If I understand the implementation correctly, the type definitions are correct here, and the docs are incorrect.
• Line datasets' tension has been renamed to lineTension.
• The new normalized option isn't supported. (However, from looking at the implementation, I'm unclear on how normalized works; the docs make it sound like it's for datasets, but the only reference I could find in the implementation was an option for a time series scale. Should I file a separate docs issue for this?)
• The type definitions mark IChartData's labels property as required, but in practice, it appears to be optional.

I'm happy to open a PR for any of the above if it would help.

Thanks for your work on this. It's great stuff.

[etimberg]

@joshkel thanks for the feedback!

Bundles

I have no strong preference on this. If we can avoid a build step, that's always nice

Type Names / Conventions

For the points, I believe the idea was that different chart types hence why they have different names.
For interfaces, I am on board with following the community, but before sending a PR I would recommend we get some consensus (@kurkle, @emmcbd, @sgratzl)

Specific Questions

• I don't recall why the defaults are marked as readonly
• Sounds like those defaults should be added
• This could use some investigation. I'm sure we'll get others noticing this too
• the tooltip callbacks are incorrect. I believe data was removed when chart was added to ITooltipItem
• the line typings should be updated for the new tension property
• normalized is used for the scale. With the time scale, we have to sort the data to know the limits. normalized prevents that. I'm sure the docs could use an upgrade too
• labels is not required. Should check that scales have optional labels defined. I know the category scale definitely supports it and likely all cartesian axes too

[benmccann]

I'd prefer to drop the I prefix as well

[santam85]

I see there was some discussion around plugin options and how to leave the configuration types extensible, but couldn't understand what was the final decision there. At the moment, with the current types, the compiler would throw an error when trying to pass plugin options.

[simonbrunel]

@etimberg I'm looking at the PR submitted to port the datalabels pulgin to Chart.js v3 and I strongly agree that interfaces should not be prefixed with I.

[etimberg]

Good feedback @simonbrunel. Let's fix this before v3 releases

[simonbrunel]

About generic names (seen here): I'm not sure that's a common practice to write generic parameter full uppercase (whatever language actually, e.g. template in C++). I think uppercase are mostly understood as constants or defines. I like the explicit names, but I think Type, Data, Label or probably better TType, TData, TLabel (as in TypeScript code itself) are more suitable. Uppercase names may also not be accepted once TS code will be linted in the main repo (it would expect CamelCase) and it's confusing with constants.

[etimberg]

Going to close this and use smaller tickets to address future issues. The types in v3 now are a big improvement over the v2 types.