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

When federation is enabled for a type, its resolvers have a wrong ParentType #4722

Open
Tracked by #8296 ...
KoltesDigital opened this issue Sep 9, 2020 · 3 comments
Open
Tracked by #8296 ...
Assignees
Labels
core Related to codegen core/cli plugins

Comments

@KoltesDigital
Copy link

KoltesDigital commented Sep 9, 2020

The goal of __resolveReference is to return a model object which is internal to the current service. Its return type is then the parent type of related resolver functions. However the current parent type for resolvers is an extract of the real parent type (via GraphQLRecursivePick). It shouldn't.

To Reproduce

See https://codesandbox.io/s/bitter-https-7yboj

  1. My GraphQL schema:
extend type User @key(fields: "id") {
    id: ID! @external
    fullName: String!
}
  1. My GraphQL operations:

None

  1. My codegen.yml config file:
schema: schema.graphql
documents: null
generates:
  types.ts:
    plugins:
      - typescript
      - typescript-resolvers
    config:
      federation: true
      mappers:
        User: ./models#User
      mapperTypeSuffix: ParentType
      noSchemaStitching: true
      useTypeImports: true

Expected behavior

This file (user-resolvers.ts) should compile:

import { UserResolvers } from "./types";

export const __resolveReference: UserResolvers["__resolveReference"] = (
  externalUser
) => ({
  id: externalUser.id,
  firstName: "John",
  lastName: "Doe"
});

export const fullName: UserResolvers["fullName"] = (user) =>
  `${user.firstName} ${user.lastName}`;

But it does not. The cause is UserResolvers is:

export type UserResolvers<ContextType = any, ParentType extends ResolversParentTypes['User'] = ResolversParentTypes['User']> = {
  __resolveReference?: ReferenceResolver<Maybe<ResolversTypes['User']>, { __typename: 'User' } & GraphQLRecursivePick<ParentType, {"id":true}>, ContextType>;

  fullName?: Resolver<ResolversTypes['String'], { __typename: 'User' } & GraphQLRecursivePick<ParentType, {"id":true}>, ContextType>;
  __isTypeOf?: IsTypeOfResolverFn<ParentType>;
};

whereas it should be

export type UserResolvers<ContextType = any, ParentType extends ResolversParentTypes['User'] = ResolversParentTypes['User']> = {
  __resolveReference?: ReferenceResolver<Maybe<ParentType>, { __typename: 'User' } & GraphQLRecursivePick<ParentType, {"id":true}>, ContextType>;

  fullName?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
  __isTypeOf?: IsTypeOfResolverFn<ParentType>;
};

Environment:

  • OS: Win10 x64
  • @graphql-codegen/...: latest as of writing
  • NodeJS: v14.8.0
@KoltesDigital
Copy link
Author

KoltesDigital commented Sep 9, 2020

If this function returns parentTypeSignature without transforming it, it works fine. EDIT: unless the resolver is __resolveReference, in which case it is expected to pick the external fields.

https://github.com/dotansimha/graphql-code-generator/blob/master/packages/utils/plugins-helpers/src/federation.ts#L157

There are no related tests. Should I add one to ensure my use case passes? EDIT: I found them.

@jakeblaxon
Copy link
Contributor

jakeblaxon commented Sep 14, 2020

I added this comment to #4724 but I'll post it here as well:

I see that you're using mapped types. I didn't address the mapped type case at all when working on adding this federation functionality, so it may be buggy and there is definitely room for improvement there. For your fullName resolver, however, the reason you're not able to access the firstName and lastName types from the external User parent type is because you haven't specified that they are required by adding them to a @requires directive. You can solve that issue by adding that:

extend type User @key(fields: "id") {
    id: ID! @external
    firstName: String! @external
    lastName: String! @external
    fullName: String! @requires(fields: "firstName lastName")
}

(You will need to add all required external fields to your extended type, otherwise the Apollo Gateway will throw an error, hence why I've added them here). If you don't specify that these fields are required, then the Apollo Gateway may not provide them to you for certain requests. For example, what if I requested the following:

{
  getUser(...) {
    id
    firstName
    fullName
  }
}

In this case, the Apollo Gateway will only request { id firstName } from the base schema and pass this on as the parent type to your fullName resolver. Since only the id is specified as a key, there's no reason the Apollo Gateway would need to request lastName from the base schema. It doesn't know that you need it for your resolver. That's why you need to tell this to the Apollo Gateway with the @requires directive.

Since User is an extended type, I believe the __resolveReference resolver will never get called (although I have to verify this). Thinking about it intuitively though, each custom field that you add to an extended type will have its own resolver, so the Apollo Gateway will just call these field-specific resolvers instead of _resolveReference. It really only makes sense to have a __resolveReference in the base type, and we already account for this (as of #4542). Again, the __resolveReference resolver gets called directly from Apollo Gateway, where the parent type is the User field not from the base schema but from a schema that extends it. The only guaranteed fields that are available in this case are the @key fields, because these are the only fields that are guaranteed to be in this type across every schema. That's why we only pass in the @key fields subset of the parentType for the __resolveReference resolver.

@KoltesDigital
Copy link
Author

Discussion continues on #4724.

@dotansimha dotansimha removed the bug label Jun 20, 2021
@charlypoly charlypoly added the core Related to codegen core/cli label Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to codegen core/cli plugins
Projects
None yet
Development

No branches or pull requests

5 participants