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

Types appear to be wrong for data #9688

Closed
rhysviz opened this issue Sep 28, 2021 · 5 comments · Fixed by #9917
Closed

Types appear to be wrong for data #9688

rhysviz opened this issue Sep 28, 2021 · 5 comments · Fixed by #9917

Comments

@rhysviz
Copy link

rhysviz commented Sep 28, 2021

Expected Behavior

I expect to be able to type the data for a bar chart as (number | null)[] and have it not error.

Current Behavior

I get the following error:

Type 'ReportingData<number | null>' is not assignable to type 'ChartData<"bar", number[], unknown>'.
  Types of property 'datasets' are incompatible.
    Type '{ data: (number | null)[]; label?: string | undefined; }[]' is not assignable to type 'ChartDataset<"bar", number[]>[]'.
      Type '{ data: (number | null)[]; label?: string | undefined; }' is not assignable to type 'ChartDataset<"bar", number[]>'.
        Type '{ data: (number | null)[]; label?: string | undefined; }' is not assignable to type 'ChartDatasetProperties<"bar", number[]>'.
          Types of property 'data' are incompatible.
            Type '(number | null)[]' is not assignable to type 'number[]'.
              Type 'number | null' is not assignable to type 'number'.
                Type 'null' is not assignable to type 'number'.ts(2322)
index.esm.d.ts(3486, 3): The expected type comes from property 'data' which is declared here on type 'ChartConfiguration<"bar", number[], unknown>'

I think null is valid because of the skipNull option, and it's useful for removing data in a stacked bar chart.

Example

https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbzgYQBYENYBo5QKYDmwAzjHlOgEYA2exOamMyEAdgGbAECuFMwbBhlgARdDHRwAvnHZQIIOAHIAxsJgA6AFbElAbgBQBlW1JwQATzESAXHAAUrbiErk4AHzhPq1AJQBtAF04AF44fwBGLAAmLABmLG9qQMMTVjNLa3RiPBhiUKQAE3Fs3OI7fwRi23MrEqlAnGoqPGpy8KUIpSwlaO6lOP6AFiVgqUNjU3gCXJYOLlCHXztGWDnOHj4BVgAeJUpMJQA+UJOEAzhL3FzeVkQLq8fHmAswPDt9w6wHp6fq9DsmRKOTyPyeUjBEKAA

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

A solution for this would be to set the 2nd parametrized type for ChartConfiguration to the data type you're using. I'm not sure if we can make the types auto detect skipNull or it we should add it to the defaults. @kurkle any thoughts?

import { Chart, registerables, ChartConfiguration, ChartData } from 'chart.js';

const myData: (number | null)[] = [1,2,3,null];
const myDatasets ={ datasets: [{data: myData}], labels: ['1','2','3','4'] };

const getConfig = (): ChartConfiguration<'bar', (number|null)[]> => {
    return {
            type: 'bar',
            data: myDatasets
        }
    }

@kurkle
Copy link
Member

kurkle commented Oct 13, 2021

The pie animation type was fixed by #9699

@kurkle
Copy link
Member

kurkle commented Oct 13, 2021

The downside for allowing null in the types by default is: when accessing the data array, for example from a scriptable option, the value can be null.

@etimberg etimberg added this to the Version 3.6.1 milestone Oct 23, 2021
@etimberg
Copy link
Member

I put this into the 3.6.1 milestone. I think a solution here is to add some documentation on the types. I don't think we want to change the default type for the reasons @kurkle mentioned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants