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

Promises fail to be resolved for Relay connection_type #26

Closed
joshjcarrier opened this issue Oct 27, 2016 · 5 comments
Closed

Promises fail to be resolved for Relay connection_type #26

joshjcarrier opened this issue Oct 27, 2016 · 5 comments

Comments

@joshjcarrier
Copy link

Relay support in graphql-ruby suggests resolvers for a connection_type should be able to return an array. Expect that graphql-batch would support field resolver returning promise of an array in this scenario. It instead fails with exception No connection implementation to wrap GraphQL::Batch::Promise.

A workaround is to have the resolver wait for promise fulfillment by calling Promise#sync on the Loader#load/#load_many result.

@cjoudrey @dylanahsmith

For a repro, consider this script using graphql (0.19.4) and graphql-batch (0.2.4) on ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]:

require 'graphql/batch'

class ShopLoader < GraphQL::Batch::Loader
  def initialize(ctx)
  end

  def perform(ids)
    ids.each { |id| fulfill(id, OpenStruct.new({ id: id })) }
  end
end

class ShopResolver
  def self.call(obj, args, ctx)
    ShopLoader.for("ctx").load_many([ 1, 2, 3 ])
  end
end

ShopType = GraphQL::ObjectType.define do
  name "Shop"

  field :id do
    type types.ID
  end
end

QueryType = GraphQL::ObjectType.define do
  name "Query"

  field :shops do
    type types[ShopType]
    resolve ShopResolver
  end

  connection :shopsConnection, ShopType.connection_type do
    resolve ShopResolver
  end
end

Schema = GraphQL::Schema.define do
  query QueryType
end

Schema.query_execution_strategy = GraphQL::Batch::ExecutionStrategy
Schema.mutation_execution_strategy = GraphQL::Batch::MutationExecutionStrategy

puts "Array use of cache loader:"
puts Schema.execute("{ shops { id } }")
# {"data"=>{"shops"=>[{"id"=>"1"}, {"id"=>"2"}, {"id"=>"3"}]}}

puts "Connection use of cache loader:"
puts Schema.execute("{ shopsConnection { edges { node { id } } } }")
# /Users/jocarrie/.rvm/gems/ruby-2.3.0/gems/graphql-0.19.4/lib/graphql/relay/base_connection.rb:37:in `connection_for_nodes': 
# No connection implementation to wrap GraphQL::Batch::Promise (#<GraphQL::Batch::Promise:0x007f8081152170>) (RuntimeError)
#   from /Users/jocarrie/.rvm/gems/ruby-2.3.0/gems/graphql-0.19.4/lib/graphql/relay/connection_resolve.rb:12:in `call'
@dylanahsmith
Copy link
Contributor

Now that graphql-relay-ruby has been merged into graphql-ruby, we could probably be clearer about the fact that the relay code in graphql-ruby isn't supported by graphql-batch.

We mention in the README that

The loader class can be used from the resolve proc for a graphql field

and in this case you are trying to use the loader in a connection resolve proc, not on the field resolve proc.

You may be able to support this by using GraphQL::Relay::BaseConnection.register_connection_implementation.

@joshjcarrier
Copy link
Author

joshjcarrier commented Nov 24, 2016

Here's the simplest thing I could do convert over:

module GraphQL
  module Batch
    class SyncPromiseConnection < GraphQL::Relay::ArrayConnection
      def nodes
        super.sync
      end
    end
  end
end

GraphQL::Relay::BaseConnection.register_connection_implementation(GraphQL::Batch::Promise, GraphQL::Batch::SyncPromiseConnection)

At least it lets this gem work by default with connections? Is it worth further investment (concern that it extends GraphQL::Relay::ArrayConnection which is subject to breakage)?

@dylanahsmith
Copy link
Contributor

Calling sync like that would prevent batching.

With graphql-ruby's new lazy execution API it will actually be possible to fix this upstream, so I'm going to consider this an upstream issue now and close the issue here.

@rmosolgo
Copy link
Contributor

Upstream fix here!

rmosolgo/graphql-ruby#425

@mengqing
Copy link

Is there an example on how to implement this? Using the association loader in the example folder would produce N+1 on connection types

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

No branches or pull requests

4 participants