8000 ngOnDestroy not called for injectables created via factory providers provided at component level · Issue #28738 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

ngOnDestroy not called for injectables created via factory providers provided at component level #28738

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.

Sign up for GitHub

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

Jump to bottom
Open
llorenspujol opened this issue Feb 14, 2019 · 16 comments
Open

ngOnDestroy not called for injectables created via factory providers provided at component level #28738

llorenspujol opened this issue Feb 14, 2019 · 16 comments
Labels
area: core Issues related to the framework runtime core: di design complexity: major freq3: high hotlist: error messages P4 A relatively minor issue that is not relevant to core functions
Milestone
Backlog

Comments

@llorenspujol
Copy link
llorenspujol commented Feb 14, 2019 •
edited
Loading

🐞 bug report

Affected Package

@angular/core

Is this a regression?

Don't know

Description

ngOnDestroy is not called for Services provided via factory function into component level.

🔬 Minimal Reproduction

I forked the Angular repo and created a Merge Request including 2 tests that both should pass. Of course, one test don't pass, that is the ones that we provide the service using the factory function.

EDIT: I also have added the code to make the tests pass, although I am not in context at all, an maybe I am missing something in the path

Here is the MR: #28737

🌍 Your Environment

Angular Version: 8.0.0-beta.3
@llorenspujol llorenspujol mentioned this issue Feb 14, 2019
Closed
@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Feb 14, 2019
@ngbot ngbot bot added this to the needsTriage milestone Feb 14, 2019
@trotyl
Copy link
Contributor
trotyl commented Feb 15, 2019

Duplicate of #14821, but possibly better to use new one for tracking.

@llorenspujol llorenspujol mentioned this issue Feb 15, 2019
Closed
10 tasks
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Apr 9, 2019
@Ghostbird
Copy link
Ghostbird commented May 17, 2019 •
edited
Loading

I still have this problem in Angular 8.0.0-rc.4, which probably means I'm doing something subtle wrong:

@Injectable({
  providedIn: 'root',
  useFactory: () => new TestService(),
})
export class TestService implements OnDestroy {
  ngOnDestroy() {
    console.log('Service destroyed');
  }
}

@Component({
  selector: 'app-test',
  template: `<button [routerLink]="['/']">Go back</button>`
})
export class TestComponent implements OnDestroy {
  constructor(test: TestService) { }
  ngOnDestroy(): void {
    console.log('Component destroyed');
  }
}

In this case the service is never destroyed when the back button is pressed, but the component is correctly destroyed.
I have instead tried:

{
  provide: TestService,
  useFactory: () => new TestService(),
}

In the root Module's providers array, but this made no difference.

@llorenspujol
Copy link
Author
llorenspujol commented May 20, 2019 •
edited
Loading

@Ghostbird ngOnDestroy from a service depends upon where it is provided. Here are you providing the service as singleton with: providedIn: 'root', so service will be destroyed (ngOnDestroy called) when the application is destroyed (theoretically).

If you want this service to be created and destroyed with the component, you need to provide it at component level, like:

@Component({
  selector: 'app-test',
  template: `<button [routerLink]="['/']">Go back</button>`.
  /** Provide here TestService */
  providers: [TestService]
})
export class TestComponent implements OnDestroy {
  constructor(test: TestService) { }
  ngOnDestroy(): void {
    console.log('Component destroyed');
  }
}

@Ghostbird
Copy link
Ghostbird commented May 21, 2019 •
edited
Loading

@llorenspujol After re-checking my code I recall that I thought that this construction:

@Injectable({
    providedIn: 'root',
    useFactory: () => new TestService()
}

Would provide the factory () => new TestService() on the root level, but would create and destroy the service instances individually for each component. Clearly that is not how the useFactory works. i misunderstood its purpose and funcionality.

@trotyl trotyl mentioned this issue Jul 17, 2019
Closed
@trotyl trotyl mentioned this issue Aug 29, 2019
Closed
@Rush
Copy link
Rush commented Oct 23, 2019

I am also suffering from this issue.

A solution for the factory provider would be to expose an alternative interface.

 {
            provide: FooService,
            useFactory(
                depService: DepService,
            ) {
                const value = new SpecificFooService(
                    depService,
                );
                return {
                    value,
                    onDestroy: () => value.ngOnDestroy(),
                 };
            }, deps: [
                DepService,
            ],
        },

@chenlijun99
Copy link

Any update on this?

@distante
8000
Copy link
distante commented Mar 4, 2020

No news? Is this still a problem in Angular 9 ?

@kara kara mentioned this issue May 28, 2020
Closed
@ngbot ngbot bot removed this from the Backlog milestone May 29, 2020
@bkcummins
Copy link

probably not ideal but I'm accomplishing this by calling this.Service.ngOnDestroy(); on the Component.ngOnDestroy() that introduced the Service and it's working.

@distante
Copy link
distante commented Nov 5, 2020 via email

llorenspujol added a commit to llorenspujol/angular that referenced this issue Nov 23, 2020
@llorenspujol
fix(core): call ngOnDestroy on useFactory providers that implement it…
fef12c3 
… and are provided at component level (angular#28738)

Fix ngOnDestroy to be called on useFactory providers that implement it
and are provided at component level. Fix applies to both ViewEngine and
@AndrewKushnir AndrewKushnir mentioned this issue Dec 16, 2020
Closed
@ngbot ngbot bot modified the milestones: Backlog, needsTriage May 13, 2021
@tarangkhandelwal
Copy link

Any updates on this? This was little strange to figure out at first (as normal services without token have their ngOnDestroy called) and one can hit issues if using stateful component level services.

@Sebi2020
Copy link
Sebi2020 commented Dec 2, 2021

Still an issue...

@jessicajaniuk jessicajaniuk added P4 A relatively minor issue that is not relevant to core functions and removed type: confusing triage #1 labels Dec 13, 2021
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Dec 13, 2021
@JoostK JoostK mentioned this issue Aug 8, 2022
Closed
@johncrim
Copy link
johncrim commented Apr 8, 2023

Is this much easier to fix now that we have DestroyRef in v16? The workaround is certainly easier/less clunky.

@arobinson
Copy link

Still an issue with Angular 18:
https://stackblitz.com/edit/stackblitz-starters-cvudxt?file=src%2Fapp%2Fapp.service.ts

But this does seem to work on a service created by a factory:

    inject(DestroyRef).onDestroy(() => {
      console.log('Destroying AppService (destroy ref)', this.instanceId);
    });

@llorenspujol
Copy link
Author

I confirm that this issue is still happening in Angular 19.

What a deja vu I just had after 6 years of opening the issue😆

@JeanMeche JeanMeche mentioned this issue May 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: core Issues related to the framework runtime core: di design complexity: major freq3: high hotlist: error messages P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Milestone
Backlog
0