From a86432c8581aa5bddf11a0c8629348469b89ed46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Lytek?= Date: Fri, 1 Feb 2019 19:48:29 +0100 Subject: [PATCH 1/3] test(decorators): add test case for nullify custom method decorators --- tests/functional/resolvers.ts | 43 +++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/tests/functional/resolvers.ts b/tests/functional/resolvers.ts index 6b41fba16..bdf2fc1f7 100644 --- a/tests/functional/resolvers.ts +++ b/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 () => { From d58295371fedd9917d6f360562a239d27e8a51ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Lytek?= Date: Fri, 1 Feb 2019 19:50:47 +0100 Subject: [PATCH 2/3] fix(decorators): replace calling method ref with getting ref from object --- src/decorators/Field.ts | 1 - src/decorators/FieldResolver.ts | 1 - src/decorators/Mutation.ts | 6 +++--- src/decorators/Query.ts | 6 +++--- src/decorators/Subscription.ts | 8 ++++---- src/helpers/{handlers.ts => resolver-metadata.ts} | 3 +-- src/metadata/definitions/resolver-metadata.ts | 2 -- src/resolvers/create.ts | 9 +++++---- 8 files changed, 16 insertions(+), 20 deletions(-) rename src/helpers/{handlers.ts => resolver-metadata.ts} (92%) diff --git a/src/decorators/Field.ts b/src/decorators/Field.ts index fd8b624d6..b21d049cf 100644 --- a/src/decorators/Field.ts +++ b/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, }); } diff --git a/src/decorators/FieldResolver.ts b/src/decorators/FieldResolver.ts index 573e86435..1fc6820d1 100644 --- a/src/decorators/FieldResolver.ts +++ b/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, diff --git a/src/decorators/Mutation.ts b/src/decorators/Mutation.ts index 0fa314903..e4a98f6ba 100644 --- a/src/decorators/Mutation.ts +++ b/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); }; } diff --git a/src/decorators/Query.ts b/src/decorators/Query.ts index 6d7717aaa..392cd81f4 100644 --- a/src/decorators/Query.ts +++ b/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); }; } diff --git a/src/decorators/Subscription.ts b/src/decorators/Subscription.ts index 7749cd842..806d586de 100644 --- a/src/decorators/Subscription.ts +++ b/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, }); diff --git a/src/helpers/handlers.ts b/src/helpers/resolver-metadata.ts similarity index 92% rename from src/helpers/handlers.ts rename to src/helpers/resolver-metadata.ts index 8d693c6da..2a311ca4f 100644 --- a/src/helpers/handlers.ts +++ b/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, diff --git a/src/metadata/definitions/resolver-metadata.ts b/src/metadata/definitions/resolver-metadata.ts index ac842876a..30308cda3 100644 --- a/src/metadata/definitions/resolver-metadata.ts +++ b/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; diff --git a/src/resolvers/create.ts b/src/resolvers/create.ts index b478a9f92..b2f66c272 100644 --- a/src/resolvers/create.ts +++ b/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 = { 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; }); }; } From 5cbd321139b7dde72ebc8bdfe2b83e013787e14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Lytek?= Date: Fri, 1 Feb 2019 20:00:39 +0100 Subject: [PATCH 3/3] docs(changelog): add info about the decorators nullifying fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0089c8689..cbe9d16ee 100644 --- a/CHANGELOG.md +++ b/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