Skip to content

failure to require leaf field selection when using fragments #610

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

Closed
VladimirAlexiev opened this issue Aug 23, 2019 · 3 comments
Closed
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)

Comments

@VladimirAlexiev
Copy link

Assume a simple schema like this:

interface Character {name:String!}
type Human implements Character {name:String!}
type Droid implements Character {name:String!}
type Query {
  characters: [Character!]
}

This query is not valid because it does not select down to leaf (scalar) fields (see https://graphql.github.io/graphql-spec/draft/#sec-Leaf-Field-Selections):

{characters {
}}

Then why is this query valid? It selects a scalar field, but only for Human. It'd return Droids without any fields i.e. {}.

{characters {
   ... on Human {name}
}}

I asked the question on:

andrewingram replied: it's valid, which can seem counter-intuitive. it's valid in graphql-js, which is aligned with the spec.

I think the query exposes an omission in the spec?

@TDiazT
Copy link

TDiazT commented Jan 29, 2020

Actually, I think that it is valid. The spec says :

  • If selectionType is an interface, union, or object
    The subselection set of that selection must NOT BE empty

It simply states that it must contain a subselection (could be an inline fragment as in your example). It doesn't specify that its subselections must be leaf field selections.

I agree that in essence it should actually contain leaf selections for every possible subtype of the field's return type (e.g Human and Droid in your scenario), otherwise it could return an empty response. For instance, if there are no humans and 3 droids in your database then it would return:

{
    "character" : [{}, {}, {}] 
}

This also happens with union types, you are not obliged to specify each concrete type and therefore can get empty results.

I imagine there must be some reason related to fragments ?

@sungam3r
Copy link
Contributor

Interesting question.

@IvanGoncharov
Copy link
Member

@VladimirAlexiev Thanks for the question 👍

This query is not valid because it does not select down to leaf (scalar) fields (see graphql.github.io/graphql-spec/draft/#sec-Leaf-Field-Selections):

We have an RFC that makes it valid, please see #674

Then why is this query valid?

Because it's a common use case to get only data for certain types from union/interface.
We shouldn't force users to ask for additional data just to make validator happy.

I agree that in essence it should actually contain leaf selections for every possible subtype of the field's return type (e.g Human and Droid in your scenario), otherwise it could return an empty response.

In general empty selection was forbidden because most of the time they were used unintentionally. Forbidding empty objects was never a goal here.

Moreover adding such requirement will break a lot of existing queries that contradicts "Backwards compatibility" principle: https://github.com/graphql/graphql-spec/blob/master/CONTRIBUTING.md#guiding-principles

@IvanGoncharov IvanGoncharov added the 🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md) label Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

4 participants