Default Root Op type names: removed duplicate statement; clarified that only Query type required

[rivantsov]

Default Root Op type names: removed duplicate statement, added that only Query type is required.
@benjie argued previously that the second sentence (starting with Likewise...) is not a duplicate, it's different, but its seems abs repeat for me. Except 'should omit' instead of 'can omit' - which I think is incorrect. If you have an optional arg with default value, you do not require it to be skipped if the explicit value is the same as default value.

[netlify]

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	fe8cee0
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/62e9d29eb9dab2000b9d84de
😎 Deploy Preview	https://deploy-preview-978--graphql-spec-draft.netlify.app/draft
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[yaacovCR]

The first sentence is talking about a schema built from SDL. The second sentence is talking about converting an existing schema into SDL. The directions are reversed SDL to schema vs schema to SDL.

This explains the can vs should:

The first line says that SDL used to build a schema can opt to not specify the default values. That is simply a statement of fact, the schema will be the same whether the default values are specified or not.

The second line says that an SDL representation of an existing schema (generated from an existing schema) should not necessarily specify the default values. Note that “should” is used rather than “must” so it would still be spec compliant to specify them. The spec does not say why they should not be specified, but my guess is that it beneficial to have a canonical way to represent an existing schema, and so the spec simply has to recommend something.

In my opinion, the room for improvement is not to remove the line, but rather to explain the use of “should.” Although there are many places in the graphql spec, as in other specifications, where reasons for the choice of can vs should vs must would be helpful to me…

[michaelstaib]

I think the sentence was very clear before, but now is confusing to read.

[benjie]

My previous comment on this was:

This is a concrete change in the interpretation of the GraphQL specification - the previous paragraph says that you can define a schema like this:

type Query { ... }
schema { query: Query }

or, equivalently, like this:

type Query { ... }

The paragraph you have deleted does not state the same thing again, it states the converse: when a GraphQL schema is represented via SDL (rather than defined by it; i.e. when you do printSchema(schema) in GraphQL.js or equivalent in other languages) you should omit the schema keyword under these circumstances.

I don't think the sentence you have added is a substitute for the paragraph you have deleted, and I believe the existing spec text is perfectly fine.

I favour no change.

[rivantsov]

@benjie and all

I believe there's some confusion here, and it's my fault, it smashed two enhancements in one. The sentence I added ("This rule applies also...") - this is NOT a replacement for the 'duplicate' second sentence that I removed. It addresses a separate issue.

From the current text about skipping the schema def, it is not clear if I have to define all 3 types (Query, Mutation, Subscription) to make skipping schema element possible, or I can define just Query, like in @benjie's example above. It actually reads like you have to define all three. I believe this clarification is necessary - that only Query is enough. My wording might not be perfect, it is not - no problem, suggest something better.

[benjie]

@rivantsov Good spot, there's actually a bit more ambiguity there; I've raised an alternative pull request at #987 which addresses this.

[rivantsov]

@benjie et al
Coming back to this second sentence in current version:

Likewise, when representing a GraphQL schema using the type system definition
language, a schema definition should be omitted if it only uses the default root
operation type names.

as far as I understood (finally), the key here is the word 'should' - I initially thought it's a mistake, a typo. But now I see, and it makes the whole meaning is even more 'strange'. What it says is: 'when SDL file is generated by a server/framework like graphql.js, the generator SHOULD omit schema element if default op names are used'.

Short version: if something is optional and CAN be omitted, it SHOULD be omitted . This sounds REALLY strange. Like for fields with optional args it would look like this, if you follow this rule.

   # field with arg 
   type MyType {
	 myField (arg: Int = 5) : Int 
   }

   # queries
   
   myField(arg: 3)   # ok 
   myField           # ok, default value 5 will be used 
   myField(arg: 5)   # wrong! error! if it can be omitted, it should be omitted!

Does it look reasonable? I think not. Not here, not with 'schema' element. It is up to the implementing framework/dev to omit or not - as long as the resulting file is valid - which it is. Just because graphql.js is doing it this way (always omitting) - it is not a reason to force it on everybody out there.

And by the way, there might be reasons to still define 'schema' element explicitly - for ex, there might be a directive defined on schema or its fields (query, mutation) - what to do then? (as far as I understand, dir location enum allows this).

So my suggestion - drop this 'should' requirement, it is unusual and unreasonable, and then the need for this second sentence goes away.

Thank you

[michaelstaib]

myField(arg: 5) # wrong! error! if it can be omitted, it should be omitted!

If you do not implement a should your implementation is still valid. So, it is up to the implementor to decide on this aspect. In general the should leads to a cleaner SDL but it still up to you to implement the should.

[benjie]

But now I see, and it makes the whole meaning is even more 'strange'. What it says is: 'when SDL file is generated by a server/framework like graphql.js, the generator SHOULD omit schema element if default op names are used'.

Let me attempt to explain this using my revised wording in #987.

Server

The type system definition language can omit the schema definition when each
root type present uses its respective default root operation type name and no
other type uses a default root operation type name.

This means: "when you are defining a GraphQL schema using SDL you don't need to add the schema keyword if you use the defaults"; i.e. my schema could be this:

const sdl = `
  type Query {
    xkcd221: Int
  }
`;
const resolvers = {
  Query: {
    xkcd221() {
      return 4; // chosen by fair dice roll.
                // guaranteed to be random.
    }
  }
};
export const schema = makeSchema(sdl, resolvers);

