Skip to content
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.

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

Add supports for Type extention ("extend type" syntax) #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

takeyoda
Copy link
Contributor

What

Add supports for Type extention ("extend type" syntax)

Why

In GraphQL, we can extend a type using the "extend type" syntax, like the following example:

type Item {
  title: String!
}
extend type Item { # HERE
  price: Int!
}
type Query {
  item: Item
}

Type "Item" contains two attributes "title" and "price".
When generating a GraphQL query with graphql-autotest from the above schema,
the "price" attribute is not output.

In GraphQL-Ruby, an "extend type" block is treated as an independent instance of ObjectTypeExtension class.
It means that multiple definitions with the same name are included in Document#definitions.
On the other hand, QueryGenerator#type_definition assumes that there is only one definition that matches the name, so the definition inside the "extend type" block is not considered.

  • Example

    require 'graphql'
    
    schema = <<~SCHEMA
      type Item {
        title: String!
      }
      extend type Item {
        price: Int!
      }
      type Query {
        item: Item!
      }
    SCHEMA
    
    doc = GraphQL::parse(schema)
    doc.definitions.map do |d|
      puts "name=#{d.name} (#{d.class})"
    end
    • Output
    name=Item (GraphQL::Language::Nodes::ObjectTypeDefinition)
    name=Item (GraphQL::Language::Nodes::ObjectTypeExtension)     # contains same named Node
    name=Query (GraphQL::Language::Nodes::ObjectTypeDefinition)
    

How to fix

Changed QueryGenerator#type_definition to merge all definitions that matched name.

⚠️ Note ⚠️

This change made graphql-autotest work as I expected, but I'm not sure it's the correct change.
For example, I'm worried about the following case expression:

case field_type_def
when nil, GraphQL::Language::Nodes::EnumTypeDefinition, GraphQL::Language::Nodes::ScalarTypeDefinition
Field.new(name: f.name, children: nil, arguments: arguments)
when GraphQL::Language::Nodes::UnionTypeDefinition
possible_types = field_type_def.types.map do |t|
t_def = type_definition(t.name)
children = testable_fields(t_def, called_fields: called_fields.dup, depth: depth + 1, ancestors: [f, *ancestors])
Field.new(name: "... on #{t.name}", children: children)
end
Field.new(name: f.name, children: possible_types + [Field::TYPE_NAME], arguments: arguments)
else
children = testable_fields(field_type_def, called_fields: called_fields.dup, depth: depth + 1, ancestors: [f, *ancestors])
Field.new(
name: f.name,
children: children,
arguments: arguments,
)
end
end.compact + [Field::TYPE_NAME]

--
takeyoda

@kuroponzu
Copy link
Contributor

@takeyoda
Sorry, please resolve conflicts 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants