drillUp. Cannot set property 'length' of undefined

[balkarov]

Cannot set property 'length' of undefined when call drillUp() after chart.applyDrilldown();
Screenshot:

Expected behaviour

Not error

Actual behaviour

Error

Live demo with steps to reproduce

https://jsfiddle.net/jus3gc2n/

Product version

Highcharts version 6.1.0

Affected browser(s)

Chrome 66.0.3359.139

[sebastianbochan]

Hi @balkarov,
Thank you for the reporting. The issue is caused by missing ddDupes in the chart object. The solution is call point.doDrilldown() before applyDrilldown.

Demo:

• https://jsfiddle.net/BlackLabel/woy4d52w/

Internal note
Should we add doDrilldown() in applyDrilldown() ?

[muodov]

Calling doDrilldown() triggers the drilldown event, which kinda breaks the async drilldown flow.

Normal async flow:

1. user click triggers drilldown event
2. event listener starts some async work and eventually calls addSeriesAsDrilldown()
3. addSeriesAsDrilldown() calls addSingleSeriesAsDrilldown() and applyDrilldown() but doesn't call doDrilldown() to avoid another drilldown event.

Programmatic flow (as suggested above):

1. call addSingleSeriesAsDrilldown(), applyDrilldown() and doDrilldown() to fix the exception, which will also trigger drilldown event as a side-effect.

Now, if I need to support both programmatic and user-click events, it becomes annoying because I will have to cancel the next drilldown event handler after a programmatic drilldown.

I looked through the code, and I believe the absence of ddDupes is just a mistake introduced in ada67c7#diff-1d9d9d6b74edd8331bf8f425ab1d7f42. It specifically handled this situation in doDrilldown() but not in drillUp(). Before that change "dupes" were always initialized: see the original fix here b633bb9

So, it feels like calling doDrilldown() is a bit of an overkill with unnecessary side effects. Why not just add a check in drillUp() similar to https://github.com/highcharts/highcharts/blob/master/js/modules/drilldown.src.js#L1087?

Or, maybe just initialize ddDupes in constructor and clear it inside destroy() to keep #3990 fixed?

[muodov]

As a workaround, I'm currently just adding these lines after a programmatic drilldown:

if (!chart.ddDupes) { // https://github.com/highcharts/highcharts/issues/8324
   chart.ddDupes = [];
}

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions!

[balkarov]

Hello. Why you close this issue? Did you fix it? What’s the release?

[sebastianbochan]

Hi @balkarov,
I apologize for inconvenience. We use auto-closer bot for the oldest issues. Im reopening the issue.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions!

[balkarov]

Hello. Why you close this issue? Did you fix it? What’s the release?

[pawellysy]

Hello @balkarov, This issue is still not fixed. The stale bot marks the issues, which are inactive for a long period of time as stale, and then closes them due to the inactivity. I will move this issue to the inbox so that it will be prioritized. Sorry for the inconvenience.