Skip to content

Make resolvers inherit description from superclass #5195

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.

Sign up for GitHub

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

Merged
rmosolgo merged 1 commit into from
Dec 27, 2024
Merged

Make resolvers inherit description from superclass #5195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/graphql/schema/resolver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class Resolver
include Schema::Member::GraphQLTypeNames
# Really we only need description & comment from here, but:
extend Schema::Member::BaseDSLMethods
extend Member::BaseDSLMethods::ConfigurationExtension
Copy link
Contributor

@gmac gmac Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a side-effect from this change where resolvers that inherit from other resolvers must now explicitly declare a typename, otherwise they inherit the parent's typename and become a duplicate definition name.

Was this intentional? It's a bit counter-intuitive to the default precedent of elements following their class names.

Copy link
Owner Author

@rmosolgo rmosolgo Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that was an accident. I've proposed a fix here: #5260

Copy link
Contributor

@gmac gmac Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, @swalkinshaw and I talked about the new behavior and agreed that it isn’t all bad. A resolver is effectively a representation of a Type, and type inheritance is generally discouraged in favor of sharing behavior through mixins. That’s internal precedent though. The former behavior is probably more consistent with the general library.

extend GraphQL::Schema::Member::HasArguments
extend GraphQL::Schema::Member::HasValidators
include Schema::Member::HasPath
Expand Down
10 changes: 10 additions & 0 deletions spec/graphql/schema/resolver_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class Resolver3 < Resolver1
class Resolver4 < BaseResolver
type Integer, null: false

description "Adds object.value to ast_node.name.size"

extras [:ast_node]
def resolve(ast_node:)
object.value + ast_node.name.size
Expand Down Expand Up @@ -724,6 +726,14 @@ def exec_query(*args, **kwargs)
end
end

describe "description" do
it "is inherited" do
expected_desc = "Adds object.value to ast_node.name.size"
assert_equal expected_desc, ResolverTest::Resolver4.description
assert_equal expected_desc, ResolverTest::Resolver5.description
end
end

describe "when applied to a field" do
it "gets the field's description" do
assert_nil ResolverTest::Schema.find("Query.resolver3").description
Expand Down
Loading