Ng-bootstrap: Tooltip: this._unregisterListenersFn is not a function

[bygrace1986]

Bug description:

If an element with the tooltip directive on it is quickly created and destroyed by a structural directive then it causes the following error:

AppComponent.html:3 ERROR TypeError: this._unregisterListenersFn is not a function
    at NgbTooltip.ngOnDestroy (tooltip.js:152)
    at callProviderLifecycles (provider.js:588)
    at callElementProvidersLifecycles (provider.js:556)
    at callLifecycleHooksChildrenFirst (provider.js:540)
    at destroyView (view.js:600)
    at callWithDebugContext (services.js:843)
    at Object.debugDestroyView [as destroyView] (services.js:382)

_unregisterListenersFn is initialized in ngOnInit so it appears that it is not being invoked before ngOnDestroy.

Link to minimally-working plunker that reproduces the issue:

https://stackblitz.com/edit/acl-directive

Version of Angular, ng-bootstrap, and Bootstrap:

Angular: 5.0.0
ng-bootstrap: 1.0.0
Bootstrap: N/A

[pkozlowski-opensource]

@bygrace1986 here is a minimal plunker that confirms my initial thinking: http://plnkr.co/edit/wTdALkovBrdIFJBHoIpk?p=preview

Please notice that calling viewRef.detectChanges() (you can uncomment a line in my plunker) triggers ngOnInit.

So, it looks like creating and immediately destroying a view can result in a situation where ngOnDestroy is called without ngOnInit first.

Given all this the real question is: is it a valid use-case? I can see how you got into this situation in your implementation, but I would argue that this implementation us sub-optimal. Here is why:

• you are creating / destroying views based on @Input's setter invocation and you will have 2 invocations potentially (like in the initial render). This is clearly wasteful as you are creating / destroying views for nothing.;
• you are using this.view.clear(); to remove a view but this is rather "brutal" as it would destroy _all_ the views in a given container, not only ones created by your directive.

If you fix 2 above problems in a different implementation of your acl directive you would have more robust implementation and you wouldn't run into issues with tooltips, see: https://stackblitz.com/edit/acl-directive-dpvcsx?file=app%2Facl.directive.ts

I believe that the implementation I'm proposing is simpler, more performant _and_ correct. You see problems with tooltips since you are creating and destroying views for nothing.

I will probably add a guard to tooltip's ngOnDestroy method to protect its implementation from scenarios like this, but I would strongly suggest changing your implementation of the aclIf directive.

[bygrace1986]

@pkozlowski-opensource Agreed, I realized that it wasn't optimal to create an destroy it so quickly but it illustrated the point so I left it alone. I've seen it come up with weird routing scenarios too and apparently someone found a way to do it with ngIf. I think that most scenarios where this is an issue can and should be avoided but it is still good to guard against it. As far as it being "brutal" ngIf is doing the same thing (https://github.com/angular/angular/blob/master/packages/common/src/directives/ng_if.ts) and that is the behavior that I wanted to mimic. I want to destroy all views in the container.

Thanks for taking the time to look into the issue and the feedback!

[devoto13]

I also have seen routing causing this issue. IIRC it was happening when redirect is triggered from the component constructor. So change detection run was creating components, and than redirect happened in one of them and it started destroying components before it got to calling ngOnInit() on them.

Happy that it is being fixed! -1 workaround in my project 😄