Skip to content
Permalink
Browse files

fix(ivy): provider override via TestBed should remove old providers f…

…rom the list (#33706)

While overriding providers in Ivy TestBed (via TestBed.overrideProvider call), the old providers were retained in the list, since the override takes precedence. However, presence of providers in the list might have side-effect: if a provider has the `ngOnDestroy` lifecycle hook, this hook will be registered and invoked later (when component is destroyed). This commit updates TestBed logic to clear provider list by removing the ones which have overrides.

PR Close #33706
  • Loading branch information
AndrewKushnir authored and kara committed Nov 9, 2019
1 parent 9396b19 commit f45d5dc3318229e51ae5caae8bd6243fbf9e6736
Showing with 51 additions and 15 deletions.
  1. +40 −2 packages/core/test/test_bed_spec.ts
  2. +11 −13 packages/core/testing/src/r3_test_bed_compiler.ts
@@ -16,6 +16,13 @@ import {onlyInIvy} from '@angular/private/testing';

const NAME = new InjectionToken<string>('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: '<hello-world></hello-world>',
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<string[]>('multiToken');
const singleToken = new InjectionToken<string>('singleToken');
@@ -651,18 +651,20 @@ export class R3TestBedCompiler {
const overrides = this.getProviderOverrides(flattenedProviders);
const overriddenProviders = [...flattenedProviders, ...overrides];
const final: Provider[] = [];
const seenMultiProviders = new Set<Provider>();
const seenOverriddenProviders = new Set<Provider>();

// 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<any> {
return value.hasOwnProperty('ngModule');
}

0 comments on commit f45d5dc

Please sign in to comment.
You can’t perform that action at this time.