diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 3254db34d6..0696e50dc2 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -16,6 +16,13 @@ import {onlyInIvy} from '@angular/private/testing'; const NAME = new InjectionToken('name'); +@Injectable({providedIn: 'root'}) +class SimpleService { + static ngOnDestroyCalls: number = 0; + id: number = 1; + ngOnDestroy() { SimpleService.ngOnDestroyCalls++; } +} + // -- module: HWModule @Component({ selector: 'hello-world', @@ -32,7 +39,22 @@ export class HelloWorld { export class GreetingCmp { name: string; - constructor(@Inject(NAME) @Optional() name: string) { this.name = name || 'nobody!'; } + constructor( + @Inject(NAME) @Optional() name: string, + @Inject(SimpleService) @Optional() service: SimpleService) { + this.name = name || 'nobody!'; + } +} + +@Component({ + selector: 'cmp-with-providers', + template: '', + providers: [ + SimpleService, // + {provide: NAME, useValue: `from Component`} + ] +}) +class CmpWithProviders { } @NgModule({ @@ -88,7 +110,7 @@ export class ComponentWithInlineTemplate { @NgModule({ declarations: [ HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp, ComponentWithPropBindings, - HostBindingDir + HostBindingDir, CmpWithProviders ], imports: [GreetingModule], providers: [ @@ -252,6 +274,22 @@ describe('TestBed', () => { expect(hello.nativeElement).toHaveText('Hello injected World a second time!'); }); + it('should not call ngOnDestroy for a service that was overridden', () => { + SimpleService.ngOnDestroyCalls = 0; + + TestBed.overrideProvider(SimpleService, {useValue: {id: 2, ngOnDestroy: () => {}}}); + const fixture = TestBed.createComponent(CmpWithProviders); + fixture.detectChanges(); + + const service = TestBed.inject(SimpleService); + expect(service.id).toBe(2); + + fixture.destroy(); + + // verify that original `ngOnDestroy` was not called + expect(SimpleService.ngOnDestroyCalls).toBe(0); + }); + describe('multi providers', () => { const multiToken = new InjectionToken('multiToken'); const singleToken = new InjectionToken('singleToken'); diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index 551b038d7e..ff5402140a 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -651,18 +651,20 @@ export class R3TestBedCompiler { const overrides = this.getProviderOverrides(flattenedProviders); const overriddenProviders = [...flattenedProviders, ...overrides]; const final: Provider[] = []; - const seenMultiProviders = new Set(); + const seenOverriddenProviders = new Set(); - // We iterate through the list of providers in reverse order to make sure multi provider - // overrides take precedence over the values defined in provider list. We also filter out all - // multi providers that have overrides, keeping overridden values only. + // We iterate through the list of providers in reverse order to make sure provider overrides + // take precedence over the values defined in provider list. We also filter out all providers + // that have overrides, keeping overridden values only. This is needed, since presence of a + // provider with `ngOnDestroy` hook will cause this hook to be registered and invoked later. forEachRight(overriddenProviders, (provider: any) => { const token: any = getProviderToken(provider); - if (isMultiProvider(provider) && this.providerOverridesByToken.has(token)) { - // Don't add overridden multi-providers twice because when you override a multi-provider, we - // treat it as `{multi: false}` to avoid providing the same value multiple times. - if (!seenMultiProviders.has(token)) { - seenMultiProviders.add(token); + if (this.providerOverridesByToken.has(token)) { + if (!seenOverriddenProviders.has(token)) { + seenOverriddenProviders.add(token); + // Treat all overridden providers as `{multi: false}` (even if it's a multi-provider) to + // make sure that provided override takes highest precedence and is not combined with + // other instances of the same multi provider. final.unshift({...provider, multi: false}); } } else { @@ -726,10 +728,6 @@ function getProviderToken(provider: Provider) { return getProviderField(provider, 'provide') || provider; } -function isMultiProvider(provider: Provider) { - return !!getProviderField(provider, 'multi'); -} - function isModuleWithProviders(value: any): value is ModuleWithProviders { return value.hasOwnProperty('ngModule'); }