amcharts / amcharts3

JavaScript Charts V3
Other
395 stars 129 forks source link

Error: axes or graphs of undefined #173

Open Skuriles opened 6 years ago

Skuriles commented 6 years ago

Hello I have a strange problem: the getExtremes function throws error (in some cases it is axes, sometimes it is graphs of undefined) I did some workarounds to prevent this error but it still occurs in some situations.

Environment: I'm using Angular 5 with angular cli. I use a component which provides a graph (multi axes serial chart). This component is used in several places. There are graphs (and accordingly value axes) added and removed dnymically. The error occurs if the chart is once created, emptied (=> destroyed) and then created again in the same or another component. It looks like that the chart in global AmCharts variable is still alive.

This is used on component destroy. chart.clear() or AmCharts.clear() (and the destroy chart functionality from the official angular plugin didn't work as well => https://github.com/amcharts/amcharts3-angular2) didn't help.

this.chart.dataProvider = []; this.amCharts.updateChart(this.chart, () => {}); const length = this.chart.valueAxes.length - 1; for (let i = length; i > 0; i--) { // this will remove chart as well this.chart.removeValueAxis(this.chart.valueAxes[i]); }

I've added this null check in the amchart.js. this.data is an empty array but getExtremes is still called and it triggers the above mentioned error.

getExtremes: function(a, b) { var c, e, d; for (d = a; d <= b; d++) { **if (!this.data[d] || !this.data[d].axes || !this.data[d].axes[this.id]) { continue; }** var f = this.data[d].axes[this.id].graphs, h; ...

Maybe you can add this null check to prevent this error, but better would be a solution which let us destroy a chart completely. I don't want to handle with an empty chart where some references are still hold somewhere which causes side effects like this.

Or as alternative: Stop adding default valueaxes if the chart removes his last valueAxe. I didn't get this to reproduce with a simple plunkr (as this seems to work in a not-angular app). The code I can't share completely but if necessary I will try to set up a angular 5 plunkr and try to reproduce the problem.

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Hi there,

So sorry for not responding earlier. We're just now setting up connection between our helpdesk and GitHub issues.

chart.clear() should completely destroy anything related with the chart.

However, some components of the chart are referenced somewhere in the code, it might not be garbage collected from memory.

I'm afraid we'll need a way to reproduce this to be able to figure out.

If it's not too much trouble, could you please set up that plunkr after all?

Yours sincerely,

Martynas Majeris amCharts

Skuriles commented 6 years ago

Hey, https://plnkr.co/edit/ss4bfklGgQKxnZ74B2z3 Angulr 5 Typescript plunkr. A interval will be started (used that for another issues I sent your customer support) and after 5 seconds the plunkr will break and then you can see that "this.chart" is still available, directly after a "this.chart.clear()".

=> the chart instance is still alive

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Thanks for this. It's helps.

I'm going to look into this and get back to you.

Yours sincerely,

Martynas Majeris amCharts

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

I briefly tried your plunkr yesterday and it working fine.

Now, when I come back, it errors out. Did you change something.

In any case, there's one thing we noticed.

You probably should not be calling clear() directly, but rather this:

this.AmCharts.destroyChart(this.chart)

Let me know if this helps.

Yours sincerely,

Martynas Majeris amCharts

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

On a related note, you shouldn't call validateData() directly on chart object, either.

Do this instead:

this.AmCharts.updateChart(this.chart, () => { ... })

Yours sincerely,

Martynas Majeris amCharts

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Oops, one more thing :)

It looks like you're not using our Angular plugin. Please do so:

https://github.com/amcharts/amcharts3-angular2

The README hs some basic instructions for you to follow.

I hope this helps.

Yours sincerely,

Martynas Majeris amCharts

Skuriles commented 6 years ago

I didn't change the plugin so far since the last change. I tried it with the angular plugin as well in our project and there was no difference (as the plugin does nothing different then call invalideData and clear on the chart object itself as far as I saw while debugging). And: the plugin does the whole thing asynchronous which can cause bad side effects, so we used the native functions in our project.

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Thanks for the additional details.

In any case, would you be willing to take a look at the plunkr and possibly resolve the issues so we can continue looking into it?

Yours sincerely,

Martynas Majeris amCharts

Skuriles commented 6 years ago

Seems like plunkr changed the whole plunkr plugin urls for angular or they are not reachable (currently) under the used links. No one of the angular plunkr will work currently. Maybe I will find some time within the next two days.

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Sounds good. Thanks.

And sorry for making you to go through this.

Yours sincerely,

Martynas Majeris amCharts

Skuriles commented 6 years ago

I've checked several plunkr with angular and they are all not working anymore. The new alpha plunkr site has several issues as well. For now it is not useable for live testing. I would recommend to set up an own local Angular 5 amchart test with angular cli and the amchart library. Most of the code can be used from my plunkr anyway. So start here: https://github.com/angular/angular-cli and just remove everything what is not needed and replace it with the code from my plunkr. This sould be enough

amcharts commented 6 years ago

New comment from Zendesk by Martynas Majeris on ticket 33370. (replying here will automatically notify amCharts support agent)

Fair enough.

We'll get back to you once we have had a chance to test it out.

Yours sincerely,

Martynas Majeris amCharts

zuice32 commented 6 years ago

Hi Martynas. I recommend you check out the solution I created in one of the feeds and possibly update the example to include a dynamic chart component that is hooked in through the viewchild directive:

https://github.com/amcharts/amcharts3-angular2/issues/12