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

Typescript .d.ts #4876

Closed
Bigous opened this issue Dec 23, 2015 · 47 comments
Closed

Typescript .d.ts #4876

Bigous opened this issue Dec 23, 2015 · 47 comments

Comments

@Bigous
Copy link

Bigous commented Dec 23, 2015

Is it possible to you to deploy the definitions for Typescript for Highcharts within the npm package (like angular)?

@ry8806
Copy link

ry8806 commented Jan 30, 2017

with the increasing popularity of TypeScript, I think this needs to happen. Many other js packages are starting to include their own (official) typings now. Rather than leave it up to the DefinitelyTyped community

@TorsteinHonsi
Copy link
Collaborator

This is something we are considering. It should be possible to make a node script to auto generate it from our API dump.

@hanssens
Copy link

hanssens commented Jul 6, 2017

+1 👍 from me as well. The DefinitelyTyped repo is outdated, where after Highcharts v4.2.x only sporadic and incidental properties were updated. But most of the changes in Highcarts are simply not processed.

So either the DefinitelyTyped repo should be updated or - more convenient - having it bundled with Highcharts would even be a much nicer solution.

Having it auto-generated sounds like a great option, but this might still require manual labor on the part of writing tests for the .d.ts file? Also, having it auto-generated might be.

I'd be willing to do some work in updating the DefinitelyTyped repo to the current release, and do a PR on https://github.com/highcharts/highcharts if you want to include the manual file there?

/cc @ry8806 @TorsteinHonsi

@hanssens
Copy link

^ I've created a PR for the DefinitelyTyped repo that implements Highcharts v5.0.10. If anyone is interested, this might be a good place to work further on (once and if it gets merged). At least for the time being, until the TypeScript definition file is deployed along with Highcharts itself.

@jonrh
Copy link

jonrh commented Dec 5, 2017

Having official TypeScript type definitions would be highly appreciated. Helps a lot when typing out a chart config.

@aaronbeall
Copy link

aaronbeall commented Dec 9, 2017

Here we are again with the type definitions a major version behind reality. :/

Given the complexity of this library I think the best solution is to generate the type-defs from the "API dump" that @TorsteinHonsi referred to. However that link appears to be broken. Is there a new one? I would be willing to take a stab at writing a generator.

@TorsteinHonsi
Copy link
Collaborator

However that link appears to be broken. Is there a new one?

Yes, the new one is available from https://api.highcharts.com/highcharts/tree.json. This tree is generated from our JSDoc comments and parsed source code.

@cvasseng FYI.

@aaronbeall
Copy link

@TorsteinHonsi Thanks I'll take a look at this. Are you using a particular flavor of "jsdoc" (Google Closure, JSDoc 3?) Does the JSON dump happen somewhere in this code-base I can look at for reference? The reason I ask is because there are existing JSDoc-to-TSDef convertors out there which might work...

@TorsteinHonsi
Copy link
Collaborator

We are using JSDoc 3, but heavily extended to be able to describe our declarative options structure.

@aaronbeall
Copy link

@TorsteinHonsi Looking at the JSON I have a few questions... you don't by any chance have a schema documentation on this format? This is what I've collected. How do you designate options that are passed to Highcharts.setOptions() vs Highcharts.chart() (for example)? Is there an API dump for the classes and namespaces as well?

@KacperMadej
Copy link

@cvasseng

@cvasseng
Copy link
Contributor

cvasseng commented Dec 13, 2017

Hi @aaronbeall, we don't have any thorough documentation on the schema at this time, but it looks like you got it all in your definition.

The options for Highcharts.setOptions(), and the series are both handled as special cases outside of the schema itself where we use it, but we should change at least global and lang to be outside of the main options structure.

As a side note, the old dump format was available at https://api.highcharts.com/dump.json, we've now moved it back to the original position at https://api.highcharts.com/highcharts/option/dump.json.

@aaronbeall
Copy link

Thanks @cvasseng. I'm starting to tinker with this... there's a lot of data, I have my work cut out for me just to understand it all. :) I noticed some fields have no type and no default to infer the type from, such as boost.seriesThreshold, yet the type appears in the docs. Are these handled as a special case or am I missing something? Also is there any API schema for the namespaces/classes, or is that handled separately? Is the declarative model meant to make those obsolete?

(I'll have more questions, is there a better place to discuss this? Gitter? I'm fine here but it's probably creating quite a bit of noise for some people.)

@cvasseng
Copy link
Contributor

cvasseng commented Dec 14, 2017

The type for boost.seriesThreshold actually looks to be wrong in tree.json (and in the live docs) - it's supposed to be a number, not a string. Looking at the actual doclet from which it was generated, the type is missing from it. This is likely to be the case for any other places where types are missing too. We're working on adding more tests and checks to the generation pass to automatically find these when the doclets change to keep this from happening.

We don't have a schema for namespaces/classes, but those are handled by vanilla JSDoc 3, so it may be possible to add an existing plugin to automatically generate typescript definitions for them. The setup for that part can be found here: https://github.com/highcharts/highcharts-docstrap

And discussing this here works just fine. :) That way we have a central public place for it, so that others may follow it too.

@aaronbeall
Copy link

aaronbeall commented Dec 14, 2017

In case it's helpful this is a dump of all the fields (in highcharts/tree.json) that don't seem to have a type. A quick scan over the inferred types from default values (string, number, boolean) looks correct to me, but the ones that say "Unable to determine type" mean I didn't find a type property or a default property, so it's assigned to any. I could write a patch file for those types, or is there a better way?

There's also a blank key in the top-level JSON and in mapNavigation.buttons.

@aaronbeall
Copy link

aaronbeall commented Dec 15, 2017

I made some progress on this today, you can see what's currently been generated here. There's a lot of work left to make this high quality. I'll push to a repo tomorrow. At a high level some issues I ran into:

  • Not sure how to organize the output. Right now there are 559 symbols, which includes important stuff like Highcharts.Options and Series down to very specific little objects like PlotOptionsBbTopLine. To avoid name conflicts it converts the full name to PascalCase, ex from plotOptions.bb.topLine. Still trying to figure out how to make this better. Trying to compare with the current @types/highcharts is a bit hard, more often than not stuff just doesn't exist over there.
  • I'm not sure I understand the relationship of highcharts/highstocks/highmaps. (I've only used Highcharts personally.) Does the API dump contain all the data of all products? How should it be split up? I'll have to understand better how to actually use highstock and highmaps in a project...
  • The way objects extend each other makes perfect sense but I'm having a hard time working out how to convert it to sensible types. At the moment objects are merged using & intersection types because extends was raising a lot of errors about incompatible extension. Currently there's no handling of excludes, I started playing with the Omit type but that raised a lot of errors about omitted fields not existing in the type. I think this is because in some cases an object property inherits indirectly from the parent, so it doesn't actually have the fields that should be omitted, the parent object's property does. plotOptions.mfi.params is an example of this, it excludes index but it does't have an index property or extends an object that does, but the parent plotOptions.mfi extends plotOptions.sma which has a child plotOptions.sma.params property that has the index which is merged on the parent with plotOptions.mfi.params. Yeah, still noodling how to deal with this. :) Seems like I need to evaluate the fully merged tree and work out the types from there...
  • Lots of generic Array, Object, and Function types... probably all well documented in descriptions but it doesn't seem to be expressed in the types. Maybe some judicious patching of the type data will fix this. My favorite is Array.<Array.<Mixed>> -- no idea what that is yet. :)

Some other specific things:

  • What are the Color, CSSObject, and Mixed types? Color seems to be a formatted string, CSSObject is a standard style object, or something special?
  • series.bellcurve.data and series.histogram.data both extend themselves, creating circular references. I think this is just a typo, they are probably meant to extend something else?

@TorsteinHonsi
Copy link
Collaborator

Also, some of your missing types are not missing in the API, for example https://api.highcharts.com/highstock/plotOptions.bb.topLine.styles.lineColor. If I remember correctly we do type guessing in the api-docs generator, but this could be moved to the highcharts.jsdoc.js script so it would be part of tree.json.

@TorsteinHonsi
Copy link
Collaborator

@cvasseng What is the status on our official TypeScript definitions?

@aaronbeall
Copy link

aaronbeall commented Dec 16, 2017

Uploaded a repo to play with this, no additions today though.

If I remember correctly we do type guessing in the api-docs generator

