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

Nesting resolvers of the same type

Open kiskoza opened this issue 3 years ago • 3 comments

Recently I ran into a cyclical load error with resolvers. After searching for a solution, I found that this it a known issue and could be resolved by an easy change described here: https://graphql-ruby.org/fields/resolvers.html#nesting-resolvers-of-the-same-type

Then it popped to my mind why we don't warn people about this potential issue with rubocop? If it sounds reasonable to you, I'd like to write a new cop suggesting that you should have a string type definition in resolvers.

# Resovers should use strings for type definitions. See: https://graphql-ruby.org/fields/resolvers.html#nesting-resolvers-of-the-same-type
# 
# @example
#   # good
# 
#   module Resolvers
#     class TasksResolver < GraphQL::Schema::Resolver
#       type '[Types::TaskType]', null: false
# 
#       def resolve
#         []
#       end
#     end
#   end
# 
#   # bad
# 
#   module Resolvers
#     class TasksResolver < GraphQL::Schema::Resolver
#       type [Types::TaskType], null: false
# 
#       def resolve
#         []
#       end
#     end
#   end

kiskoza avatar Mar 03 '21 10:03 kiskoza

Hi @kiskoza, sounds reasonable, I'll be happy to review this PR!

DmitryTsepelev avatar Mar 03 '21 12:03 DmitryTsepelev

Cool. I'm going to implement it soon

kiskoza avatar Mar 29 '21 12:03 kiskoza

@DmitryTsepelev, hey! I did some research on this. This problem is not repeated in every resolver, and if there is a connection_type, we cannot convert it to string. In addition, graphql-ruby has added a more detailed error message for this case (https://github.com/rmosolgo/graphql-ruby/blob/v2.0.21/lib/graphql/schema/field.rb#L584):

Can't determine the return type for Task.tasks (it has `resolver: Resolvers::TasksResolver`, perhaps that class is missing a `type ...` declaration, or perhaps its type causes a cyclical loading issue)

Should we still add a new cop?

RasimKhusaenov avatar May 09 '23 09:05 RasimKhusaenov