graphql-ruby
graphql-ruby copied to clipboard
Fields with lazy results aren't resolved inside directive context
Describe the bug
If you have a field that is resolved lazily, e.g. using a GraphQL::Batch::Loader, and that field is selected in a query using a directive which applies to the field, the field will be resolved outside of the directive's scope (outside of the yield of the directive's resolve).
Say you're using a gem with a class method that's given a block and that method sets/reverts a class variable before/after the block is called. Then you write a directive that passes resolve's block into that gem's class method as its block. Given a lazy field that uses the class variable, that field will be resolved using the "after" value instead of the "during" value.
Versions
graphql version: 2.3.7
rails (or other framework): N/A
other applicable versions (graphql-batch, etc): graphql-batch 0.6.0
GraphQL schema
class Attributed
class << self
attr_reader :attr
def with_attribute(attr)
previous_attr = @attr
@attr = attr
yield
ensure
@attr = previous_attr
end
end
end
class TestLoader < GraphQL::Batch::Loader
def perform(keys)
keys.each { fulfill(_1, Attributed.attr) }
end
end
class TestSchema < GraphQL::Schema
class TestQuery < GraphQL::Schema::Object
field :lazy_value, String
def lazy_value
TestLoader.for.load(nil)
end
field :value, String
def value
Attributed.attr
end
end
class WithAttribute < GraphQL::Schema::Directive
locations(GraphQL::Schema::Directive::QUERY)
def self.resolve(*, &block)
Attributed.with_attribute("red", &block)
end
end
query(TestQuery)
directives(WithAttribute)
use(GraphQL::Batch)
end
GraphQL query
query @withAttribute {
value
lazyValue
}
{
"data": {
"value": "red",
"lazyValue": nil
}
}
Steps to reproduce
Run the query above using the provided schema and classes
Expected behavior
Both value and lazyValue resolve to red
Actual behavior
lazyValue resolves to nil because it's outside of the withAttribute directive scope when it gets resolved and so Attributed.with_attributed has reverted to its previous attr value.
Hey, thanks for the detailed report. I agree this is how this should work. Unfortunately it doesn't, and it's not going to be easy.
The problem is that directives are .resolve'd inside run_eager, which is executing query fields that aren't lazy:
https://github.com/rmosolgo/graphql-ruby/blob/516a2f631b233b3566746b6910eeb11db253425f/lib/graphql/execution/interpreter/runtime.rb#L86
(At first, I thought putting GraphQL::Batch.batch { ... } inside def self.resolve would fix this problem, but it doesn't, because of that problem.)
And it won't be as easy as "just moving it up" to a higher spot in the call stack because of how GraphQL-Ruby does multiplexing. Given two queries like this:
q1 = "query Query1 @withAttribute { lazyValue}"
q2 = "query Query2 { lazyValue }"
r1, r2 = MySchema.multiplex([q1, q2])
We'd expect the first lazyValue to be run inside @withAttribute, but the second lazyValue not to have @withAttribute. But GraphQL-Ruby can't make that guarantee because it runs all lazy values inside the same pool.
It might be possible to split them out -- detect queries which have directives and run them separately. That's the technique GraphQL-Ruby uses for fields and fragments, but there isn't already a place to do it for queries.)
I'll keep this open while I'm thinking it over and follow up here when I have anything more to report.