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

The type definition of highstock is incorrect #9705

Closed
scott-ho opened this issue Dec 18, 2018 · 23 comments
Closed

The type definition of highstock is incorrect #9705

scott-ho opened this issue Dec 18, 2018 · 23 comments
Assignees
Milestone

Comments

@scott-ho
Copy link

Expected behaviour

import the highstock types correctly.

Actual behaviour

import with build errors

Live demo with steps to reproduce

Just install the v7 highcharts and drilldown to see node_modules/highcharts/highstock, the path is invalid

image

Product version

v7.0

Affected browser(s)

ALL

@bre1470 bre1470 self-assigned this Dec 18, 2018
@bre1470 bre1470 added this to the v7.0.1 milestone Dec 18, 2018
@bre1470
Copy link
Contributor

bre1470 commented Dec 18, 2018

Thank you for the report! This will be indirectly resolved with #9692

@scott-ho
Copy link
Author

@bre1470 how does the PR resolve the problem. The changes inside it doesn't involve related code.

@bre1470
Copy link
Contributor

bre1470 commented Dec 18, 2018

The fix is part of v1.0.5 of the declarations generator.

@bre1470 bre1470 closed this as completed Dec 18, 2018
@KacperMadej
Copy link

@scott-ho
If you need to load Highstock with current version in your project you could use a module.
Workaround: load stock as a module, so like:

import * as Highcharts from 'highcharts';
import StockModule from 'highcharts/modules/stock';
StockModule(Highcharts);

@scott-ho
Copy link
Author

I'm afraid that it's not very feasible. We have many many imports from highstock.

BTW. When will it be released?

@bre1470
Copy link
Contributor

bre1470 commented Dec 19, 2018

Highcharts v7.0.1 will be released this week and fix the problem.

@gbenthomas
Copy link

It looks like even in v7.0.1, the path in highcharts/highstock.d.ts is wrong:

import StockModule from '../modules/stock';

@KacperMadej
Copy link

@gbenthomas Thank you very much for finding this one out. Only that file has this problem.

As a workaround you could use un-minified version of Highstock that has correct .d.ts

@bre1470 Looks like a single typo.

@KacperMadej KacperMadej reopened this Dec 20, 2018
@bre1470
Copy link
Contributor

bre1470 commented Dec 20, 2018

I am sorry for this. The bug actually should have been detected by dtslint here: https://github.com/highcharts/highcharts/blob/master/test/typescript/highstock.ts#L17 , because the stockChart factory is only available, if the stock module declaration can be found.

We will now improve our test systems, so it detects bugs in general more reliable.

@scott-ho
Copy link
Author

Hope this could be fixed soon.

@SayakMukhopadhyay
Copy link

Is there a timeline for this fix to be released?

@KacperMadej
Copy link

The fix is expected to be published within next few weeks. It should be a part of next path release (7.0.2 or any next version of Highcharts that will be published) if a new version of declarations generator (probably 1.0.6) will be included there with the typo fix (at this very moment the fix is a part of 1.0.6 version of the declarations generator).

tl;dr ETA within next few weeks.

@jon-a-nygaard jon-a-nygaard modified the milestones: v7.0.1, v7.0.2 Jan 3, 2019
@slavede
Copy link

slavede commented Jan 14, 2019

Well, this error makes the lib unusable, because when you are building on CI, it uses 7.0.1 version without the change/workaround you are mentioning, couldn't you make a quick fix and release 7.0.2?

@bre1470
Copy link
Contributor

bre1470 commented Jan 14, 2019

This issue is now fixed with the master branch and will be part of the next release, due within a few weeks.

@bre1470 bre1470 closed this as completed Jan 14, 2019
@slavede
Copy link

slavede commented Jan 14, 2019

How much is a few? The thing I'm wondering, why can't you bump minor version to 7.0.2 and release that (or even 7.1.0)? And if you have one additional major release coming that's fine, you can bump it again later?

I'm not trying to be ungrateful here, but to me it seems really critical issue to leave it hanging for weeks just because it doesn't match release plan.

@bre1470
Copy link
Contributor

bre1470 commented Jan 14, 2019

We prepare v7.0.2 for release in the next ten days.

@chukwu
Copy link

chukwu commented Jan 17, 2019

Honestly, I wish my clients have 7-10 days waiting time. Alot has been added to highchart 7 such as cylinder chart type and a more advanced exporting functionality. Being unable to use this is frustrating especially when it's a requirement.

@cebor
Copy link

cebor commented Jan 17, 2019

When you import highstock from the src files, there is not this issue:

import * as Highstock from 'highcharts/highstock.src';

@bre1470
Copy link
Contributor

bre1470 commented Jan 17, 2019

Thank you for your feedback, @chukwu. We are sorry for the frustration this bug is causing.

The workarounds mentioned by @KacperMadej and @cebor (Thank you!) are helpful temporary solutions during the development process, which you should consider:

import * as Highcharts from 'highcharts';
import StockModule from 'highcharts/modules/stock';
StockModule(Highcharts);

 - or -

import * as Highstock from 'highcharts/highstock.src';

@chukwu
Copy link

chukwu commented Jan 17, 2019

The main issue for me is with angular-highchart plugin/wrapper. The plugin doesn't work with highchart ~7 as pointed in cebor/angular-highcharts#250
Plugin developer can't do anything until highchart is fixed. So, I'm forced to install highchart 6.2--

@attilacsanyi
Copy link

Same for us we need to wait, but looking forward to try / fix the new better official beta Highcharts types instead of DefinitelyTyped types ;-)

Thanks for the effort to push things forward for better Typescript support.

@bre1470
Copy link
Contributor

bre1470 commented Jan 18, 2019

Just to make sure, that everyone gets notified: The v7.0.2 update has been released and fixes this issue.

@attilacsanyi
Copy link

Thanks for the quick release Highcharts team! Really appreciated :)

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

No branches or pull requests

10 participants