highcharts / highcharts-vue

Other
685 stars 150 forks source link

Insufficient check before calling .destroy() #249

Closed shenie closed 8 months ago

shenie commented 1 year ago

Creating an issue per request (see https://github.com/highcharts/highcharts-vue/pull/236#issuecomment-1748526369) for the PR https://github.com/highcharts/highcharts-vue/pull/248 that addresses the issue on https://github.com/highcharts/highcharts-vue/blob/30bfdd7fe438984be816c6e66fc3420ce41593d2/src/component.js#L96-L98 where if (chart.value) will always evaluate to true even because the value is {}, i.e. onBeforeUnmount is called before onMounted has had a chance to set chart.value to a highcharts' chart.

It doesn't happen on all scenario, at least not with the current demo (https://github.com/highcharts/highcharts-vue/tree/master/demo-v3) but I have updated demo-v3 to reproduce the bug, see https://github.com/highcharts/highcharts-vue/compare/master...shenie:highcharts-vue:demo?expand=1

From what I found, it happens when using vue-router, async setup + suspense without keep-alive.

To see the bug, run demo-v3 on the branch demo on my fork and hit the url http://localhost:8080/chart-composition-api then you'll see the bug where the chart is not rendered and the console has the following error.

Screenshot 2023-10-06 at 12 04 26 pm
jszuminski commented 1 year ago

Closed as completed with https://github.com/highcharts/highcharts-vue/pull/248.

thaupersol commented 9 months ago

@jakubSzuminski when do we release this change?

jszuminski commented 9 months ago

@thaupersol I'll try to push a release this week with a few additional changes.

thaupersol commented 9 months ago

@jakubSzuminski Thanks a lot. We're facing the same issue.

lucasmcht commented 8 months ago

I'm facing the same problem, is it possible to create a new tag with this fix ?