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

Please add TypeScript definitions #20

Closed
JannesMeyer opened this issue May 16, 2018 · 25 comments · Fixed by #97
Closed

Please add TypeScript definitions #20

JannesMeyer opened this issue May 16, 2018 · 25 comments · Fixed by #97
Assignees

Comments

@JannesMeyer
Copy link

No description provided.

@KacperMadej
Copy link
Contributor

Hi @JannesMeyer

For the wrapper?

If, for the Highcharts then please refer to this issue - highcharts/highcharts#4876

@JannesMeyer
Copy link
Author

JannesMeyer commented May 17, 2018

Hi @KacperMadej !

Yes, for the props of the HighchartsReact component. (options, highcharts, constructorType, etc...)

I see in the other issue that you are unable to provide type definitions for the options, but you could already provide types for the other two props.

Of course, the main issue is adequate typing for the options prop...

@JannesMeyer
Copy link
Author

JannesMeyer commented May 17, 2018

By the way, the company I work for has a single developer license for the Highcharts Suite with the Premium Support.

Let me know if you want me to email the proof of license.

@KacperMadej
Copy link
Contributor

For technical support please use support channels - https://www.highcharts.com/support

It would be for the best to keep issues in github for bugs - at least this it what we are trying to do.

@JannesMeyer
Copy link
Author

Ok, I have written an email to support@highcharts.com requesting this enhancement.

Sorry for using the wrong channel.

@KacperMadej
Copy link
Contributor

Not a problem. I'm keeping this issue open and labeled as an enhancement.

@BrandonWilliamsCS
Copy link

I created a types file for my personal use; hopefully it serves as a good starting point for someone to do so more formally (whether part of the package or in DefinitelyTyped). However, some work definitely needs to be done before Highstocks and Highmaps are covered.
It's short, so I'll just paste the contents @types/highcharts-react-official/index.d.ts (where @types is parallel to node_modules).

declare module "highcharts-react-official" {
    import * as React from "react";
    import * as Highcharts from "highcharts";

    /**
     * Represents a "constructor" function for a ChartObject.
     * Not technically a constructor, as it's not called with `new`.
     */
    type Constructor = (
        renderTo: HTMLElement,
        options: Highcharts.Options,
    ) => Highcharts.ChartObject;
    /**
     * Represents all of the strings that are keys of T that map to a Constructor function.
     */
    type ConstructorName<T> = {
        [K in keyof T]: T[K] extends Constructor ? K : never
    }[keyof T];

    export interface HighchartsReactProps<TContainerProps = {}> {
        highcharts?: Highcharts.Static;
        constructorType?: ConstructorName<Highcharts.Static>;
        update?: boolean;
        options: Highcharts.Options;
        containerProps?: TContainerProps;
    }
    export class HighchartsReact<TContainerProps = {}> extends React.Component<
        HighchartsReactProps<TContainerProps>
    > {}

    export default HighchartsReact;
}

@dandv
Copy link

dandv commented Jul 18, 2018

See #36. Official definitions are expected in Q3.

@JonasGilg
Copy link

JonasGilg commented Oct 1, 2018

As specified in #36 the official definitions are only for the normal highcharts api. So we still need official definitions for highcharts-react.

@JannesMeyer
Copy link
Author

I agree with @TheJhonny007. Pull request #36 is only for the options attribute. Like I mentioned in my first comment, there would still be the need for types for the rest of the HighchartsReact component.

@JannesMeyer
Copy link
Author

Sorry, I just realised that the rest of the component's types are included in the pull request. Nevermind!!

@JonasGilg
Copy link

@JannesMeyer in which pull request? #36 was rejected and closed.

@bre1470
Copy link
Member

bre1470 commented Oct 2, 2018

Hello! We will also work on declarations for Highcharts React. Development will start with the end of the beta phase for Highcharts declarations.

@bre1470 bre1470 self-assigned this Oct 2, 2018
@JonasGilg
Copy link

Thank you, that was the answer that I think we all wanted. 🙂

@jonfreedman
Copy link

Is there any ETA on this?

@bre1470
Copy link
Member

bre1470 commented Nov 26, 2018

No there is no ETA for this yet, because development has not started. First the Highcharts declerations have to be stable and generally available.

@bre1470
Copy link
Member

bre1470 commented Nov 26, 2018

I expect though that the support will be quite fast implemented, once the Highcharts declarations are leaving the beta state.

@jonfreedman
Copy link

@bre1470 thanks - my understanding for now is that I should follow the instructions @ https://github.com/highcharts/highcharts-declarations-beta plus the types from @BrandonWilliamsCS updated to use your beta types e.g.
Highcharts.ChartObject -> Highcharts.Chart
highcharts?: Highcharts.Static; -> highcharts?: typeof Highcharts;
Is that fair?

@bre1470
Copy link
Member

bre1470 commented Nov 26, 2018

@jonfreedman Yes, that sounds reasonable and should help in the meantime.

I just like to point out, that the beta of the Highcharts declarations is not intended for regular development use. You might have to change some code after a future beta update.

@jonfreedman
Copy link

Happy to change later but the good news is this is working right now.

@pdeva
Copy link

pdeva commented Mar 1, 2019

now that Highcharts 7 contains typescript definitions by default, any updates on this issue?

@pdeva
Copy link

pdeva commented Mar 1, 2019

declare module "highcharts-react-official" {
    import * as React from "react";
    import * as Highcharts from "highcharts";

    /**
     * Represents a "constructor" function for a ChartObject.
     * Not technically a constructor, as it's not called with `new`.
     */
    type Constructor = (
        renderTo: HTMLElement,
        options: Highcharts.Options,
    ) => Highcharts.ChartObject;
    /**
     * Represents all of the strings that are keys of T that map to a Constructor function.
     */
    type ConstructorName<T> = {
        [K in keyof T]: T[K] extends Constructor ? K : never
    }[keyof T];

    export interface HighchartsReactProps<TContainerProps = {}> {
        highcharts?: Highcharts.Static;
        constructorType?: ConstructorName<Highcharts.Static>;
        update?: boolean;
        options: Highcharts.Options;
        containerProps?: TContainerProps;
    }
    export class HighchartsReact<TContainerProps = {}> extends React.Component<
        HighchartsReactProps<TContainerProps>
    > {}

    export default HighchartsReact;
}

@BrandonWilliamsCS this no longer works with Highcharts 7.
getting error saying Highcharts.ChartObject and Highcharts.Static do not exist

@BrandonWilliamsCS
Copy link

@pdeva, Unfortunately, I don't use this actively use library any longer, so it won't be likely that I'll be taking a close look at v7.
It could be that there are other types that can be swapped in for Highcharts.ChartObject and Highcharts.Static, but it's also possible that v7 came with enough of an API change that the types I wrote are essentially incompatible.

@ppotaczek
Copy link
Contributor

The TypeScript declarations for this wrapper have been added in verion 2.1.0.

Please report in case of any problems.

Best regards!

@ppotaczek ppotaczek reopened this Mar 7, 2019
@jonfreedman
Copy link

I am seeing the following error:

node_modules/highcharts-react-official/dist/highcharts-react.min.d.ts
(51,29): An index signature parameter type must be 'string' or 'number'.

See https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#3.9.4

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.

9 participants