1

I am trying to remove data (yAxis/series) from the charts, but i can not understand why my code is not work correctly. I am sending Observable data from the parent to the child component with chart. I have 2 codes which works or not. In my opinion there is a problem with synchronization forEach loop with remove(true).

I mean. When i am using forEach() or for() loop and I try to remove yAxis then some iterations of loop are missed and the chart does not remove all data.

In the other hand if i use setTimeout() with remove(true), the problem is solved and everything is deleting. Any Idea what is the problem?

Workable code:

    this.chart.series.forEach((serie: Highcharts.Series) => {
        if (!this._series.has(serie.options.id)) {
            const yAxis = this.chart.get(serie.options.id) as Highcharts.Axis;
            if (yAxis) {
                 setTimeout(() => {
                     yAxis.remove(true);
                 });
            }
        }
    });

Broken code:

        this.chart.series.forEach((serie: Highcharts.Series) => {
            if (!this._series.has(serie.options.id)) {
                const yAxis = this.chart.get(serie.options.id) as Highcharts.Axis;
                if (yAxis) {
                    yAxis.remove(true);
                }
            }
        });

The first one is working correctly. I am looking for the answer why iterations of loop are missing when i do not use setTimeout(). Is there a problem with synchronization?

Thanks for the answers.

Piotr Gudalewicz
  • 75
  • 9
  • Could you reproduce this issue in an online code editor like codesandbox? You can use this demo as a template: https://codesandbox.io/s/543l0p0qq4. – Wojciech Chmiel Jul 25 '19 at 08:30
  • I can not. I am using a NgRx and real Api data. The project is big, so it would take a lot of time to reproduce it. I know this would be easier to show it for you. I can tell you that, the chart's component are getting data from the Store and i am clearing it with the Store. When i am getting information from the store that the series(@Input()) were cleared, then the chart is updating. In this update method i am checking the chart's series which were saved and removing them as in the example. I have debugged the code, and if i do not use setTimeout() some of iteration are missed. – Piotr Gudalewicz Jul 25 '19 at 08:44
  • In my opinion there is a problem with synchronization or something like that. remove() does not synchronize with the loop, and some of iteration came faster than the remove() operation is done. But i am not sure if it is the answer. I am looking someone who could be know why that is happened :) – Piotr Gudalewicz Jul 25 '19 at 08:48
  • Another Curiosity. If we comment the line with yAxis.remove(true) the loop is working correctly. This looks that the yAxis.remove(true) operation makes problem here. – Piotr Gudalewicz Jul 25 '19 at 09:50

1 Answers1

2

That is a classic problem with removing elements from array by a loop. The array is being re-indexed when you use remove method with redraw parameter.

Please check this live example: http://jsfiddle.net/BlackLabel/fy97u2bv/

The solution is to loop backwards:

for (var i = chart.yAxis.length-1; i >= 0; i--) {
    chart.yAxis[i].remove();
}

Live demo: http://jsfiddle.net/BlackLabel/4dt6xjvk/

ppotaczek
  • 36,341
  • 2
  • 14
  • 24
  • I have tried it before. There is a small problem. Before i am generating chart i have an empty chart with the initial yAxis, i mean chart.yAxis.lengt = 1. If i will remove all of them it will remove initial too and will get "ERROR TypeError: Cannot read property 'pos' of undefined". This is possible to remove all series without this error if we will remove all yAxies expect the initial yAxis. I mean i > 0; – Piotr Gudalewicz Jul 25 '19 at 10:42
  • Hi @Piotr Gudalewicz, So I understand that your problem has been solved? By loop backwards you can remove only specific elements: http://jsfiddle.net/BlackLabel/ankf6c19/ – ppotaczek Jul 25 '19 at 10:54
  • 2 solutions works. I am just looking for an answer is it possible that remove() is blocking the for()/forEach() loops somehow? In my opionion this is impossible to block it. If i have an array of series array.length = 2, then it should move twice times, but if i have used remove() it moved only first time, and did not check another item of array. The setTimeout() solved this problem, same your idea (if i will leave the first item). Can not understand why does it possible to block the another iteration of loop – Piotr Gudalewicz Jul 25 '19 at 11:16
  • You had right. Thanks for the help. We have to reverse loop. I have recreated your code and it looks like in the example: http://jsfiddle.net/dpu89qgf/4/ Now it all works fine. Thanks for the help! – Piotr Gudalewicz Jul 25 '19 at 11:54
  • The `remove` method does not block the loop in any way. That is normal synchronous code. The whole case is about re-indexing array. For example, if you remove the first item with index 0, the second becomes the first and is skipped. The workaround with `setTimeout` works because it is asynchronous code. Please check this thread: https://stackoverflow.com/questions/37977602/settimeout-not-working-inside-foreach – ppotaczek Jul 25 '19 at 11:55
  • Yes i agree with you. Anyway i do not have to use reverse() to make it work correct. this.chart.series.slice().forEach((serie: Highcharts.Series) => {...} works fine. I think we can close the problem. Thanks for help :) – Piotr Gudalewicz Jul 25 '19 at 12:03