Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
DerekDuchesne opened this issue Apr 5, 2021 · 9 comments
Assignees
Labels

Comments

@DerekDuchesne
Copy link

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,

@sassomedia
Copy link

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

@DerekDuchesne
Copy link
Author

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.

@martynasma
Copy link
Collaborator

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?

@DerekDuchesne
Copy link
Author

DerekDuchesne commented Apr 7, 2021

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).

@martynasma martynasma self-assigned this Apr 7, 2021
@martynasma
Copy link
Collaborator

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.

@sabouflage
Copy link

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

@martynasma
Copy link
Collaborator

Could we flush the createDeferred Queue somehow?

Yup:

a4core.registry.deferred = [];

@martynasma martynasma added bug and removed question labels May 7, 2021
@oliverlj
Copy link

oliverlj commented Aug 28, 2021

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

@valikbond-napier
Copy link

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants