graphql-client icon indicating copy to clipboard operation
graphql-client copied to clipboard

Weird type exception when making a named mutation

Open gleopoldo opened this issue 6 years ago • 7 comments

Hello!

I discovered a tricky error when using graphql-client (0.13.0). Today I make my queries to an external GraphQL API through graphiql. So, normally, my queries are all named.

When I finally adjusted all parameters, I had:

      CreatePartner = Client.parse <<-GRAPHQL
        mutation CreatePartner($pubkey: String!, $name: String!, $email: String!) {
          User_AddPartnerKey(input: { # ... }) {
             # response
          }
        }
      GRAPHQL

Then, when querying it, I got:

[5] pry(Quanto::API)> Client::Client.query(CreatePartner)
TypeError: expected definition to be a GraphQL::Client::OperationDefinition but was GraphQL::Language::Nodes::Document
from /quanto-gem/.gems/gems/graphql-client-0.13.0/lib/graphql/client.rb:307:in `query'

However, if I remove the mutation name, the query works fine:

      CreatePartner = Client.parse <<-GRAPHQL
        mutation($pubkey: String!, $name: String!, $email: String!) {
          User_AddPartnerKey(input: { # ... }) {
             # response
          }
        }
      GRAPHQL

Is this a desired behavior? I know that having a named query seems duplicated, since I have a constant defining a name for my query. However, this error is not very helpful to find the origin of the problem.

Can we have a more specific error to handle this scenario?

gleopoldo avatar Sep 27 '18 14:09 gleopoldo

@gleopoldo Thanks for documenting this! I had the same issue, the named queries, mutations, and fragments worked everywhere but here...

matthewlein avatar Oct 31 '18 13:10 matthewlein

Hello, took me several hours to indeed figure this one out. Naming the query causes a different kind of document to be generated and this is apparently not supported.

Can this be documented and possibly fixed? It also seems that the name of the query or mutation is used to define a constant, so doing query test blows up with

/ruby/gems/2.5.0/gems/graphql-client-0.14.0/lib/graphql/client.rb:249:in `const_set': wrong constant name test (NameError)

JanStevens avatar Feb 19 '19 12:02 JanStevens

Ha after some source reading I came across this little nugget: https://github.com/github/graphql-client/blob/master/lib/graphql/client.rb#L235

Basically if your query / mutation is anonymous then all fine and it works, but if it's named then a new Module constant is defined with the definition, so this means that you have to reference it correctly. Using your example it would mean you need to use: CreatePartner::CreatePartner.

I think this behaviour is ment to allow writing one big parse block with multiple queries and then reference them by name:

Queries = Client.parse <<~GRAPHQL
  query ById { id }
  query ByName { name }
GRAPHQL

This would then allow to do Queries::ById and Queries::ByName

JanStevens avatar Feb 19 '19 13:02 JanStevens

+1

stephenhmarsh avatar Jun 19 '19 00:06 stephenhmarsh

We also hit this issue because we try to give all of our GraphQL operations easily readable names for monitoring purposes. It's isn't pretty but we were able to workaround the issue by wrapping all access to GraphQL::Client with something like this:

class GraphQLEndpoint
  def initialize(client)
    @client = client
  end

  def parse(*args)
    operations = @client.parse(*args)
    return operations unless compound_document?(operations)

    executable_operations = operations.constants.each_with_object([]) do |operation_name, result|
      operation = operations.const_get(operation_name.to_s)
      if operation.is_a?(GraphQL::Client::OperationDefinition)
        operation.define_singleton_method(:definition_name) { operation_name }
        result << operation
      end
    end

    # If there is a single named executable operation in the Module, then make it available via
    # the `__primary_operation` method so the operations Module can be passed directly to `execute`
    if executable_operations.size == 1
      operations.define_singleton_method(:__primary_operation) { executable_operations.first }
    end

    operations
  end

  def execute(query, **options)
    if compound_document?(query)
      query = query.try(:__primary_operation) || raise(ArgumentError.new('No operation specified'))
    end

    @client.query(query, **options)
  end

  private

  def compound_document?(document)
    document.is_a?(Module) && !document.is_a?(GraphQL::Client::Definition)
  end
end

jturkel avatar Jun 19 '19 13:06 jturkel

I just hit this problem (amongst many others). @JanStevens , thanks for your tip.

It's ridiculous how GrapQL-Client library tries to make you use constants. Constants are fine but not if you want to have any configuration or any control over when things get instantiated. I'm sorry but this is the final drop and I'm going to stop trying to use this library for the project I'm working on.

werdlerk avatar Jul 02 '20 14:07 werdlerk

I really recommend https://github.com/yuki24/artemis which sits on top of this graphql-client and provides a better interface (writing .graphql file) to manage client queries

JanStevens avatar Jul 02 '20 14:07 JanStevens