fix(core): fix invalid transient & request-scoped behavior nestjs#3303

Author: kamilmysliwiec

integration/scopes/e2e/transient-scope.spec.ts

@@ -62,16 +62,16 @@ describe('Transient scope', () => {
       expect(Meta.COUNTER).to.be.eql(7);
     });
 
-    it(`should create transient pipe for each controller`, async () => {
-      expect(UserByIdPipe.COUNTER).to.be.eql(2);
+    it(`should create transient pipe for each controller (3 requests, 1 static)`, async () => {
+      expect(UserByIdPipe.COUNTER).to.be.eql(4);
     });
 
-    it(`should create transient interceptor for each controller`, async () => {
-      expect(Interceptor.COUNTER).to.be.eql(2);
+    it(`should create transient interceptor for each controller (3 requests, 1 static)`, async () => {
+      expect(Interceptor.COUNTER).to.be.eql(4);
     });
 
-    it(`should create transient guard for each controller`, async () => {
-      expect(Guard.COUNTER).to.be.eql(2);
+    it(`should create transient guard for each controller (3 requests, 1 static)`, async () => {
+      expect(Guard.COUNTER).to.be.eql(4);
     });
   });
 

packages/core/injector/injector.ts

@@ -12,6 +12,7 @@ import {
   isFunction,
   isNil,
   isObject,
+  isString,
   isUndefined,
 } from '@nestjs/common/utils/shared.utils';
 import { RuntimeException } from '../errors/exceptions/runtime.exception';
@@ -242,7 +243,7 @@ export class Injector {
       : [];
 
     let isResolved = true;
-    const resolveParam = async (param, index) => {
+    const resolveParam = async (param: unknown, index: number) => {
       try {
         const paramWrapper = await this.resolveSingleParam<T>(
           wrapper,
@@ -251,6 +252,7 @@ export class Injector {
           module,
           contextId,
           inquirer,
+          index,
         );
         const instanceHost = paramWrapper.getInstanceByContextId(
           contextId,
@@ -259,7 +261,6 @@ export class Injector {
         if (!instanceHost.isResolved && !paramWrapper.forwardRef) {
           isResolved = false;
         }
-        wrapper.addCtorMetadata(index, paramWrapper);
         return instanceHost && instanceHost.instance;
       } catch (err) {
         const isOptional = optionalDependenciesIds.includes(index);
@@ -296,6 +297,7 @@ export class Injector {
     module: Module,
     contextId = STATIC_CONTEXT,
     inquirer?: InstanceWrapper,
+    keyOrIndex?: string | number,
   ) {
     if (isUndefined(param)) {
       throw new UndefinedDependencyException(
@@ -312,6 +314,7 @@ export class Injector {
       wrapper,
       contextId,
       inquirer,
+      keyOrIndex,
     );
   }
 
@@ -333,6 +336,7 @@ export class Injector {
     wrapper: InstanceWrapper<T>,
     contextId = STATIC_CONTEXT,
     inquirer?: InstanceWrapper,
+    keyOrIndex?: string | number,
   ): Promise<InstanceWrapper> {
     const providers = module.providers;
     const instanceWrapper = await this.lookupComponent(
@@ -343,6 +347,10 @@ export class Injector {
       contextId,
       inquirer,
     );
+    isString(keyOrIndex)
+      ? wrapper.addPropertiesMetadata(keyOrIndex, instanceWrapper)
+      : wrapper.addCtorMetadata(keyOrIndex, instanceWrapper);
+
     return this.resolveComponentHost(
       module,
       instanceWrapper,
@@ -535,12 +543,11 @@ export class Injector {
             module,
             contextId,
             inquirer,
+            item.key,
           );
           if (!paramWrapper) {
             return undefined;
           }
-          wrapper.addPropertiesMetadata(item.key, paramWrapper);
-
           const inquirerId = this.getInquirerId(inquirer);
           const instanceHost = paramWrapper.getInstanceByContextId(
             contextId,

packages/core/injector/instance-wrapper.ts

@@ -263,7 +263,7 @@ export class InstanceWrapper<T = any> {
     inquirer: InstanceWrapper | undefined,
   ): boolean {
     const isInquirerRequestScoped =
-      inquirer && inquirer.scope === Scope.REQUEST;
+      inquirer && !inquirer.isDependencyTreeStatic();
 
     return (
       this.isDependencyTreeStatic() &&
@@ -278,7 +278,7 @@ export class InstanceWrapper<T = any> {
     inquirer: InstanceWrapper | undefined,
   ): boolean {
     const isInquirerRequestScoped =
-      inquirer && inquirer.scope === Scope.REQUEST;
+      inquirer && !inquirer.isDependencyTreeStatic();
     const isStaticTransient = this.isTransient && !isInquirerRequestScoped;
 
     return (

packages/core/test/injector/injector.spec.ts

@@ -526,81 +526,84 @@ describe('Injector', () => {
 
     describe('when instanceWrapper is not resolved and does not have forward ref', () => {
       it('should call loadProvider', async () => {
+        const wrapper = new InstanceWrapper({ isResolved: false });
+
         const loadStub = sinon
           .stub(injector, 'loadProvider')
           .callsFake(() => null);
         sinon
           .stub(injector, 'lookupComponent')
-          .returns(Promise.resolve(new InstanceWrapper({ isResolved: false })));
+          .returns(Promise.resolve(wrapper));
 
         await injector.resolveComponentInstance(
           module,
           '',
           { index: 0, dependencies: [] },
-          {} as any,
+          wrapper,
         );
         expect(loadStub.called).to.be.true;
       });
       it('should not call loadProvider (isResolved)', async () => {
+        const wrapper = new InstanceWrapper({ isResolved: true });
         const loadStub = sinon
           .stub(injector, 'loadProvider')
           .callsFake(() => null);
+
         sinon
           .stub(injector, 'lookupComponent')
-          .returns(Promise.resolve(new InstanceWrapper({ isResolved: true })));
+          .returns(Promise.resolve(wrapper));
 
         await injector.resolveComponentInstance(
           module,
           '',
           { index: 0, dependencies: [] },
-          {} as any,
+          wrapper,
         );
         expect(loadStub.called).to.be.false;
       });
       it('should not call loadProvider (forwardRef)', async () => {
+        const wrapper = new InstanceWrapper({
+          isResolved: false,
+          forwardRef: true,
+        });
         const loadStub = sinon
           .stub(injector, 'loadProvider')
           .callsFake(() => null);
+
         sinon
           .stub(injector, 'lookupComponent')
-          .returns(
-            Promise.resolve(
-              new InstanceWrapper({ isResolved: false, forwardRef: true }),
-            ),
-          );
+          .returns(Promise.resolve(wrapper));
 
         await injector.resolveComponentInstance(
           module,
           '',
           { index: 0, dependencies: [] },
-          {} as any,
+          wrapper,
         );
         expect(loadStub.called).to.be.false;
       });
     });
 
     describe('when instanceWraper has async property', () => {
       it('should await instance', async () => {
-        const loadStub = sinon
-          .stub(injector, 'loadProvider')
-          .callsFake(() => null);
+        sinon.stub(injector, 'loadProvider').callsFake(() => null);
 
         const instance = Promise.resolve(true);
-        sinon.stub(injector, 'lookupComponent').returns(
-          Promise.resolve(
-            new InstanceWrapper({
-              isResolved: false,
-              forwardRef: true,
-              async: true,
-              instance,
-            }),
-          ),
-        );
+        const wrapper = new InstanceWrapper({
+          isResolved: false,
+          forwardRef: true,
+          async: true,
+          instance,
+        });
+        sinon
+          .stub(injector, 'lookupComponent')
+          .returns(Promise.resolve(wrapper));
+
         const result = await injector.resolveComponentInstance(
           module,
           '',
           { index: 0, dependencies: [] },
-          {} as any,
+          wrapper,
         );
         expect(result.instance).to.be.true;
       });