or it could be

const sdl = `
  type Query {
    xkcd221: Int
  }
  schema {
    query: Query
  }
`;
// ...

The specification doesn't care either way - they're equivalent as far as it's concerned, the details are lost as soon as the schema is built so it's irrelevant, hence the use of may.

No matter which option I choose, I have constructed an identical GraphQL schema. Let's imagine I use something like express-graphql to serve this GraphQL schema over HTTP.

Client

Now, I use my client, GraphiQL, to introspect the schema. It issues the query query IntrospectionQuery { __schema { ... } } to the server and gets a lot of JSON data as a response. This JSON fully describes the GraphQL schema, but is not so easy for humans to read. As a convenience, my version of GraphiQL includes functionality that can represent the GraphQL schema not as a big lump of JSON but as text that the user can read. To do so, it follows the guidance in the spec:

Likewise, when representing a GraphQL schema using the type system definition
language, a schema definition should be omitted if each root type present uses
its respective default root operation type name and no other type uses a
default root operation type name.

So the client will render to the user:

type Query {
  xkcd221: Int
}

It won't add schema { query: Query } to the definition because the specification recommends not to. Doing so would lead to a concretely different result, hence the should to encourage consistency.

[rivantsov]

Again, sorry for the delay in response. Stuff keeps happening. I really appreciate your detailed responses, guys.

@michaelstaib
thank you for pointing out that 'should' is actually SHOULD, in the formal meaning of RFC 2119; but I still think it is too strong of a recommendation:

SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

Meaning: we RECOMMEND this way, and you must have damn good reasons not to follow. That's too much. Why should I look for strong reasons to follow the path (add an explicit small schema element) that is absolutely valid in terms of the resulted doc? We (WG and spec) better simply RECOMMEND, at most - use this word instead of 'should'.

@benjie,
thank you for the detailed response. I understand your explanations, in short: if you handwrite the SDL doc - you CAN skip schema element; if SDL doc is produced by a tool - it should skip the 'schema' if possible. I have one minor and one major problems with this.

• minor problem: I do not think we (WG) should recommend one way or another. We stated when document is valid in the first sentence, and that's enough. I do not see the need to recommend to skip it, and therefore 2nd sentence ("Likewise...") is not needed. But I can live with that.
• major problem - the CASE for this recommendation. Looks like the assumption is that the current phrase

... when representing a GraphQL schema using the type system definition
language ...

is equivalent to

... when GraphQL SDL file is generated by a tool or some software ... (rivantsov: that's what I derived from extra explanations)

Well, I think this is a really wrong assumption. For a newcomer, the first (current) version is hell confusing, to the level of 'wtf does this mean?'. When we 'represent' rather than 'define'?! That's my BIG problem with this sentence.

So, to sum it up, my suggestion(s) are still, either

1. Remove this second sentence and take the neutral stance on skipping the schema element
2. OR if we keep it, change it to:

Likewise, when the GraphQL schema file is generated by a tool or software, it is recommended to omit the schema element if ...

[benjie]

minor problem: I do not think we (WG) should recommend one way or another. We stated when document is valid in the first sentence, and that's enough. I do not see the need to recommend to skip it, and therefore 2nd sentence ("Likewise...") is not needed. But I can live with that.

I think it should be recommended because it makes GraphQL schemas represented as SDL more consistent across different vendors. Following the recommendations of the spec, whitespace aside, the same GraphQL schema should have the same representation in SDL independent of the tooling used to generate it. (Same type order, same field order, same descriptions, same deprecations, same arguments in order, same directives in order, etc etc etc.)

... when GraphQL SDL file is generated by a tool or some software ... (rivantsov: that's what I derived from extra explanations)

That's not the full story - it's not specific to generating a "file", it's not limited to when using "a tool or some software" (it could be hand written), and it's specifically talking about representing a GraphQL schema that already exists. So a more full definition expanding on yours would be "when type system definition language is generated by a tool or some software or some other process to represent an existing GraphQL schema"

Since the process that triggers the TSDL to be generated is unimportant (it's any cause that means we're representing a GraphQL schema in TSDL), we can shorten this to: "when the type system definition language is used to represent a GraphQL schema"

We could rewrite that a little to give "when representing a GraphQL schema using the type system definition language." At this point we've come full circle. I prefer this final wording as the most straightforward and accurate without unnecessary narrowing of use-cases.

[rivantsov]

@benjie

Following the recommendations of the spec, whitespace aside, the same GraphQL schema should have the same representation in SDL independent of the tooling used to generate it. (Same type order, same field order, same descriptions, same deprecations, same arguments in order, same directives in order, etc etc etc.)

Same types order? where the spec says it that types should be printed in some specific order? For introspection API, __Schema.types is defined as a set, without any specific order.

Then, in what sense is it even possible to have 'the same' representation for different tools, if there's no specified order for anything? They will be equivalent, but textually quite different. You mention 'same arguments in order' - which order if spec specifically says that 'order of args is not important/irrelevant' ?

[rivantsov]

Closing this PR
the first part (mutation/subscr ambiguity) is fixed by another PR.
I still think the second paragraph (Likewise...) is extremely confusing, but I am out of arguments here.