From 2fd0c9a47d4ef4a0f4b9f1a7ebc4e39126985129 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Fri, 11 Oct 2019 09:43:37 -0700 Subject: [PATCH 1/7] fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 19 ++++++++ .../core/testing/src/r3_test_bed_compiler.ts | 47 +++++-------------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 74673d4b749d5..54ea6e1ae8bbf 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -231,6 +231,25 @@ describe('TestBed', () => { expect(hello.nativeElement).toHaveText('Hello injected World !'); }); + it('allow to override multi provider', () => { + const token = new InjectionToken('token'); + @NgModule({providers: [{provide: token, useValue: 'valueFromModule', multi: true}]}) + class MyModule { + } + + @NgModule({providers: [{provide: token, useValue: 'valueFromModule2', multi: true}]}) + class MyModule2 { + } + + TestBed.configureTestingModule({imports: [MyModule, MyModule2]}); + const overrideValue = ['override']; + TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); + + const value = TestBed.inject(token); + expect(value.length).toEqual(overrideValue.length); + expect(value).toEqual(overrideValue); + }); + it('should allow overriding a provider defined via ModuleWithProviders (using TestBed.overrideProvider)', () => { const serviceOverride = { diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index 0c510b16c850e..f0630fd19f1eb 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -154,14 +154,12 @@ export class R3TestBedCompiler { overrideProvider( token: any, provider: {useFactory?: Function, useValue?: any, deps?: any[], multi?: boolean}): void { - const providerDef = provider.useFactory ? - { - provide: token, - useFactory: provider.useFactory, - deps: provider.deps || [], - multi: provider.multi, - } : - {provide: token, useValue: provider.useValue, multi: provider.multi}; + const providerDef = provider.useFactory ? { + provide: token, + useFactory: provider.useFactory, + deps: provider.deps || [], + } : + {provide: token, useValue: provider.useValue}; let injectableDef: InjectableDef|null; const isRoot = @@ -620,39 +618,18 @@ export class R3TestBedCompiler { if (!providers || !providers.length || this.providerOverridesByToken.size === 0) return []; const overrides = this.getProviderOverrides(providers); - const hasMultiProviderOverrides = overrides.some(isMultiProvider); const overriddenProviders = [...providers, ...overrides]; + const tokenToProviderMap: Map = new Map(); - // No additional processing is required in case we have no multi providers to override - if (!hasMultiProviderOverrides) { - return overriddenProviders; - } - - const final: Provider[] = []; - const seenMultiProviders = 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 fiter out all - // multi providers that have overrides, keeping overridden values only. + // Iterate through providers from the end so only the most recent provider is used for a given + // token and overrides are processed first. forEachRight(overriddenProviders, (provider: any) => { const token: any = getProviderToken(provider); - if (isMultiProvider(provider) && this.providerOverridesByToken.has(token)) { - if (!seenMultiProviders.has(token)) { - seenMultiProviders.add(token); - if (provider && provider.useValue && Array.isArray(provider.useValue)) { - forEachRight(provider.useValue, (value: any) => { - // Unwrap provider override array into individual providers in final set. - final.unshift({provide: token, useValue: value, multi: true}); - }); - } else { - final.unshift(provider); - } - } - } else { - final.unshift(provider); + if (!tokenToProviderMap.has(token)) { + tokenToProviderMap.set(token, provider); } }); - return final; + return Array.from(tokenToProviderMap.values()); } private hasProviderOverrides(providers?: Provider[]): boolean { From a4cde902429086715bbdc16aef9257a489f2bef1 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Fri, 11 Oct 2019 13:31:37 -0700 Subject: [PATCH 2/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 28 ++++++++++++++----- .../core/testing/src/r3_test_bed_compiler.ts | 15 ++++++---- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 54ea6e1ae8bbf..748ddc65ec237 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -231,7 +231,7 @@ describe('TestBed', () => { expect(hello.nativeElement).toHaveText('Hello injected World !'); }); - it('allow to override multi provider', () => { + describe('allow override of multi provider', () => { const token = new InjectionToken('token'); @NgModule({providers: [{provide: token, useValue: 'valueFromModule', multi: true}]}) class MyModule { @@ -241,13 +241,27 @@ describe('TestBed', () => { class MyModule2 { } - TestBed.configureTestingModule({imports: [MyModule, MyModule2]}); - const overrideValue = ['override']; - TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); + beforeEach(() => { TestBed.configureTestingModule({imports: [MyModule, MyModule2]}); }); + + it('with an array', () => { + const overrideValue = ['override']; + TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); + + const value = TestBed.inject(token); + expect(value.length).toEqual(overrideValue.length); + expect(value).toEqual(overrideValue); + }); + + it('with a non-array', () => { + // This is actually invalid because multi providers return arrays. We have this here so we can + // ensure Ivy behaves the same as VE does currently. + const overrideValue = 'override'; + TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); - const value = TestBed.inject(token); - expect(value.length).toEqual(overrideValue.length); - expect(value).toEqual(overrideValue); + const value = TestBed.inject(token); + expect(value.length).toEqual(overrideValue.length); + expect(value).toEqual(overrideValue as {} as string[]); + }); }); it('should allow overriding a provider defined via ModuleWithProviders (using TestBed.overrideProvider)', diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index f0630fd19f1eb..f76268abaa5d7 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -154,12 +154,15 @@ export class R3TestBedCompiler { overrideProvider( token: any, provider: {useFactory?: Function, useValue?: any, deps?: any[], multi?: boolean}): void { - const providerDef = provider.useFactory ? { - provide: token, - useFactory: provider.useFactory, - deps: provider.deps || [], - } : - {provide: token, useValue: provider.useValue}; + // Note, we explicitly avoid setting `multi: true` here. If multi is true and we set it, TestBed + // will override + // all instances of the token and when the test module is finalized, the injector will add each + // occurrence of the + // override to the records. We only ever want one when it's overridden, so we treat it as + // non-multi. + const providerDef = provider.useFactory ? + {provide: token, useFactory: provider.useFactory, deps: provider.deps || []} : + {provide: token, useValue: provider.useValue}; let injectableDef: InjectableDef|null; const isRoot = From 14438f347a3b6128d46e601379098f5cdbef1ea5 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Fri, 18 Oct 2019 14:31:28 -0700 Subject: [PATCH 3/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 25 +++++++++++++++++-- .../core/testing/src/r3_test_bed_compiler.ts | 5 ++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 748ddc65ec237..078348b438a32 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -225,10 +225,31 @@ describe('TestBed', () => { }); it('allow to override a provider', () => { - TestBed.overrideProvider(NAME, {useValue: 'injected World !'}); + TestBed.overrideProvider(NAME, {useValue: 'injected World!'}); const hello = TestBed.createComponent(HelloWorld); hello.detectChanges(); - expect(hello.nativeElement).toHaveText('Hello injected World !'); + expect(hello.nativeElement).toHaveText('Hello injected World!'); + }); + + it('uses the most recent provider override', () => { + TestBed.overrideProvider(NAME, {useValue: 'injected World!'}); + TestBed.overrideProvider(NAME, {useValue: 'injected World a second time!'}); + const hello = TestBed.createComponent(HelloWorld); + hello.detectChanges(); + expect(hello.nativeElement).toHaveText('Hello injected World a second time!'); + }); + + it('overrides a providers in an array', () => { + TestBed.configureTestingModule({ + imports: [HelloWorldModule], + providers: [ + [{provide: NAME, useValue: 'injected World!'}], + ] + }); + TestBed.overrideProvider(NAME, {useValue: 'injected World a second time!'}); + const hello = TestBed.createComponent(HelloWorld); + hello.detectChanges(); + expect(hello.nativeElement).toHaveText('Hello injected World a second time!'); }); describe('allow override of multi provider', () => { diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index f76268abaa5d7..3be0e64d67b7c 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -620,8 +620,9 @@ export class R3TestBedCompiler { private getOverriddenProviders(providers?: Provider[]): Provider[] { if (!providers || !providers.length || this.providerOverridesByToken.size === 0) return []; - const overrides = this.getProviderOverrides(providers); - const overriddenProviders = [...providers, ...overrides]; + const flattenedProviders = flatten(providers); + const overrides = this.getProviderOverrides(flattenedProviders); + const overriddenProviders = [...flattenedProviders, ...overrides]; const tokenToProviderMap: Map = new Map(); // Iterate through providers from the end so only the most recent provider is used for a given From bc8b418a7d0f2a8b0f98ae6faec6464c6d8f42a3 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 21 Oct 2019 09:14:27 -0700 Subject: [PATCH 4/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 33 +++++++++++------ .../core/testing/src/r3_test_bed_compiler.ts | 36 +++++++++++-------- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 078348b438a32..df3d9c455bb5e 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -252,34 +252,47 @@ describe('TestBed', () => { expect(hello.nativeElement).toHaveText('Hello injected World a second time!'); }); - describe('allow override of multi provider', () => { - const token = new InjectionToken('token'); - @NgModule({providers: [{provide: token, useValue: 'valueFromModule', multi: true}]}) + describe('multi providers', () => { + const multiToken = new InjectionToken('multiToken'); + const singleToken = new InjectionToken('singleToken'); + @NgModule({providers: [{provide: multiToken, useValue: 'valueFromModule', multi: true}]}) class MyModule { } - @NgModule({providers: [{provide: token, useValue: 'valueFromModule2', multi: true}]}) + @NgModule({ + providers: [ + {provide: singleToken, useValue: 't1'}, + {provide: multiToken, useValue: 'valueFromModule2', multi: true}, + {provide: multiToken, useValue: 'secondValueFromModule2', multi: true} + ] + }) class MyModule2 { } beforeEach(() => { TestBed.configureTestingModule({imports: [MyModule, MyModule2]}); }); - it('with an array', () => { + it('is preserved when other provider is overridden', () => { + TestBed.overrideProvider(singleToken, {useValue: ''}); + const value = TestBed.inject(multiToken); + expect(value.length).toEqual(3); + }); + + it('overridden with an array', () => { const overrideValue = ['override']; - TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); + TestBed.overrideProvider(multiToken, { useValue: overrideValue, multi: true } as any); - const value = TestBed.inject(token); + const value = TestBed.inject(multiToken); expect(value.length).toEqual(overrideValue.length); expect(value).toEqual(overrideValue); }); - it('with a non-array', () => { + it('overridden with a non-array', () => { // This is actually invalid because multi providers return arrays. We have this here so we can // ensure Ivy behaves the same as VE does currently. const overrideValue = 'override'; - TestBed.overrideProvider(token, { useValue: overrideValue, multi: true } as any); + TestBed.overrideProvider(multiToken, { useValue: overrideValue, multi: true } as any); - const value = TestBed.inject(token); + const value = TestBed.inject(multiToken); expect(value.length).toEqual(overrideValue.length); expect(value).toEqual(overrideValue as {} as string[]); }); diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index 3be0e64d67b7c..0b31a25ac4031 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -154,15 +154,14 @@ export class R3TestBedCompiler { overrideProvider( token: any, provider: {useFactory?: Function, useValue?: any, deps?: any[], multi?: boolean}): void { - // Note, we explicitly avoid setting `multi: true` here. If multi is true and we set it, TestBed - // will override - // all instances of the token and when the test module is finalized, the injector will add each - // occurrence of the - // override to the records. We only ever want one when it's overridden, so we treat it as - // non-multi. const providerDef = provider.useFactory ? - {provide: token, useFactory: provider.useFactory, deps: provider.deps || []} : - {provide: token, useValue: provider.useValue}; + { + provide: token, + useFactory: provider.useFactory, + deps: provider.deps || [], + multi: provider.multi + } : + {provide: token, useValue: provider.useValue, multi: provider.multi}; let injectableDef: InjectableDef|null; const isRoot = @@ -623,17 +622,26 @@ export class R3TestBedCompiler { const flattenedProviders = flatten(providers); const overrides = this.getProviderOverrides(flattenedProviders); const overriddenProviders = [...flattenedProviders, ...overrides]; - const tokenToProviderMap: Map = new Map(); + const final: Provider[] = []; + const seenMultiProviders = new Set(); - // Iterate through providers from the end so only the most recent provider is used for a given - // token and overrides are processed first. + // 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. forEachRight(overriddenProviders, (provider: any) => { const token: any = getProviderToken(provider); - if (!tokenToProviderMap.has(token)) { - tokenToProviderMap.set(token, 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); + final.unshift({...provider, multi: false}); + } + } else { + final.unshift(provider); } }); - return Array.from(tokenToProviderMap.values()); + return final; } private hasProviderOverrides(providers?: Provider[]): boolean { From 1cd2b43f64929b001187b1da3fb05fff8e2d2b39 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 22 Oct 2019 09:36:50 -0700 Subject: [PATCH 5/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 30 +++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index df3d9c455bb5e..7428945cedc91 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -6,13 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, ModuleWithProviders, NgModule, Optional, Pipe, getModuleFactory, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; +import {APP_INITIALIZER, Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, ModuleWithProviders, NgModule, Optional, Pipe, getModuleFactory, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; import {registerModuleFactory} from '@angular/core/src/linker/ng_module_factory_registration'; import {NgModuleFactory} from '@angular/core/src/render3'; import {TestBed, getTestBed} from '@angular/core/testing/src/test_bed'; import {By} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {onlyInIvy} from '@angular/private/testing'; +import {RouterModule} from '@angular/router'; const NAME = new InjectionToken('name'); @@ -269,7 +270,12 @@ describe('TestBed', () => { class MyModule2 { } - beforeEach(() => { TestBed.configureTestingModule({imports: [MyModule, MyModule2]}); }); + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [MyModule, MyModule2], + providers: [{provide: multiToken, useValue: ['override'], multi: true}] + }); + }); it('is preserved when other provider is overridden', () => { TestBed.overrideProvider(singleToken, {useValue: ''}); @@ -298,6 +304,26 @@ describe('TestBed', () => { }); }); + it('overrides multi provider in forRoot', () => { + const TOKEN = new InjectionToken('token'); + @NgModule() + class MyMod { + static forRoot() { + return { + ngModule: MyMod, + providers: [[{provide: TOKEN, multi: true, useValue: 'forRootValue'}]] + }; + } + } + @NgModule({imports: [MyMod.forRoot()]}) + class MyMod2 { + } + + TestBed.configureTestingModule({imports: [MyMod2]}); + TestBed.overrideProvider(TOKEN, { useValue: ['override'], multi: true } as any); + expect(TestBed.inject(TOKEN)).toEqual(['override']); + }); + it('should allow overriding a provider defined via ModuleWithProviders (using TestBed.overrideProvider)', () => { const serviceOverride = { From e70383472914d52b6bdd2f8fc39a1bf71b20533a Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 22 Oct 2019 12:23:49 -0700 Subject: [PATCH 6/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 37 +++++++++++++------ .../core/testing/src/r3_test_bed_compiler.ts | 11 +++++- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 7428945cedc91..11a752041fdc9 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -6,14 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_INITIALIZER, Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, ModuleWithProviders, NgModule, Optional, Pipe, getModuleFactory, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; +import {Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, ModuleWithProviders, NgModule, Optional, Pipe, getModuleFactory, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; import {registerModuleFactory} from '@angular/core/src/linker/ng_module_factory_registration'; import {NgModuleFactory} from '@angular/core/src/render3'; import {TestBed, getTestBed} from '@angular/core/testing/src/test_bed'; import {By} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {onlyInIvy} from '@angular/private/testing'; -import {RouterModule} from '@angular/router'; const NAME = new InjectionToken('name'); @@ -273,7 +272,6 @@ describe('TestBed', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [MyModule, MyModule2], - providers: [{provide: multiToken, useValue: ['override'], multi: true}] }); }); @@ -304,24 +302,41 @@ describe('TestBed', () => { }); }); - it('overrides multi provider in forRoot', () => { + describe('overrides providers in ModuleWithProviders', () => { const TOKEN = new InjectionToken('token'); @NgModule() class MyMod { + static multi = false; + static forRoot() { return { ngModule: MyMod, - providers: [[{provide: TOKEN, multi: true, useValue: 'forRootValue'}]] + providers: [{provide: TOKEN, multi: MyMod.multi, useValue: 'forRootValue'}] }; } } - @NgModule({imports: [MyMod.forRoot()]}) - class MyMod2 { - } - TestBed.configureTestingModule({imports: [MyMod2]}); - TestBed.overrideProvider(TOKEN, { useValue: ['override'], multi: true } as any); - expect(TestBed.inject(TOKEN)).toEqual(['override']); + beforeEach(() => MyMod.multi = false); + + it('when provider is a "regular" provider', () => { + MyMod.multi = false; + @NgModule({imports: [MyMod.forRoot()]}) + class MyMod2 { + } + TestBed.configureTestingModule({imports: [MyMod2]}); + TestBed.overrideProvider(TOKEN, {useValue: ['override']}); + expect(TestBed.inject(TOKEN)).toEqual(['override']); + }); + + it('when provider is multi', () => { + MyMod.multi = true; + @NgModule({imports: [MyMod.forRoot()]}) + class MyMod2 { + } + TestBed.configureTestingModule({imports: [MyMod2]}); + TestBed.overrideProvider(TOKEN, {useValue: ['override']}); + expect(TestBed.inject(TOKEN)).toEqual(['override']); + }); }); it('should allow overriding a provider defined via ModuleWithProviders (using TestBed.overrideProvider)', diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index 0b31a25ac4031..9e5db8597c55a 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -381,8 +381,15 @@ export class R3TestBedCompiler { // Apply provider overrides to imported modules recursively const moduleDef: any = (moduleType as any)[NG_MOD_DEF]; - for (const importType of moduleDef.imports) { - this.applyProviderOverridesToModule(importType); + for (const importedModule of moduleDef.imports) { + this.applyProviderOverridesToModule(importedModule); + } + // Also override the providers on any ModuleWithProviders imports since those don't appear in + // the moduleDef. + for (const importedModule of flatten(injectorDef.imports)) { + if (isModuleWithProviders(importedModule)) { + importedModule.providers = this.getOverriddenProviders(importedModule.providers); + } } } } From 199a407297f1252586a55f73ac9416c2a9135d97 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 24 Oct 2019 12:31:27 -0700 Subject: [PATCH 7/7] fixup! fix(ivy): ensure overrides for 'multi: true' only appear once in final providers --- packages/core/test/test_bed_spec.ts | 16 ++++++++++++-- .../core/testing/src/r3_test_bed_compiler.ts | 21 ++++++++++++------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 11a752041fdc9..4251d4c7c1ca5 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -316,7 +316,7 @@ describe('TestBed', () => { } } - beforeEach(() => MyMod.multi = false); + beforeEach(() => MyMod.multi = true); it('when provider is a "regular" provider', () => { MyMod.multi = false; @@ -329,7 +329,6 @@ describe('TestBed', () => { }); it('when provider is multi', () => { - MyMod.multi = true; @NgModule({imports: [MyMod.forRoot()]}) class MyMod2 { } @@ -337,6 +336,19 @@ describe('TestBed', () => { TestBed.overrideProvider(TOKEN, {useValue: ['override']}); expect(TestBed.inject(TOKEN)).toEqual(['override']); }); + + it('restores the original value', () => { + @NgModule({imports: [MyMod.forRoot()]}) + class MyMod2 { + } + TestBed.configureTestingModule({imports: [MyMod2]}); + TestBed.overrideProvider(TOKEN, {useValue: ['override']}); + expect(TestBed.inject(TOKEN)).toEqual(['override']); + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({imports: [MyMod2]}); + expect(TestBed.inject(TOKEN)).toEqual(['forRootValue']); + }); }); it('should allow overriding a provider defined via ModuleWithProviders (using TestBed.overrideProvider)', diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index 9e5db8597c55a..d6f2da644e4f5 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -35,9 +35,9 @@ type Resolvers = { }; interface CleanupOperation { - field: string; - def: any; - original: unknown; + fieldName: string; + object: any; + originalValue: unknown; } export class R3TestBedCompiler { @@ -388,6 +388,11 @@ export class R3TestBedCompiler { // the moduleDef. for (const importedModule of flatten(injectorDef.imports)) { if (isModuleWithProviders(importedModule)) { + this.defCleanupOps.push({ + object: importedModule, + fieldName: 'providers', + originalValue: importedModule.providers + }); importedModule.providers = this.getOverriddenProviders(importedModule.providers); } } @@ -492,10 +497,10 @@ export class R3TestBedCompiler { } } - private storeFieldOfDefOnType(type: Type, defField: string, field: string): void { + private storeFieldOfDefOnType(type: Type, defField: string, fieldName: string): void { const def: any = (type as any)[defField]; - const original: any = def[field]; - this.defCleanupOps.push({field, def, original}); + const originalValue: any = def[fieldName]; + this.defCleanupOps.push({object: def, fieldName, originalValue}); } /** @@ -526,7 +531,9 @@ export class R3TestBedCompiler { restoreOriginalState(): void { // Process cleanup ops in reverse order so the field's original value is restored correctly (in // case there were multiple overrides for the same field). - forEachRight(this.defCleanupOps, (op: CleanupOperation) => { op.def[op.field] = op.original; }); + forEachRight(this.defCleanupOps, (op: CleanupOperation) => { + op.object[op.fieldName] = op.originalValue; + }); // Restore initial component/directive/pipe defs this.initialNgDefs.forEach( (value: [string, PropertyDescriptor | undefined], type: Type) => {