Allow inheritance of interface resolvers (#1517)

Urigo · Jan 8, 2021 · 9a59787 · 9a59787
1 parent 49df096
commit 9a59787
Show file tree

Hide file tree

Showing 11 changed files with 239 additions and 311 deletions.
diff --git a/.changeset/calm-gifts-nail.md b/.changeset/calm-gifts-nail.md
@@ -0,0 +1,5 @@
+---
+'graphql-modules': patch
+---
+
+Allow inheritance of resolvers defined in interfaces
diff --git a/packages/graphql-modules/src/module/resolvers.ts b/packages/graphql-modules/src/module/resolvers.ts
@@ -1,5 +1,4 @@
 import {
-  GraphQLResolveInfo,
   GraphQLScalarType,
   concatAST,
   Kind,
@@ -60,7 +59,7 @@ export function createResolvers(
         continue;
       } else if (isEnumResolver(obj)) {
         continue;
-      } else if (isObjectResolver(obj)) {
+      } else if (obj && typeof obj === 'object') {
         for (const fieldName in obj) {
           if (obj.hasOwnProperty(fieldName)) {
             ensure.type(typeName, fieldName);
@@ -73,7 +72,8 @@ export function createResolvers(
                 resolver: obj[fieldName],
                 middlewareMap,
                 path,
-                isTypeResolver: fieldName === '__isTypeOf',
+                isTypeResolver:
+                  fieldName === '__isTypeOf' || fieldName === '__resolveType',
               });
               resolvers[typeName][fieldName] = resolver;
             } else if (isResolveOptions(obj[fieldName])) {
@@ -101,15 +101,6 @@ export function createResolvers(
             }
           }
         }
-      } else if (isInterfaceOrUnionResolver(obj)) {
-        const resolver = wrapResolver({
-          config,
-          resolver: obj.__resolveType,
-          middlewareMap,
-          path: [typeName, '__resolveType'],
-          isTypeResolver: true,
-        });
-        resolvers[typeName].__resolveType = resolver;
       }
     }
   }
