miragejs / graphql

A library for handling GraphQL requests with Mirage JS
MIT License
74 stars 12 forks source link

TypeError on `returnType` when returning Union from mutations #53

Open sebamza17 opened 2 years ago

sebamza17 commented 2 years ago

This is related to a comment made on https://github.com/miragejs/graphql/issues/52 that was out of context for the issue where it was added to.

We've defined a mutation to work with basic updates on our MembershipInstance model, we defined this schema

const schema = {
  type Membership {
    id: ID!
    name: String!
    membershipInstances(id: ID): [MembershipInstance]
  }

  type MembershipInstance {
    id: ID!
    ...
  }

  type Mutation {
    updateMembershipInstance(input: UpdateMembershipInstanceInput): UpdateMembershipInstanceResponse
  }

  input UpdateMembershipInstanceInput {
    id: ID!
    name: String!
  }

  union UpdateMembershipInstanceResponse = UpdateMembershipInstanceSuccess | Error

  type UpdateMembershipInstanceSuccess {
    membershipInstance: MembershipInstance
  }

  type Error {
    message: String
  }
}

and we have this mutation handler on our resolvers we're passing to createGraphQLHandler(...)

const resolvers = {
    updateMembershipInstance: (parent, args, context, info) => {
      const { mirageServer } = context
      const { id: membershipInstanceId } = args.input
      const membershipInstance = mirageServer.schema.membershipInstance.find(membershipInstanceId)

      if (membershipInstance) {
        membershipInstance.update(args.input)
      }

      const response = new UpdateMembershipInstanceSuccess()
      response.membershipInstance = membershipInstance

      return response
    }
}

export class UpdateMembershipInstanceSuccess {
  get __typename () {
    return 'UpdateMembershipInstanceSuccess'
  }
}

this code, as is, works, but we're wondering if this is the graphql way to do things, specially because we're calling mirage directly to update records, then we're also building a typed response to avoid graphql union errors. What we're discussing with the rest of the team is if this shouldn't have a different handler, deferring the actual data update work to mirage/graphql, we tried this also

const resolvers = {
  updateMembershipInstance: {
    __typename: 'UpdateMembershipInstanceResponse',
    __resolveType: (obj, context, info) => {
      debugger
    },
    resolveType: (obj, context, info) => {
      debugger
    },
    __returnType: () => {
      debugger
    },
    returnType: () => {
      debugger
    }
  },

  UpdateMembershipInstanceResponse: {
    __resolveType: (obj, context, info) => {
      debugger
    },
    resolveType: (obj, context, info) => {
      debugger
    },
    __returnType: () => {
      debugger
    },
    returnType: () => {
      debugger
    }
  },

  updateMembershipInstance: (parent, args, context, info) => {
    const membershipInstance = mirageGraphQLFieldResolver(...arguments)

    const response = new UpdateMembershipInstanceSuccess()
    response.membershipInstance = membershipInstance

    return response
  }
}

export class UpdateMembershipInstanceSuccess {
  get __typename () {
    return 'UpdateMembershipInstanceSuccess'
  }
}

but this approach fails, with this error

TypeError: Cannot read properties of undefined (reading 'returnType') at mirageGraphQLFieldResolver
{
  "message": "Cannot read properties of undefined (reading 'returnType')",
  "locations": [
    {
      "line": 2,
      "column": 3
    }
  ],
  "path": [
    "updateMembershipInstance"
  ]
}

which makes me thing we're not using mirageGraphQLFieldResolver correctly, but what I find specially confusing is that we also added returnType to our types resolvers, but it seems we're not even hitting those, and we cannot still find which type from our schema this error is originating from

any help or comment would be super welcomed!

jneurock commented 2 years ago

Hey, this resolver seems good to me. There is no "GraphQL" way to resolve anything and Mirage GraphQL doesn't have any opinions about how you should resolve things either. I would, however, highly encourage usage of Mirage's API above all else, as you have already done here 👍

const resolvers = {
    updateMembershipInstance: (parent, args, context, info) => {
      const { mirageServer } = context
      const { id: membershipInstanceId } = args.input
      const membershipInstance = mirageServer.schema.membershipInstance.find(membershipInstanceId)

      if (membershipInstance) {
        membershipInstance.update(args.input)
      }

      const response = new UpdateMembershipInstanceSuccess()
      response.membershipInstance = membershipInstance

      return response
    }
}

export class UpdateMembershipInstanceSuccess {
  get __typename () {
    return 'UpdateMembershipInstanceSuccess'
  }
}

As for this approach...

  updateMembershipInstance: (parent, args, context, info) => {
    const membershipInstance = mirageGraphQLFieldResolver(...arguments)

    const response = new UpdateMembershipInstanceSuccess()
    response.membershipInstance = membershipInstance

    return response
  }

...you are using mirageGraphQLFieldResolver correctly; however, the library can't auto-resolve this mutation for you so it won't work.

sebamza17 commented 2 years ago

@jneurock thank you so much for your time ❤️ , and understood, what I find a bit confusing is that mirageGraphQLFieldResolver is not resolving into anything, it just throws the error from the first comment, seems like there's something not well defined on our schema, or we might be missing a type resolver 🤔

jneurock commented 2 years ago

Could have something to do with this:

  updateMembershipInstance: {
    __typename: 'UpdateMembershipInstanceResponse',
    __resolveType: (obj, context, info) => {
      debugger
    },
    resolveType: (obj, context, info) => {
      debugger
    },
    __returnType: () => {
      debugger
    },
    returnType: () => {
      debugger
    }
  },

I wonder if it's somehow altering what gets passed as the info argument to the resolver. I believe the error comes from Mirage GraphQL code that expects the info argument to be an object having a returnType property.

export default function mirageGraphQLFieldResolver(obj, args, context, info) {
  let { isList, type } = unwrapType(info.returnType);

  // ...
}