amcharts4 icon indicating copy to clipboard operation
amcharts4 copied to clipboard

Mixing chart.dispose() with createDeferred() makes us unable to create new graphs

Open DerekDuchesne opened this issue 4 years ago • 9 comments

Hello,

We have several Vue components with a single amcharts chart on each component. We use the mounted() hook to create the charts with am4core.createDeferred() and the beforeDestroy() hook to clean up the charts with this.chart.dispose(). We've found that when we navigate quickly between the components, something goes wrong where graphs can no longer be created.

Demo: https://codesandbox.io/s/blazing-flower-61plo To replicate, try rapidly switching between the "Chart 1" and "Chart 2" links. If done quickly enough, the graphs should disappear and clicking on the links will no long create them.

We might not be using createDeferred() and dispose() in the correct way, so I wanted to double-check with your team if this is expected behavior. If so, can you give us some tips on how to make sure that charts are created and disposed correctly?

Thank you,

DerekDuchesne avatar Apr 05 '21 18:04 DerekDuchesne

fyi, tried your demo but couldn't get it to break. It'd probably be an issue with more complex charts.

sassomedia avatar Apr 06 '21 17:04 sassomedia

I think with more complex charts it'd be easier to demonstrate the issue, but I can still demonstrate it from this demo. You just have to alternate between the components quickly.

DerekDuchesne avatar Apr 06 '21 17:04 DerekDuchesne

I too couldn't reproduce the issue with simple demo.

Couple of questions:

  1. Does it happen when using simple am4core.create() as opposed to am4core.crateDeferred()?
  2. Does it happen in any particular or all browsers?

martynasma avatar Apr 07 '21 05:04 martynasma

I added more data to the charts in the demo, which should hopefully make them slower and easier to demonstrate the issue.

  1. Does it happen when using am4core.create() as opposed to am4core.createDeferred()?

It only happens with am4core.createDeferred() and not am4core.create().

  1. Does it happen in any particular or all browsers?

It's happened in all of the browsers I tried (Chrome and Firefox for Windows).

DerekDuchesne avatar Apr 07 '21 17:04 DerekDuchesne

Thanks. I was able to replicate it now.

We will take a look if this can be perhaps fixed.

However, using createDeferred might not be suitable in your use case altogether.

It relies on ready event of the previously created chart, so if you kill it off before the event kicks in, it never gets to creating a deferred chart.

martynasma avatar Apr 07 '21 19:04 martynasma

We have the same problem with the same described behavior: Only for am4core.createDeferred() and not .create(), in tested browsers (Chrome & Firefox on Linux/Manjaro, and on iOS) - if we switch pages before the full chart rendered, it won't create new graphs. We used the createDeferred() to reduce the workload on the client browser (many data-points, no data aggregation for value-pair-charts) - as suggested in https://www.amcharts.com/docs/v4/concepts/performance/. With the queue option it is slower than the deferred creation variant.

What would be suitable for improving performance indirectly, if we use multiple bigger charts separated onto two pages, if not createDeferred? Assuming, that there is a waiting entry/chart for a "ready" event - Could we flush the createDeferred Queue somehow?

Thanks in advance

sabouflage avatar Apr 15 '21 14:04 sabouflage

Could we flush the createDeferred Queue somehow?

Yup:

a4core.registry.deferred = [];

martynasma avatar Apr 15 '21 18:04 martynasma

I reproduce it. If all charts are generated before a dispose, this works well.

otherwise, charts will fill the array.

		if (registry.deferred.length == 1) {
			processNextDeferred();
		}

because you check the array of length 1, new charts will not be generated.

and if this method :

function processNextDeferred(): void {
	let next = registry.deferred[0];
	if (next) {
		let sprite = next.callback.call(next.scope, ...next.args);
		sprite.events.on("ready", () => {
			next.resolve(sprite);
			registry.deferred.shift();
			if (options.deferredDelay) {
				setTimeout(processNextDeferred, options.deferredDelay);
			}
			else {
				processNextDeferred();
			}
		});
	}
}

I think the issue is, chart is generated and dispose when assign the sprite, so "events.on("ready", () => {" is not executed.

=> let next = registry.deferred[0]; I will do a shift here instead in the on ready event.

Hope this help

oliverlj avatar Aug 28 '21 09:08 oliverlj

I have the same problem for am4core.createDeferred(). When the chart must be deleted before the ready event is emitted. My solution before the official fix:

am4core.options.deferredDelay = 0;

am4core.createDeferred(function(div) {
  return am4core.create(div, am4charts.PieChart);
}, "chartdiv2").then(chart) {
     this.chart = chart;
     this.chart.events.once('beforedisposed', () => {
                this.chart.dispatchImmediately('ready');
      });
}

Also, need to destroy the chart on the component destroy hook this.chart.dispose() to call the beforedisposed event.

To clear the am4core.registry.deferred array need to dispatch the ready chart event

(From my tests) The am4core.options.deferredDelay must be 0, if it's not the graph will be not deleted after the dispatch graph ready event (or another instance of the graph will be created).

valikbond-napier avatar Sep 01 '21 08:09 valikbond-napier