AccordionTab throwing ViewDestroyedError during unit testing

[brian428]

I'm submitting a ... (check one with "x")

[X] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Plunkr Case (Bug Reports)
Unfortunately, I'm not sure how to create a demo of the problem because the problem only manifests during unit testing. However, the source of the problem is very straightforward:

1. If a spec completes (e.g. calls done()) before the Accordion finishes animating, a ViewDestroyedError: Attempt to use a destroyed view: detectChanges is thrown.
2. When the animation finishes, it triggers onToggleDone(). This in turn calls the animation setter, which tries to run change detection (https://github.com/primefaces/primeng/blame/master/src/app/components/accordion/accordion.ts#L77).
3. If the component has already been destroyed, the ViewDestroyedError is thrown.

It doesn't cause the spec to fail, but it does create a ton of these errors in the log. Perhaps just add an internal property for isDestroyed that is set to true in onDestroy(), and then check to make sure it is still false before calling detectChanges()?

[sticl]

Same problem here (same error coming from onToggleDone()), this only shows up when in debug mode.

[cicciogeppo88]

Hello!
I am having the same troubles here, tests are passing but errors are thrown.
Is there any fix available to this bug? all help will be appreciated,

Details:
Angular cli v 8.1.2
Angular core 8.1.3
Karma 4.2.0
Jasmine core 3.4.0
Prime NG v 8.0.2

Thanks

[yigitfindikli]

Hi,
Can you provide stackblitz sample using with https://stackblitz.com/github/primefaces/primeng-issue-template .

[brian428]

Like I said in the initial report, I'm not sure how to provide an example because I only see this during unit testing, where a test completes and the component is destroyed before the Accordion finishes animating.

I think my initial description tells the whole story though. If you just look at the code I linked to above you can clearly see the problem. There's no check in place to make sure the component hasn't been destroyed before it runs detectChanges().

I think onDestroy() just needs to set a flag like this.isDestroyed = true;, and then the animating setter would just become:

set animating(val: boolean) {
    this._animating = val;
    if(!this.isDestroyed) this.changeDetector.detectChanges();
}

[cicciogeppo88]

@yigitfindikli same comment as @brian428 can't provide an example as well; what I could understand is that a singular test ran with Fit (even if is obsolete) is not throwing errors, but if I run more tests together, some of these are throwing errors.

[gferreira-keeps]

I've done a pull request using a solition also proposed by @brian428 (#8083).

[cicciogeppo88]

Hello there,
Thank you @gferreira-keeps , do you know when this will be approved/released?
I would like to try it to see if is working and fixing this topic's issue,

Thanks