@@ -205,9 +196,7 @@ function mergeResolvers(config: ModuleConfig): Single<Resolvers> {
           addScalar({ typeName, resolver: value, container, config });
         } else if (isEnumResolver(value)) {
           addEnum({ typeName, resolver: value, container, config });
-        } else if (isInterfaceOrUnionResolver(value)) {
-          addInterfaceOrUnion({ typeName, fields: value, container, config });
-        } else if (isObjectResolver(value)) {
+        } else if (value && typeof value === 'object') {
           addObject({ typeName, fields: value, container, config });
         } else {
           throw new ResolverInvalidError(
@@ -222,38 +211,6 @@ function mergeResolvers(config: ModuleConfig): Single<Resolvers> {
   return container;
 }
 
-function addInterfaceOrUnion({
-  typeName,
-  fields,
-  container,
-  config,
-}: {
-  typeName: string;
-  fields: InterfaceOrUnionResolver;
-  container: Single<Resolvers>;
-  config: ModuleConfig;
-}): void {
-  if (container[typeName]) {
-    throw new ResolverDuplicatedError(
-      `Duplicated resolver of "${typeName}" union or interface`,
-      useLocation({ dirname: config.dirname, id: config.id })
-    );
-  }
-
-  if (Object.keys(fields).length > 1) {
-    throw new ResolverInvalidError(
-      `Invalid resolver of "${typeName}" union or interface`,
-      `Only __resolveType is allowed`,
-      useLocation({ dirname: config.dirname, id: config.id })
-    );
-  }
-
-  writeResolverMetadata(fields.__resolveType, config);
-  container[typeName] = {
-    __resolveType: fields.__resolveType,
-  };
-}
-
 function addObject({
   typeName,
   fields,
@@ -509,22 +466,6 @@ function addDefaultResolvers(
 // Resolver helpers
 //
 
-interface InterfaceOrUnionResolver {
-  __resolveType(parent: any, ctx: any, info: GraphQLResolveInfo): string | void;
-}
-
-function isInterfaceOrUnionResolver(obj: any): obj is InterfaceOrUnionResolver {
-  return isDefined(obj.__resolveType);
-}
-
-interface ObjectResolver {
-  [key: string]: ResolveFn;
-}
-
-function isObjectResolver(obj: any): obj is ObjectResolver {
-  return !isDefined(obj.__resolveType);
-}
-
 function isResolveFn(value: any): value is ResolveFn {
   return typeof value === 'function';
 }

diff --git a/packages/graphql-modules/tests/bootstrap.spec.ts b/packages/graphql-modules/tests/bootstrap.spec.ts
@@ -1,24 +1,24 @@
 import 'reflect-metadata';
-import { createApplication, createModule } from '../src';
-import { parse } from 'graphql';
+import { createApplication, createModule, testkit, gql } from '../src';
+import { makeExecutableSchema } from '@graphql-tools/schema';
 
 test('fail when modules have non-unique ids', async () => {
   const modFoo = createModule({
     id: 'foo',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         foo: String
       }
-    `),
+    `,
   });
 
   const modBar = createModule({
     id: 'foo',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         bar: String
       }
-    `),
+    `,
   });
 
   expect(() => {
@@ -31,7 +31,7 @@ test('fail when modules have non-unique ids', async () => {
 test('should allow multiple type extensions in the same module', async () => {
   const m1 = createModule({
     id: 'test',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         dummy: String!
       }
@@ -44,7 +44,7 @@ test('should allow multiple type extensions in the same module', async () => {
       extend type Mutation {
         test: String!
       }
-    `),
+    `,
     resolvers: {
       Mutation: {
         foo: () => '1',
@@ -67,7 +67,7 @@ test('should allow multiple type extensions in the same module', async () => {
 test('should not thrown when isTypeOf is used', async () => {
   const m1 = createModule({
     id: 'test',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         entity: Node
       }
@@ -78,7 +78,7 @@ test('should not thrown when isTypeOf is used', async () => {
         id: ID!
         f: String
       }
-    `),
+    `,
     resolvers: {
       Query: {
         entity: () => ({
@@ -98,20 +98,16 @@ test('should not thrown when isTypeOf is used', async () => {
     modules: [m1],
   });
 
-  const executeFn = app.createExecution();
-
-  const result = await executeFn({
-    schema: app.schema,
-    document: parse(/* GraphQL */ `
+  const result = await testkit.execute(app, {
+    document: gql`
       query test {
         entity {
           ... on Entity {
             f
           }
         }
       }
-    `),
-    variableValues: {},
+    `,
   });
 
   expect(result.errors).toBeUndefined();
@@ -125,7 +121,7 @@ test('should not thrown when isTypeOf is used', async () => {
 test('should allow to add __isTypeOf to type resolvers', () => {
   const m1 = createModule({
     id: 'test',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         entity: Node
       }
@@ -136,7 +132,7 @@ test('should allow to add __isTypeOf to type resolvers', () => {
         id: ID
         f: String
       }
-    `),
+    `,
     resolvers: {
       Query: {
         entity: () => ({
@@ -162,7 +158,7 @@ test('should allow to add __isTypeOf to type resolvers', () => {
 test('should support __resolveType', async () => {
   const m1 = createModule({
     id: 'test',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         entity: Node
         item: Item
@@ -183,7 +179,7 @@ test('should support __resolveType', async () => {
         id: ID!
         e: String
       }
-    `),
+    `,
     resolvers: {
       Query: {
         entity: () => ({
@@ -214,11 +210,8 @@ test('should support __resolveType', async () => {
     modules: [m1],
   });
 
-  const executeFn = app.createExecution();
-
-  const result = await executeFn({
-    schema: app.schema,
-    document: parse(/* GraphQL */ `
+  const result = await testkit.execute(app, {
+    document: gql`
       query test {
         entity {
           ... on Entity {
@@ -231,8 +224,7 @@ test('should support __resolveType', async () => {
           }
         }
       }
-    `),
-    variableValues: {},
+    `,
   });
 
   expect(result.errors).toBeUndefined();
@@ -246,10 +238,10 @@ test('should support __resolveType', async () => {
   });
 });
 
-test('do not support inheritance of field resolvers of an interface', async () => {
+test('allow field resolvers in an interface without objects inheriting them', async () => {
   const mod = createModule({
     id: 'test',
-    typeDefs: parse(/* GraphQL */ `
+    typeDefs: gql`
       type Query {
         entity: Node
         item: Item
@@ -273,7 +265,7 @@ test('do not support inheritance of field resolvers of an interface', async () =
         e: String
         d: String
       }
-    `),
+    `,
     resolvers: {
       Query: {
         entity: () => ({
@@ -293,17 +285,100 @@ test('do not support inheritance of field resolvers of an interface', async () =
       },
       Node: {
         __resolveType: (obj: any) => obj.type,
-        d: () => 'should not work, because graphql-js does not support it',
+        d: () => `should work only when there's a custom schema builder`,
       },
       Item: {
         __resolveType: (obj: any) => obj.type,
       },
     },
   });
 
-  expect(() => {
-    createApplication({
-      modules: [mod],
-    });
-  }).toThrow('Only __resolveType is allowed');
+  const app = createApplication({
+    modules: [mod],
+  });
+
+  const result = await testkit.execute(app, {
+    document: gql`
+      query test {
+        entity {
+          ... on Entity {
+            d
+          }
+        }
+      }
+    `,
+  });
+
+  expect(result.errors).toBeUndefined();
+  expect(result.data).toEqual({
+    entity: {
+      d: null,
+    },
+  });
+});
+
+test('pass field resolvers of an interface to schemaBuilder', async () => {
+  const mod = createModule({
+    id: 'test',
+    typeDefs: gql`
+      type Query {
+        entity: Node
+      }
+
+      interface Node {
+        id: ID!
+        d: String
+      }
+
+      type Entity implements Node {
+        id: ID!
+        f: String
+        d: String
+      }
+    `,
+    resolvers: {
+      Query: {
+        entity: () => ({
+          type: 'Entity',
+        }),
+      },
+      Entity: {
+        id: () => 1,
+        f: () => 'test',
+      },
+      Node: {
+        __resolveType: (obj: any) => obj.type,
+        d: () => `works`,
+      },
+    },
+  });
+
+  const app = createApplication({
+    modules: [mod],
+    schemaBuilder(input) {
+      return makeExecutableSchema({
+        ...input,
+        inheritResolversFromInterfaces: true,
+      });
+    },
+  });
+
+  const result = await testkit.execute(app, {
+    document: gql`
+      query test {
+        entity {
+          ... on Entity {
+            d
+          }
+        }
+      }
+    `,
+  });
+
+  expect(result.errors).toBeUndefined();
+  expect(result.data).toEqual({
+    entity: {
+      d: 'works',
+    },
+  });
 });