I'd be curious to understand how the api-docs generator works. It does a good job of making sense out of the API dump. I'm finding things that I'm not sure how to handle. For example series.bullet.data.targetOptions extends series.bullet.targetOptions, but a definition for series.bullet.targetOptions doesn't exist... however the properties comes through in the docs just fine. I guess it's because series.bullet extends plotOptions.bullet which has plotOptions.bullet.targetOptions, so series.bullet.targetOptions resolves to plotOptions.bullet.targetOptions?

Edit: Small bit of progress tonight, my truthy check for default value was discarding all literal false values, fixed that and a lot more stuff is inferred as a boolean. Not sure boolean is the correct type to infer in some places, though. I also check the values for literal types (that info is superb to have!). Anyway the missing types dump is updated.

@aaronbeall
Copy link

aaronbeall commented Dec 20, 2017

Made some progress after thinking about how the tree extends objects:

  • Came up with a method to resolve all the defs that are merged at a given path. (This stretched my brain... I imagine the docs generator does something like this?)
  • Then I reduce the entire tree by removing redundant defs at paths that shadow other defs merged from other parts of the tree, which removes about 100 redundant object defs and hundreds of redundant properties. (Many of those "redundant" defs seemed to be there to provide doc additions, like description or default values, but didn't help the type defs... sound right?)
  • After that only a few defs in the reduced tree were actually missing types that can't be inferred, which I patch here (those changes would probably be good in the jsdoc itself). What's left can all be inferred (see latest "missing types" dump), though I haven't validated the inferred type is always correct. There's also still the issue of vague types like object, array, and function.
  • Here's a dump of how the def tree gets changed before outputting TS type defs. This gives us this generated output which is starting to look promising.

What's next

  • Deal with excluded fields, which I think I have a strategy using Omit<> and making a pass to add explicit extends to all objects that have excludes (using the method mentioned in the first bullet), and changing back to use interfaces with all optional properties. I think this will work, as long as we can be sure that properties will all be optional? Are there any required properties?
  • Some other details: type improvements, cleanup, and tests
  • I'll pick this back up after the holidays. Would love any feedback if you guys think this is even going in the right direction or not.

Questions

  • What does "memberof": "yaxis" in tooltipValueFormat doclet mean?
  • One of the context values is is PlotLineOrBand but I don't see that in the classes docs, is this a sub-class of Series?
  • plotOptions.series.states has doclet type name "plotOptions.series.states" -- does that mean something special?

@scott-ho
Copy link

scott-ho commented Mar 6, 2018

@aaronbeall do you have any progress? I've also dive a little bit into type generation for highcharts.

One major flaw is that the generated tree.json doesn't contain all the typings of highcharts, like static methods on highcharts namespace. There are 703 symbol after valid symbol pruning. But after that, the output symbols inside tree.json is about 100. That's a lot of them ignored.

@aaronbeall
Copy link

aaronbeall commented Mar 6, 2018

@scott-ho It's been a little bit because I moved off the project using Highcharts in the new year, but I am back on it now and will get back into this project soon.

From my last post I went down the route of converting type to interface and using extends instead of & (this was my strategy to properly support the tree.json use of excludes of specific fields), and ran into problems: TS will not allow an interface to extend another interface who shares a field name which is an object type that doesn't share at least 1 property. Here's an abstract example. I forget the exact examples of this I ran into but there were a lot, because of the way tree.json freely extends objects (in some cases extension chains exceeded 5) and freely excludes fields at any point. This led me to think my previous approach of using & intersection types might be the best route. Basically that will result in some properties that are "excluded" not actually appearing excluded in the type hinting (because they are merged in from something extended higher in the tree which can't be omitted from the current node), but I can't think of a better way to do it. The docs get around this issue because they render the properties from a single point, but structurally we can't describe the types that way without ditching extends and creating a ton of mostly duplicate type definitions.

There are 703 symbol after valid symbol pruning. But after that, the output symbols inside tree.json is about 100. That's a lot of them ignored.

What method of pruning are you referring to? This has been the hardest part to deal with...

Yes, I also noticed that that the classes/namespaces are not described in tree.json (that's basically describing the declarative init options as I understand it), but @cvasseng said they are vanilla jsdoc3 so hopefully we can use a standard convertor for that.

@scott-ho
Copy link

scott-ho commented Mar 7, 2018

pruning is referred from this line.

All of the data collection for tree.json lives inside publish method.

I think there could be some misuse of jsdoc api inside it.

We could figure out a better way to generate a new version of tree.json based on the original output from jsdoc.

@scott-ho
Copy link

Today, I spent a while to find out what the tree.json represent. Then I finally figure out that tree.json only hosts the types of chart options. See here https://api.highcharts.com/highcharts/.

Types collecting and generation logic is written here - https://github.com/highcharts/highcharts/blob/master/tools/jsdoc/plugins/highcharts.jsdoc.js.

We might need to generate the full-version of types ourselves and do something similar to https://github.com/englercj/tsd-jsdoc.

@bre1470
Copy link
Contributor

bre1470 commented Jun 28, 2018

We now work on the quality and try to reduce the number of generated interfaces in the options tree of Highcharts. After this we will start a public beta phase.

Our ETA is for now the 3rd quarter 2018 for the Beta.

@aaronbeall
Copy link

aaronbeall commented Jun 28, 2018

@sophiebremer That's great news! Do you have a DTS generator or are you using an existing conversion tool?

@bre1470
Copy link
Contributor

bre1470 commented Jun 29, 2018

We use a custom one. We tried the approach of this pull request #8307 using dts-dom underneath, but this did not work well.

@tbnovaes
Copy link

tbnovaes commented Aug 1, 2018

Any news on this one? It seems that @types/highcharts is not being updated anymore.

@bre1470
Copy link
Contributor

bre1470 commented Aug 2, 2018

We are still working on this with high priority. Unfortunately I can not publish a preview of the declaration right now. There are still types, that need to be consolidated, or that need to be introduced. The "highcharts.d.ts" will be a huge file with more than 200.000 lines of declarations and comments.

@scott-ho
Copy link

scott-ho commented Aug 2, 2018

@sophiebremer how do you produce that file? Manually?

@bre1470
Copy link
Contributor

bre1470 commented Aug 6, 2018

@scott-ho
Although the declaration files are generated automatically, fixing and updating the doclets in the source code is a manual process.

@bre1470
Copy link
Contributor

bre1470 commented Aug 30, 2018

Hi everyone! I published a preview of the declarations for Highcharts in my personal repository. You can test it by npm i https://github.com/sophiebremer/highcharts-declarations-alpha.git or download the declaration from https://github.com/sophiebremer/highcharts-declarations-alpha/blob/master/highcharts.d.ts and place it directly into ./node_modules/highcharts/. Please let us know, if the declaration is working for you or if you have problems. Thank you!

Known issues:

  • type pinning in options.series property is not working. Workaround right now is to cast the object to your series type like { data: [0, 1, 2]} as Highcharts.SeriesLineOptions
  • style options are not everywhere of CSSObject type

@muperi
Copy link

muperi commented Oct 10, 2018

ChartEventsOptions functions like selection do not seem to be taking in the event function parameter

@muperi
Copy link

muperi commented Oct 10, 2018

Couple of other issues that I found

  • Not able to import the exporting, offlinexporting modules. I had to change export = factory to export default factory and use it as a default import

  • Missing annotations module

  • PlotSeriesEventsOptions click needs to be modified as click?: (e:PointerEventObject) => boolean;

@bre1470
Copy link
Contributor

bre1470 commented Oct 12, 2018

Thank you for the feedback, @muperi !
Some comments about your second post:

  • The regular modules do no default export. Please check your tsconfig for ES6 settings.
  • Only a few modules have declarations yet. We will of course add more over the next couple of months.
  • We will look into this issue.

@bre1470
Copy link
Contributor

bre1470 commented Oct 13, 2018

ChartEventsOptions functions like selection do not seem to be taking in the event function parameter

Fix is part of #9110 and will be included in next update of https://github.com/highcharts/highcharts-declarations-beta.

@bre1470
Copy link
Contributor

bre1470 commented Oct 13, 2018

Please add further issues with the declaration files to this repository: https://github.com/highcharts/highcharts-declarations-beta.

Thank you.

@bre1470 bre1470 closed this as completed Oct 13, 2018
@ghost ghost removed the In progress label Oct 13, 2018
@bre1470 bre1470 added this to the v7.0.0 milestone Nov 15, 2018
@bre1470 bre1470 reopened this Nov 15, 2018
@bre1470 bre1470 closed this as completed Dec 3, 2018
@highcharts highcharts locked as resolved and limited conversation to collaborators Jul 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.