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

ngOnDestroy on `providedIn: 'root'` service not called #28927

Closed
sod opened this issue Feb 22, 2019 · 1 comment

Comments

Projects
None yet
3 participants
@sod
Copy link

commented Feb 22, 2019

angular 7.2.5 - reproduction: https://stackblitz.com/edit/angular-begg91

Issue
When you call destroy() on an application, ngOnDestroy on services that are @Injectable({providedIn: 'root'}), is never called.

@Injectable() // injected in an NgModule
export class ServiceInModule {
    ngOnDestroy() {} // <- working: is called when app is destroyed
}

@Injectable({providedIn: 'root'})
export class ServiceProvidedInRoot {
    ngOnDestroy() {} // <- not working: is never called
}

You see here, the constructor of ServiceProvidedInRoot is called in both bootstraps ... so it's not reused and should be parse of the livecycle. But ngOnDestroy is not called if module is destroyed:

image

This is currently an issue for as as we try to use angular/universal, but we can't tear down objects in services when provided in root.

@ngbot ngbot bot added this to the needsTriage milestone Feb 22, 2019

JoostK added a commit to JoostK/angular that referenced this issue Feb 23, 2019

fix(core): call ngOnDestroy for tree-shakeable providers
Prior to this change, any provider that was independently resolved using
its InjectableDef would not be considered when destroying the module it
was requested from. This commit provides a fix for this issue by storing
the resolved provider in the module's list of provider definitions.

Fixes angular#28927

JoostK added a commit to JoostK/angular that referenced this issue Feb 23, 2019

test(ivy): verify ngOnDestroy is called for tree-shakeable providers
This test verifies that Ivy's module injector does not suffer from
angular#28927 to prevent regressing on this behavior going forward.
@JoostK

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

Thanks for the StackBlitz, that helped in tracking down this issue. I opened PR #28943 to fix it.

@IgorMinar IgorMinar closed this in 30b0442 Apr 5, 2019

IgorMinar added a commit that referenced this issue Apr 5, 2019

test(ivy): verify ngOnDestroy is called for tree-shakeable providers (#…
…28943)

This test verifies that Ivy's module injector does not suffer from
#28927 to prevent regressing on this behavior going forward.

PR Close #28943

DeveloperFromUkraine added a commit to DeveloperFromUkraine/angular that referenced this issue Apr 11, 2019

fix(core): call ngOnDestroy for tree-shakeable providers (angular#28943)
Prior to this change, any provider that was independently resolved using
its InjectableDef would not be considered when destroying the module it
was requested from. This commit provides a fix for this issue by storing
the resolved provider in the module's list of provider definitions.

Fixes angular#28927

PR Close angular#28943

DeveloperFromUkraine added a commit to DeveloperFromUkraine/angular that referenced this issue Apr 11, 2019

test(ivy): verify ngOnDestroy is called for tree-shakeable providers (a…
…ngular#28943)

This test verifies that Ivy's module injector does not suffer from
angular#28927 to prevent regressing on this behavior going forward.

PR Close angular#28943

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

fix(core): call ngOnDestroy for tree-shakeable providers (angular#28943)
Prior to this change, any provider that was independently resolved using
its InjectableDef would not be considered when destroying the module it
was requested from. This commit provides a fix for this issue by storing
the resolved provider in the module's list of provider definitions.

Fixes angular#28927

PR Close angular#28943

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

test(ivy): verify ngOnDestroy is called for tree-shakeable providers (a…
…ngular#28943)

This test verifies that Ivy's module injector does not suffer from
angular#28927 to prevent regressing on this behavior going forward.

PR Close angular#28943
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.