graphql-ruby: GraphQL v1.8.7 break compatibility with BatchLoader

GraphQL V1.8.6 works perfectly with https://github.com/exAspArk/batch-loader after upgrading to v1.8.7 the fields are not lazy resolved, so something changed

Using Batch Loader and GraphQL v1.8.6

  Event Load (0.9ms)  SELECT "events".* FROM "events"
  ↳ app/controllers/api/graphql_controller.rb:11
  EventPeriod Load (0.9ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."event_id" IN ($1, $2, $3)  [["event_id", 1], ["event_id", 2], ["event_id", 3]]
  ↳ app/graphql/resolvers/event_periods_resolver.rb:10
  EventPeriod::Grouping Load (0.7ms)  SELECT "event_period_groupings".* FROM "event_period_groupings" WHERE "event_period_groupings"."event_period_group_id" IN ($1, $2)  [["event_period_group_id", 5], ["event_period_group_id", 12]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10
  EventPeriod::Day Load (0.7ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."type" IN ('day') AND "event_periods"."id" IN ($1, $2, $3, $4, $5)  [["id", 1], ["id", 2], ["id", 3], ["id", 4], ["id", 11]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10

Using GraphQL v1.8.7

  Event Load (0.8ms)  SELECT "events".* FROM "events"
  ↳ app/controllers/api/graphql_controller.rb:11
  EventPeriod Load (1.1ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."event_id" = $1  [["event_id", 1]]
  ↳ app/graphql/resolvers/event_periods_resolver.rb:10
  EventPeriod Load (0.6ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."event_id" = $1  [["event_id", 2]]
  ↳ app/graphql/resolvers/event_periods_resolver.rb:10
  EventPeriod Load (0.4ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."event_id" = $1  [["event_id", 3]]
  ↳ app/graphql/resolvers/event_periods_resolver.rb:10
  EventPeriod::Grouping Load (0.8ms)  SELECT "event_period_groupings".* FROM "event_period_groupings" WHERE "event_period_groupings"."event_period_group_id" = $1  [["event_period_group_id", 5]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10
  EventPeriod::Day Load (0.8ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."type" IN ('day') AND "event_periods"."id" IN ($1, $2, $3, $4)  [["id", 1], ["id", 2], ["id", 3], ["id", 4]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10
  EventPeriod::Grouping Load (0.4ms)  SELECT "event_period_groupings".* FROM "event_period_groupings" WHERE "event_period_groupings"."event_period_group_id" = $1  [["event_period_group_id", 12]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10
  EventPeriod::Day Load (0.4ms)  SELECT "event_periods".* FROM "event_periods" WHERE "event_periods"."type" IN ('day') AND "event_periods"."id" IN ($1, $2, $3, $4, $5)  [["id", 1], ["id", 2], ["id", 3], ["id", 4], ["id", 11]]
  ↳ app/graphql/resolvers/event_period_days_resolver.rb:10
Completed 200 OK in 248ms (Views: 0.4ms | ActiveRecord: 19.9ms)

Related issue on Batch-Loader: https://github.com/exAspArk/batch-loader/issues/22

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 22 (13 by maintainers)

Most upvoted comments

Thanks for the suggestion, that’s really good! I was brainstorming other ideas over at https://github.com/rmosolgo/graphql-ruby/pull/1411#discussion_r194159423, but your suggestion here is better because it’s simple, schema-local and backwards-compatible. I’ll try it out soon!

By the way, another solution might be to add class to IMPLEMENTED_INSTANCE_METHODS.

rmosolgo on Aug 15, 2018

Ok, so what we did to fix this issue is the following. We upgraded from the graphql gem from 1.8.5 to 1.8.7. While upgrading, the tests have shown that the batch-loader stopped batching requests.

The reason for that is the new :scope option that broke the batch-loader. Until there is a good API to fix this, and since we don’t use scoping, we have introduced a BaseField that has the scope set to false.

Adding a base field is used for providing defaults to all the fields (that are easily overridable on a per field basis). It is following recipes that can be found here: http://graphql-ruby.org/fields/introduction.html#field-parameter-default-values http://graphql-ruby.org/type_definitions/extensions.html#customizing-fields

module Types
  class BaseField < GraphQL::Schema::Field
    def initialize(*args, scope: false, **kwargs, &block)
      super
    end
  end
end

module Types
  class BaseObject < GraphQL::Schema::Object
    field_class BaseField
  end
end

DamirSvrtan on Oct 13, 2018

Ok, I think I see. batch-loader checks the return type of every field call to see whether it’s a loader that should be wrapped:

https://github.com/exAspArk/batch-loader/blob/6730dfa775c230de5502d356ca974e486f6bdd8c/lib/batch_loader/graphql.rb#L22-L25

But, in 1.8.7, sometimes return values are wrapped with a GraphQL::Execution::Lazy, so that scope_items can be called on their eventual return value:

https://github.com/rmosolgo/graphql-ruby/blob/34c0bdbf14ed7bd6dafc321c9acd06abb7e79098/lib/graphql/schema/field.rb#L499-L509

So the problem is, in 1.8.7, fields that return a batch loader now return a Lazy which wraps the batch loader, so the instrumentation doesn’t properly wrap it.

🤔 Ok, now how to fix! 😆

rmosolgo on Aug 15, 2018