Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resolvers): use types that implement an interface in __resolveType as first argument #3618

Merged
merged 2 commits into from
Mar 11, 2020

Conversation

kamilkisiela
Copy link
Collaborator

@kamilkisiela kamilkisiela commented Mar 10, 2020

not interface itself

  /** Mapping between all available schema types and the resolvers types */
  export type ResolversTypes = {
    String: ResolverTypeWrapper<Scalars['String']>,
    Boolean: ResolverTypeWrapper<Scalars['Boolean']>,
-   Book: ResolverTypeWrapper<Book>,
+   Book: ResolversTypes['ColoringBook'] | ResolversTypes['TextBook'],
    TextBook: ResolverTypeWrapper<TextBook>,
    Float: ResolverTypeWrapper<Scalars['Float']>,
    ColoringBook: ResolverTypeWrapper<ColoringBook>,
  };

  /** Mapping between all available schema types and the resolvers parents */
  export type ResolversParentTypes = {
    String: Scalars['String'],
    Boolean: Scalars['Boolean'],
-   Book: Book,
+   Book: ResolversParentTypes['ColoringBook'] | ResolversParentTypes['TextBook'],
    TextBook: TextBook,
    Float: Scalars['Float'],
    ColoringBook: ColoringBook,
  };
  /** Mapping between all available schema types and the resolvers types */
  export type ResolversTypes = {
    String: ResolverTypeWrapper<Scalars['String']>,
    Boolean: ResolverTypeWrapper<Scalars['Boolean']>,
-   Book: ResolverTypeWrapper<Book>,
+   Book: ResolversTypes['ColoringBook'] | ResolversTypes['TextBook'],
    TextBook: ResolverTypeWrapper<TextBook>,
    Float: ResolverTypeWrapper<Scalars['Float']>,
    ColoringBook: ResolverTypeWrapper<ColoringBook>,
  };

  /** Mapping between all available schema types and the resolvers parents */
  export type ResolversParentTypes = {
    String: Scalars['String'],
    Boolean: Scalars['Boolean'],
-   Book: Book,
+   Book: ResolversParentTypes['ColoringBook'] | ResolversParentTypes['TextBook'],
    TextBook: TextBook,
    Float: Scalars['Float'],
    ColoringBook: ColoringBook,
  };
const resolvers: Resolvers = {
  Book: {
    __resolveType(parent, context, info) {
-    // parent is ResolversParentTypes['Book']
+    // parent is ResolversParentTypes['TextBook'] | ResolversParentTypes['ColoringBook']
+
+    // previously `parent.__typename` didn't exist
+    if (parent.__typename === 'TextBook') {
+      return 'TextBook';
+    }
+
+    if (parent.__typename === 'ColoringBook') {
+      return 'ColoringBook';
+    }
+
+    return null;
    },
  },
};

Related #3538

@theguild-bot
Copy link
Collaborator

The latest changes of this PR are available as alpha in npm: 1.13.1-alpha-e99e7f6b.5

Repository owner deleted a comment from theguild-bot Mar 10, 2020
@dotansimha dotansimha merged commit cbd8793 into master Mar 11, 2020
@dotansimha dotansimha deleted the fix/resolvers-interface branch March 11, 2020 06:35
@zomble
Copy link

zomble commented Mar 19, 2020

Great change.

This appears to have a side effect that when an interface doesn't have an implementing type it no longer is omitted in the generated types.

This presents an issue when using https://graphql-modules.com/ and a module defines an interface for others to use, but inside the interface defining module the generated types no longer have that interface.

Module A:
interface Example

Module B:
type Fruit implements Example

Before this version generating types for Module A (in isolation) would include definitions for Example.

After this change, only Module B does as it has an implementing type of Example (which now resolves to Fruit).

@kamilkisiela @dotansimha thoughts? I am aware that from a graph client perspective of a schema, one would not ever care about an interface with out implementing types.

Would it be okay when then are no implementing types to let it generate definitions as it always has done, rather then never?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants