Repository: nestjs/nest
Pull request #9915 (2022-07-11 20:19:28)

refactor(core): extract instance wrapper merge logic
## PR Checklist
Please check if your PR fulfills the following requirements:

- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [x] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)


## PR Type
What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->
- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [x] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Other... Please describe:

## What is the current behavior?
The `InstanceWrapper#mergeWith` method was using some verifications to determine how it would merge with a given provider. I extracted this logic to another file to improve its readability and also because it could be potentially used by other classes.

Moreover, I added some missing test cases for it.


## What is the new behavior?
No new behavior, only refactor of existing one.

## Does this PR introduce a breaking change?
- [ ] Yes
- [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->


## Other information


Patch:
diff --git a/packages/core/injector/helpers/provider-classifier.ts b/packages/core/injector/helpers/provider-classifier.ts
new file mode 100644
index 000000000..b883a4ea8
--- /dev/null
+++ b/packages/core/injector/helpers/provider-classifier.ts
@@ -0,0 +1,26 @@
+import {
+  ClassProvider,
+  FactoryProvider,
+  Provider,
+  ValueProvider,
+} from '@nestjs/common';
+import { isUndefined } from '@nestjs/common/utils/shared.utils';
+
+export function isClassProvider<T = any>(
+  provider: Provider,
+): provider is ClassProvider<T> {
+  return Boolean((provider as ClassProvider<T>)?.useClass);
+}
+
+export function isValueProvider<T = any>(
+  provider: Provider,
+): provider is ValueProvider<T> {
+  const providerValue = (provider as ValueProvider)?.useValue;
+  return !isUndefined(providerValue);
+}
+
+export function isFactoryProvider<T = any>(
+  provider: Provider,
+): provider is FactoryProvider<T> {
+  return Boolean((provider as FactoryProvider).useFactory);
+}
diff --git a/packages/core/injector/instance-wrapper.ts b/packages/core/injector/instance-wrapper.ts
index 2357ac198..773a9b4f5 100644
--- a/packages/core/injector/instance-wrapper.ts
+++ b/packages/core/injector/instance-wrapper.ts
@@ -13,6 +13,11 @@ import {
 } from '@nestjs/common/utils/shared.utils';
 import { iterate } from 'iterare';
 import { STATIC_CONTEXT } from './constants';
+import {
+  isClassProvider,
+  isFactoryProvider,
+  isValueProvider,
+} from './helpers/provider-classifier';
 import { InstanceToken, Module } from './module';
 
 export const INSTANCE_METADATA_SYMBOL = Symbol.for('instance_metadata:cache');
@@ -384,22 +389,23 @@ export class InstanceWrapper<T = any> {
   }
 
   public mergeWith(provider: Provider) {
-    if (!isUndefined((provider as ValueProvider).useValue)) {
+    if (isValueProvider(provider)) {
       this.metatype = null;
       this.inject = null;
+
       this.scope = Scope.DEFAULT;
 
       this.setInstanceByContextId(STATIC_CONTEXT, {
-        instance: (provider as ValueProvider).useValue,
+        instance: provider.useValue,
         isResolved: true,
         isPending: false,
       });
-    } else if ((provider as ClassProvider).useClass) {
+    } else if (isClassProvider(provider)) {
       this.inject = null;
-      this.metatype = (provider as ClassProvider).useClass;
-    } else if ((provider as FactoryProvider).useFactory) {
-      this.metatype = (provider as FactoryProvider).useFactory;
-      this.inject = (provider as FactoryProvider).inject || [];
+      this.metatype = provider.useClass;
+    } else if (isFactoryProvider(provider)) {
+      this.metatype = provider.useFactory;
+      this.inject = provider.inject || [];
     }
   }
 


Test patch:
diff --git a/packages/core/test/injector/helpers/provider-classifier.spec.ts b/packages/core/test/injector/helpers/provider-classifier.spec.ts
new file mode 100644
index 000000000..091600bfb
--- /dev/null
+++ b/packages/core/test/injector/helpers/provider-classifier.spec.ts
@@ -0,0 +1,132 @@
+import { ClassProvider, FactoryProvider, ValueProvider } from '@nestjs/common';
+import { expect } from 'chai';
+import {
+  isClassProvider,
+  isFactoryProvider,
+  isValueProvider,
+} from '../../../injector/helpers/provider-classifier';
+
+describe('provider classifier', () => {
+  describe('isClassProvider', () => {
+    it('should return true if useClass is present', () => {
+      const classProvider: ClassProvider = {
+        useClass: class TestClass {},
+        provide: 'token',
+      };
+
+      expect(isClassProvider(classProvider)).to.be.true;
+    });
+
+    it('should return false if useClass is undefined', () => {
+      const classProvider: ClassProvider = {
+        useClass: undefined,
+        provide: 'token',
+      };
+
+      expect(isClassProvider(classProvider)).to.be.false;
+    });
+
+    it('should return false if useClass is not present', () => {
+      const classProvider = {
+        provide: 'token',
+      };
+
+      expect(isClassProvider(classProvider as ClassProvider)).to.be.false;
+    });
+
+    it('should return false if provider is undefined', () => {
+      const classProvider = undefined;
+
+      expect(isClassProvider(classProvider as ClassProvider)).to.be.false;
+    });
+  });
+
+  describe('isValueProvider', () => {
+    it('should return true if useValue is not undefined', () => {
+      const valueProvider: ValueProvider = {
+        useValue: 'value',
+        provide: 'token',
+      };
+
+      expect(isValueProvider(valueProvider)).to.be.true;
+    });
+
+    it('should return true if useValue is "false"', () => {
+      const valueProvider: ValueProvider = {
+        useValue: false,
+        provide: 'token',
+      };
+
+      expect(isValueProvider(valueProvider)).to.be.true;
+    });
+
+    it('should return true if useValue is "null"', () => {
+      const valueProvider: ValueProvider = {
+        useValue: null,
+        provide: 'token',
+      };
+
+      expect(isValueProvider(valueProvider)).to.be.true;
+    });
+
+    it('should return true if useValue is an empty string', () => {
+      const valueProvider: ValueProvider = {
+        useValue: null,
+        provide: '',
+      };
+
+      expect(isValueProvider(valueProvider)).to.be.true;
+    });
+
+    it('should return false if useValue is undefined', () => {
+      const valueProvider: ValueProvider = {
+        useValue: undefined,
+        provide: 'token',
+      };
+
+      expect(isValueProvider(valueProvider)).to.be.false;
+    });
+
+    it('should return false if useValue is not present', () => {
+      const valueProvider = {
+        provide: 'token',
+      };
+
+      expect(isValueProvider(valueProvider as ValueProvider)).to.be.false;
+    });
+
+    it('should return false if provider is undefined', () => {
+      const valueProvider = undefined;
+
+      expect(isValueProvider(valueProvider as ValueProvider)).to.be.false;
+    });
+  });
+
+  describe('isFactoryProvider', () => {
+    it('should return true if useFactory is present', () => {
+      const factoryProvider: FactoryProvider = {
+        provide: 'token',
+        useFactory: () => {},
+      };
+
+      expect(isFactoryProvider(factoryProvider)).to.be.true;
+    });
+
+    it('should return false if useFactory is not present', () => {
+      const factoryProvider = {
+        provide: 'token',
+      };
+
+      expect(isFactoryProvider(factoryProvider as FactoryProvider)).to.be.false;
+    });
+
+    it('should return false if useFactory is undefined', () => {
+      const factoryProvider: FactoryProvider = {
+        provide: 'token',
+        useFactory: undefined,
+      };
+
+      expect(isFactoryProvider(factoryProvider as FactoryProvider)).to.be.false;
+    });
+  });
+});
diff --git a/packages/core/test/injector/instance-wrapper.spec.ts b/packages/core/test/injector/instance-wrapper.spec.ts
index 6b0e1734c..d5ab67c15 100644
--- a/packages/core/test/injector/instance-wrapper.spec.ts
+++ b/packages/core/test/injector/instance-wrapper.spec.ts
@@ -434,4 +434,68 @@ describe('InstanceWrapper', () => {
       });
     });
   });
