rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.38k stars 1.39k forks source link

GraphQL gem allows empty object types (in violation of GraphQL spec) #4509

Open myronmarston opened 1 year ago

myronmarston commented 1 year ago

Describe the bug

According to the GraphQL spec:

An Object type must define one or more fields.

However, the GraphQL gem appears to treat object types that have no fields as valid.

Versions

graphql version: 2.0.22 rails (or other framework): N/A other applicable versions (graphql-batch, etc): N/A

GraphQL schema

type EmptyObject {
}

type Query {
  something: EmptyObject
}

GraphQL query

N/A

Steps to reproduce

Create a graphql_gem_bug_no_fields.rb file with these contents:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "graphql", "2.0.22"
end

schema_sdl = <<~EOS
  type EmptyObject {
  }

  type Query {
    something: EmptyObject
  }
EOS

require "graphql"

puts GraphQL::Schema.from_definition(schema_sdl).to_definition

Run the file.

Expected behavior

I expect the GraphQL gem's schema validation (which happens when you round-trip an SDL string through the gem's parser and dumper) to fail and indicate that EmptyObject isn't valid.

Actual behavior

The GraphQL gem treats this schema as valid valid, resulting in this output:

type EmptyObject

type Query {
  something: EmptyObject
}

Additional context

1923 was previously reported for this issue and was apparently resolved by #2462. So looks like a regression? Not sure when it crept back in.

rmosolgo commented 1 year ago

Thanks for the detailed report -- I agree that it should work like you described but instead, I seem to have a spec with the opposite:

https://github.com/rmosolgo/graphql-ruby/blob/master/spec/graphql/schema/build_from_definition_spec.rb#L15-L35

🤦 I'm open to correcting this but, out of curiousity, how did it become a problem for you?

myronmarston commented 1 year ago

🤦 I'm open to correcting this but, out of curiousity, how did it become a problem for you?

I work on a framework that leverages the GraphQL gem as part of providing a GraphQL interface. A new user of the framework ran into a situation where autocomplete wasn't working in GraphiQL and when he asked us to look into it we noticed GraphiQL had given him this error:

{
  "errors": [
    "Type SomeType must define one or more fields."
  ]
}

He had defined SomeType in our framework with no fields. To ensure the validity of the schema.graphql that our framework generates, we roundtrip it through GraphQL::Schema.from_definition(schema_sdl).to_definition which generally provides great, useful errors, but didn't provide any error in this case. That allowed a deploy with a botched/invalid schema.

ojab commented 1 year ago

Also https://the-guild.dev/graphql/codegen fails on empty object types with cryptic errors

rmosolgo commented 2 weeks ago

Also, input fields must define at least one argument: http://spec.graphql.org/October2021/#sec-Input-Objects.Type-Validation