Support for new GraphQL Interpreter

JanStevens commented 5 years ago

I was looking at converting my own analysers to the new Interpreter and stumbled on the fact that if I enabled both use GraphQL::Analysis::AST and use GraphQL::Execution::Interpreter BatchLoader::GraphQL is not working anymore.

Reading up on it and a bit in the source code I think I know why since the ability to redefine a proc for a field is removed (in favour of simplicity and speed). More information here: https://graphql-ruby.org/queries/interpreter.html

Any chance this gem can be updated to support the new GraphQL Interpreter?

exAspArk commented 5 years ago

Hey @JanStevens!

Thank you for sharing the link. I'll be able to take a look into it once the Interpreter became a default runtime and became more stable (not experimental to avoid potential breaking changes in BatchLoader).

ghost commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

bessey commented 4 years ago

@exAspArk, does this work now? I mean, we're just starting to use this gem on a grapqhl-ruby 1.9 project and it seems to work, but thought i'd confirm since I see nothing in the changelogs.

exAspArk commented 4 years ago

It seems like GraphQL::Execution::Interpreter is now considered stable since graphql gem version 1.10+ (January 2020). I'm reopening the issue, will try to make it work 🙏

exAspArk commented 4 years ago

@exAspArk, does this work now? I mean, we're just starting to use this gem on a grapqhl-ruby 1.9 project and it seems to work, but thought i'd confirm since I see nothing in the changelogs.

Long story short:

if you use BatchLoader::GraphQL.for, then you're fine 👍
if you use BatchLoader.for and Interpreter, then you'd need to use https://github.com/exAspArk/batch-loader/pull/62 (draft PR)
if you use BatchLoader.for without Interpreter, then it works with all GraphQL versions except the range between 1.8.7 https://github.com/rmosolgo/graphql-ruby/issues/1778 - 1.8.10 https://github.com/exAspArk/batch-loader/issues/26#issuecomment-443329962

I.e. please use BatchLoader::GraphQL.for

I'm going to deprecate using BatchLoader.for in GraphQL, which adds unnecessary coupling with the graphql gem. Alternatively, it's possible to keep BatchLoader.for and wrap it:

field :user, UserType, null: false

def user # resolver
  BatchLoader::GraphQL.wrap(lazy_user)
end

def lazy_user
  BatchLoader.for...
end

ro-savage commented 4 years ago

Thanks @exAspArk - came across the today and updating all our resolvers to use BatchLoader::GraphQL.for fixed it.

exAspArk commented 4 years ago

I released a new version which still keeps compatibility with BatchLoader.for + GraphQL with Interpreter, but adds a deprecation warning. More info in the changelog.

exAspArk / batch-loader

Support for new GraphQL Interpreter #41