+
+  describe('mergeWith', () => {
+    describe('when provider is a ValueProvider', () => {
+      it('should provide the given value in the STATIC_CONTEXT', () => {
+        const wrapper = new InstanceWrapper();
+        wrapper.mergeWith({
+          useValue: 'value',
+          provide: 'token',
+        });
+
+        expect(
+          wrapper.getInstanceByContextId(STATIC_CONTEXT).instance,
+        ).to.be.equal('value');
+      });
+    });
+
+    describe('when provider is a ClassProvider', () => {
+      it('should alter the instance wrapper metatype with the given class', () => {
+        const wrapper = new InstanceWrapper();
+
+        wrapper.mergeWith({
+          useClass: TestClass,
+          provide: 'token',
+        });
+
+        expect(wrapper.metatype).to.be.eql(TestClass);
+      });
+    });
+
+    describe('when provider is a FactoryProvider', () => {
+      describe('and it has injected dependencies', () => {
+        it('should alter the instance wrapper metatype and inject attributes with the given values', () => {
+          const wrapper = new InstanceWrapper();
+
+          const factory = (_dependency1: any, _dependency2: any) => {};
+          const injectedDependencies = ['dependency1', 'dependency2'];
+
+          wrapper.mergeWith({
+            provide: 'token',
+            useFactory: factory,
+            inject: injectedDependencies,
+          });
+
+          expect(wrapper.metatype).to.be.eql(factory);
+          expect(wrapper.inject).to.be.eq(injectedDependencies);
+        });
+      });
+
+      describe('and it has no injected dependencies', () => {
+        it('should alter the instance wrapper metatype with the given values', () => {
+          const wrapper = new InstanceWrapper();
+          const factory = (_dependency1: any, _dependency2: any) => {};
+
+          wrapper.mergeWith({
+            provide: 'token',
+            useFactory: factory,
+          });
+
+          expect(wrapper.metatype).to.be.eql(factory);
+          expect(wrapper.inject).to.be.eql([]);
+        });
+      });
+    });
+  });
 });