From 8e2280db056712f0428abf6ba2626f5fcac656c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Mon, 18 Mar 2019 10:47:05 +0100 Subject: [PATCH] bugfix(core/testing): override provider only if exists in the scope --- packages/core/injector/module.ts | 33 ++++++++++++++++----- packages/core/test/injector/module.spec.ts | 34 ++++++++++++++++++++++ packages/testing/testing-module.builder.ts | 4 +-- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/packages/core/injector/module.ts b/packages/core/injector/module.ts index 1fb4112e12e..46b6c84eccc 100644 --- a/packages/core/injector/module.ts +++ b/packages/core/injector/module.ts @@ -207,9 +207,7 @@ export class Module { provider: CustomFactory | CustomValue | CustomClass, collection: Map, ): string { - const { provide } = provider; - const name = isFunction(provide) ? provide.name : provide; - + const name = this.getProviderStaticToken(provider.provide) as string; provider = { ...provider, name, @@ -359,13 +357,32 @@ export class Module { } public replace(toReplace: string | symbol | Type, options: any) { - if (options.isProvider) { + if (options.isProvider && this.hasProvider(toReplace)) { return this.addProvider({ provide: toReplace, ...options }); + } else if (!options.isProvider && this.hasInjectable(toReplace)) { + this.addInjectable({ + provide: toReplace, + ...options, + }); } - this.addInjectable({ - provide: toReplace, - ...options, - }); + } + + public hasProvider(token: string | symbol | Type): boolean { + const name = this.getProviderStaticToken(token); + return this._providers.has(name); + } + + public hasInjectable(token: string | symbol | Type): boolean { + const name = this.getProviderStaticToken(token); + return this._injectables.has(name); + } + + public getProviderStaticToken( + provider: string | symbol | Type, + ): string | symbol { + return isFunction(provider) + ? (provider as Function).name + : (provider as string | symbol); } public getProviderByKey(name: string | symbol): InstanceWrapper { diff --git a/packages/core/test/injector/module.spec.ts b/packages/core/test/injector/module.spec.ts index b156a7890c8..b3b27b473ea 100644 --- a/packages/core/test/injector/module.spec.ts +++ b/packages/core/test/injector/module.spec.ts @@ -274,6 +274,8 @@ describe('Module', () => { describe('when provider', () => { it('should call `addProvider`', () => { const addProviderSpy = sinon.spy(module, 'addProvider'); + sinon.stub(module, 'hasProvider').callsFake(() => true); + module.replace(null, { isProvider: true }); expect(addProviderSpy.called).to.be.true; }); @@ -281,6 +283,8 @@ describe('Module', () => { describe('when guard', () => { it('should call `addInjectable`', () => { const addInjectableSpy = sinon.spy(module, 'addInjectable'); + sinon.stub(module, 'hasInjectable').callsFake(() => true); + module.replace(null, {}); expect(addInjectableSpy.called).to.be.true; }); @@ -379,4 +383,34 @@ describe('Module', () => { }); }); }); + + describe('hasProvider', () => { + describe('when module has provider', () => { + it('should return true', () => { + const token = 'test'; + module.providers.set(token, new InstanceWrapper()); + expect(module.hasProvider(token)).to.be.true; + }); + }); + describe('otherwise', () => { + it('should return false', () => { + expect(module.hasProvider('_')).to.be.false; + }); + }); + }); + + describe('hasInjectable', () => { + describe('when module has injectable', () => { + it('should return true', () => { + const token = 'test'; + module.injectables.set(token, new InstanceWrapper()); + expect(module.hasInjectable(token)).to.be.true; + }); + }); + describe('otherwise', () => { + it('should return false', () => { + expect(module.hasInjectable('_')).to.be.false; + }); + }); + }); }); diff --git a/packages/testing/testing-module.builder.ts b/packages/testing/testing-module.builder.ts index 62fc794ede2..b3286038131 100644 --- a/packages/testing/testing-module.builder.ts +++ b/packages/testing/testing-module.builder.ts @@ -81,8 +81,8 @@ export class TestingModuleBuilder { } private applyOverloadsMap() { - [...this.overloadsMap.entries()].forEach(([component, options]) => { - this.container.replace(component, options); + [...this.overloadsMap.entries()].forEach(([item, options]) => { + this.container.replace(item, options); }); }