Merge pull request MichalLytek#247 from 19majkel94/fix/custom-decorat…

…ors-bug

Fix nullifying other custom method decorators bug

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ## Fixes
 - fix calling return type getter function `@Field(type => Foo)` before finishing module evaluation (allow for extending circular classes using `require`)
+- fix nullifying other custom method decorators - call the method on target instance, not the stored reference to original function (#247)
 
 ## v0.16.0
 ### Features

src/decorators/Field.ts

@@ -51,7 +51,6 @@ export function Field(
         methodName: propertyKey,
         schemaName: options.name || propertyKey,
         target: prototype.constructor,
-        handler: isResolverMethod ? (prototype as any)[propertyKey] : undefined,
         complexity: options.complexity,
       });
     }

src/decorators/FieldResolver.ts

@@ -47,7 +47,6 @@ export function FieldResolver(
       methodName: propertyKey,
       schemaName: options.name || propertyKey,
       target: prototype.constructor,
-      handler: (prototype as any)[propertyKey],
       getType,
       typeOptions,
       complexity: options.complexity,

src/decorators/Mutation.ts

@@ -1,6 +1,6 @@
 import { ReturnTypeFunc, AdvancedOptions } from "./types";
 import { getMetadataStorage } from "../metadata/getMetadataStorage";
-import { getHandlerInfo } from "../helpers/handlers";
+import { getResolverMetadata } from "../helpers/resolver-metadata";
 import { getTypeDecoratorParams } from "../helpers/decorators";
 
 export function Mutation(): MethodDecorator;
@@ -15,7 +15,7 @@ export function Mutation(
 ): MethodDecorator {
   const { options, returnTypeFunc } = getTypeDecoratorParams(returnTypeFuncOrOptions, maybeOptions);
   return (prototype, methodName) => {
-    const handler = getHandlerInfo(prototype, methodName, returnTypeFunc, options);
-    getMetadataStorage().collectMutationHandlerMetadata(handler);
+    const metadata = getResolverMetadata(prototype, methodName, returnTypeFunc, options);
+    getMetadataStorage().collectMutationHandlerMetadata(metadata);
   };
 }

src/decorators/Query.ts

@@ -1,6 +1,6 @@
 import { ReturnTypeFunc, AdvancedOptions } from "./types";
 import { getMetadataStorage } from "../metadata/getMetadataStorage";
-import { getHandlerInfo } from "../helpers/handlers";
+import { getResolverMetadata } from "../helpers/resolver-metadata";
 import { getTypeDecoratorParams } from "../helpers/decorators";
 
 export function Query(): MethodDecorator;
@@ -12,7 +12,7 @@ export function Query(
 ): MethodDecorator {
   const { options, returnTypeFunc } = getTypeDecoratorParams(returnTypeFuncOrOptions, maybeOptions);
   return (prototype, methodName) => {
-    const handler = getHandlerInfo(prototype, methodName, returnTypeFunc, options);
-    getMetadataStorage().collectQueryHandlerMetadata(handler);
+    const metadata = getResolverMetadata(prototype, methodName, returnTypeFunc, options);
+    getMetadataStorage().collectQueryHandlerMetadata(metadata);
   };
 }

src/decorators/Subscription.ts

@@ -5,7 +5,7 @@ import {
   SubscriptionTopicFunc,
 } from "./types";
 import { getMetadataStorage } from "../metadata/getMetadataStorage";
-import { getHandlerInfo } from "../helpers/handlers";
+import { getResolverMetadata } from "../helpers/resolver-metadata";
 import { getTypeDecoratorParams } from "../helpers/decorators";
 import { MissingSubscriptionTopicsError } from "../errors";
 
@@ -25,13 +25,13 @@ export function Subscription(
 ): MethodDecorator {
   const { options, returnTypeFunc } = getTypeDecoratorParams(returnTypeFuncOrOptions, maybeOptions);
   return (prototype, methodName) => {
-    const handler = getHandlerInfo(prototype, methodName, returnTypeFunc, options);
+    const metadata = getResolverMetadata(prototype, methodName, returnTypeFunc, options);
     const subscriptionOptions = options as SubscriptionOptions;
     if (Array.isArray(options.topics) && options.topics.length === 0) {
-      throw new MissingSubscriptionTopicsError(handler.target, handler.methodName);
+      throw new MissingSubscriptionTopicsError(metadata.target, metadata.methodName);
     }
     getMetadataStorage().collectSubscriptionHandlerMetadata({
-      ...handler,
+      ...metadata,
       topics: subscriptionOptions.topics,
       filter: subscriptionOptions.filter,
     });

src/helpers/handlers.ts → src/helpers/resolver-metadata.ts

@@ -3,7 +3,7 @@ import { ReturnTypeFunc, AdvancedOptions } from "../decorators/types";
 import { findType } from "./findType";
 import { SymbolKeysNotSupportedError } from "../errors";
 
-export function getHandlerInfo(
+export function getResolverMetadata(
   prototype: object,
   propertyKey: string | symbol,
   returnTypeFunc?: ReturnTypeFunc,
@@ -26,7 +26,6 @@ export function getHandlerInfo(
   return {
     methodName,
     schemaName: options.name || methodName,
-    handler: (prototype as any)[methodName],
     target: prototype.constructor,
     getReturnType: getType,
     returnTypeOptions: typeOptions,

src/metadata/definitions/resolver-metadata.ts

@@ -13,7 +13,6 @@ export interface BaseResolverMetadata {
   methodName: string;
   schemaName: string;
   target: Function;
-  handler: Function | undefined;
   complexity: Complexity | undefined;
   resolverClassMetadata?: ResolverClassMetadata;
   params?: ParamMetadata[];
@@ -23,7 +22,6 @@ export interface BaseResolverMetadata {
 
 export interface ResolverMetadata extends BaseResolverMetadata {
   getReturnType: TypeValueThunk;
-  handler: Function;
   returnTypeOptions: TypeOptions;
   description?: string;
   deprecationReason?: string;

src/resolvers/create.ts

@@ -34,7 +34,7 @@ export function createHandlerResolver(
         globalValidate,
         pubSub,
       );
-      return resolverMetadata.handler!.apply(targetInstance, params);
+      return targetInstance[resolverMetadata.methodName].apply(targetInstance, params);
     });
   };
 }
@@ -62,18 +62,19 @@ export function createAdvancedFieldResolver(
     const resolverData: ResolverData<any> = { root, args, context, info };
     const targetInstance: any = convertToType(targetType, root);
     return applyMiddlewares(container, resolverData, middlewares, async () => {
+      const handlerOrGetterValue = targetInstance[fieldResolverMetadata.methodName];
       // method
-      if (fieldResolverMetadata.handler) {
+      if (typeof handlerOrGetterValue === "function") {
         const params: any[] = await getParams(
           fieldResolverMetadata.params!,
           resolverData,
           globalValidate,
           pubSub,
         );
-        return fieldResolverMetadata.handler.apply(targetInstance, params);
+        return handlerOrGetterValue.apply(targetInstance, params);
       }
       // getter
-      return targetInstance[fieldResolverMetadata.methodName];
+      return handlerOrGetterValue;
     });
   };
 }

tests/functional/resolvers.ts

@@ -1082,14 +1082,27 @@ describe("Resolvers", () => {
     let queryRoot: any;
     let queryContext: any;
     let queryInfo: any;
+    let descriptorEvaluated: boolean;
+
+    function DescriptorDecorator(): MethodDecorator {
+      return (obj, methodName, descriptor: any) => {
+        const originalMethod: Function = descriptor.value;
+        descriptor.value = function() {
+          descriptorEvaluated = true;
+          return originalMethod.apply(this, arguments);
+        };
+      };
+    }
 
     // helpers
     function generateAndVisitComplexMethod(maximumComplexity: number): ValidationContext {
-      const query = `query {
-                          sampleQuery {
-                          complexResolverMethod
-                          }
-                        }`;
+      const query = /* graphql */ `
+        query {
+          sampleQuery {
+            complexResolverMethod
+          }
+        }
+      `;
       const ast = parse(query);
       const typeInfo = new TypeInfo(schema);
       const context = new ValidationContext(schema, ast, typeInfo);
@@ -1105,6 +1118,7 @@ describe("Resolvers", () => {
       queryRoot = undefined;
       queryContext = undefined;
       queryInfo = undefined;
+      descriptorEvaluated = false;
     });
 
     beforeAll(async () => {
@@ -1216,6 +1230,12 @@ describe("Resolvers", () => {
           return true;
         }
 
+        @Query()
+        @DescriptorDecorator()
+        queryWithCustomDescriptorDecorator(): boolean {
+          return true;
+        }
+
         @Mutation()
         mutationWithArgs(@Args() args: SampleArgs): number {
           if (args.isTrue()) {
@@ -1507,6 +1527,19 @@ describe("Resolvers", () => {
       expect(queryRoot).toEqual(2);
       expect(queryContext).toEqual("present");
     });
+
+    it("should allow for overwriting descriptor value in custom decorator", async () => {
+      const query = /* graphql */ `
+        query {
+          queryWithCustomDescriptorDecorator
+        }
+      `;
+
+      const { data } = await graphql(schema, query);
+
+      expect(descriptorEvaluated).toBe(true);
+      expect(data.queryWithCustomDescriptorDecorator).toBe(true);
+    });
   });
 
   describe("buildSchema", async () => {