A list of small issues in the spec

[rivantsov]

Hi all
I've been finding small things in the spec, typos or small mistakes, for a while now. Tried to write them down to report later, but kept losing them. Finally, decided to spend some time, go thru the spec line-by-line and get these little annoying things into one list. Below is the result. The issues are in the order as they appear in the spec. Some are obvious typos, others may require some discussions.

I assigned each item an ID like P1, P2 etc, for easier references. We should probably first discuss how to better handle the list (track it), maybe some spreadsheet or smth. Feedback, comments are welcome.

thank you

---

P1

Section "2 Language"
https://spec.graphql.org/October2021/#sec-Language

A GraphQL document is defined as a syntactic grammar where terminal symbols are tokens (indivisible lexical units).

1. Maybe 'is defined BY a syntactic grammar' ?
2. In commonly accepted terminology, terminals are different from tokens; terminals are elements of grammar (lexing rules), while tokens are source fragments produced by lexer at runtime; tokens are instances of terminals. Tokens relate to Terminals is the same as objects relate to classes.
   Wiki: "... lexing or tokenization is the process of converting a sequence of characters... into a sequence of tokens ...".
   Link: https://en.wikipedia.org/wiki/Lexical_analysis
   In general the sentence does not make much sense. Better version: The structure of a GraphQL document is defined by a syntactic grammar and lexical definitions (terminal symbols).

---

P2

Section 2.1.3 Line Terminators
https://spec.graphql.org/October2021/#sec-Line-Terminators

Line terminators are not found within any other token.

Not true, block string can contain line terminators

---

P3

Section 2.5 Fields
https://spec.graphql.org/October2021/#sec-Language.Fields

All GraphQL operations must specify their selections down to fields which return scalar values to ensure an unambiguously shaped response.

Should be 'scalar or enum values, or lists of those types'; technically enums are not scalars. We can introduce 'primitive' types = scalars + enums, and lists of these.

---

P4

Section 2.8.2 Inline Fragments
https://spec.graphql.org/October2021/#sec-Inline-Fragments

This is done to conditionally include fields based on their runtime type.

should be: ... based on parent object's runtime type.

---

P5

Section 2.10 Variables
https://spec.graphql.org/October2021/#sec-Language.Variables

A GraphQL operation can be parameterized with variables, maximizing reuse, and avoiding costly string building in clients at runtime.

Well, sounds like a typical cry-baby client dev - oh, building strings is so hard! Hard?! Try parsing them!

Now seriously. We should point to the real win - reusing parsed queries on the server side: "... maximizing reuse of parsed queries by the GraphQL service."

---

P6

Section 3. Type system
Sub-section 3.3.1 Root Operation Types,
sub-section Default Root Operation Type Names
https://spec.graphql.org/October2021/#sec-Root-Operation-Types.Default-Root-Operation-Type-Names

While any type can be the root operation type for a GraphQL operation, the type system definition language can omit the schema definition when the query, mutation, and subscription root types are named "Query", "Mutation", and "Subscription" respectively.
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.

1. It feels like the second paragraph desperately tries to say something different from the first paragraph, but looks like it fails.
2. In the second paragraph, is it supposed to be "may/might be omitted" or "should be omitted"? I think 'might' would be correct, like in the first paragraph, skipping is optional.

---

P7

Section 3.6 Objects, sub-section Result Coersion,
https://spec.graphql.org/October2021/#sec-Objects.Result-Coercion

Determining the result of coercing an object is the heart of the GraphQL executor, so this is covered in that section of the spec

• which that section? there is no section "GraphQL executor". Need to provide an exact ref here.

---

P8

Section 3.12.1 Combining List and Non-Null
http://spec.graphql.org/October2021/#sec-Combining-List-and-Non-Null

When a field error is raised on a non-null value, the error propagates to the parent field. For more information on this process, see “Errors and Non-Nullability” within the Execution section.

There is no such section or subsection. There is section "Errors and Non-Null Fields"
http://spec.graphql.org/October2021/#sec-Executing-Selection-Sets.Errors-and-Non-Null-Fields

---

P9

Section 4 Introspection, sub-section Introspection/Reserved Names
https://spec.graphql.org/October2021/#sec-Introspection.Reserved-Names
talks about reserved "__" prefix, but not particular reserved names. Spec reserves a prefix, not names. Section better be titled "Name restrictions" and should be moved to 'type system'. There is also "Reserved Names" subsection in chapter 2: https://spec.graphql.org/October2021/#sec-Names.Reserved-Names - probably should be renamed too.

By the way, we do have reserved words: 'on', 'true', 'false', may be some others; they are explicitly mentioned in Grammar as lookahead restrictions or non-names. We just need to explicitly declare these as Reserved Names(words) - those names that cannot be used for custom identifiers. It is a common practice in programming languages.

---

P10

Section 4.2.2 The _Type Type, sub-section Introspection/Interfaces
https://spec.graphql.org/October2021/#sec-The-__Type-Type.Interface

interfaces must return the set of interfaces that an object implements (if none, interfaces must return the empty set).

should be "that an INTERFACE implements"

---

P11

Section 5 Validation
https://spec.graphql.org/October2021/#sec-Validation

GraphQL does not just verify if a request is ...

GraphQL-who? Who verifies? GraphQL? is it a person? Or code?

a service may validate a request once and memoize the result to avoid validating the same request again in the future.

one of these cases when spec suggests that server devs are stupid, and don't know about caching techniques. We should remove this statement. Server devs do not need permission from spec to do optimization tricks like caching if they do not change the outcome (the visible functionality of the service)

---

P12

Section Subscriptions/Delivery Agnostic
https://spec.graphql.org/October2021/#sec-Subscription.Delivery-Agnostic

Subscriptions specifies algorithms for the creation of a stream

Typo: should be 'specify' (for plural form)
Specifies algorithms? - maybe specifies parameters/settings/env, but not algorithms?

---

P13

Section 5.3.2 Field Selection Merging
right after this example: https://spec.graphql.org/October2021/#example-a2230

Identical arguments are also merged if they have identical arguments

comment: not sure what it means; maybe 'identical FIELDS are also merged...'?

(same section)

However, the field responses must be shapes which can be merged. For example, scalar values must not differ. In this example, someValue might be a String or an Int

in the context of the example, probably it should be "scalar types must not differ"

---

P14

Section 5.3.3 Leaf Field Selections
Subsection Formal Specification
https://spec.graphql.org/October2021/#sec-Leaf-Field-Selections.Formal-Specification

If selectionType is a scalar or enum: The subselection set of that selection must be empty
If selectionType is an interface, union, or object The subselection set of that selection must NOT BE empty

In both cases, array type of these types must be included. Also, there's a semicolon after first condition, but not after the second one (after ', or object'). The same (about arrays) applies to text below in Explanatory Text:

Conversely the leaf field selections of GraphQL operations must be of type scalar or enum.

---

P15

Section 6 Execution
https://spec.graphql.org/October2021/#sec-Execution

GraphQL generates a response from a request via execution.

Should be GraphQL service.

---

P16

Section 6.2.2
https://spec.graphql.org/October2021/#sec-Mutation

It is expected that the top level fields in a mutation operation perform side-effects on the underlying data system. Serial execution of the provided mutations ensures against race conditions during these side-effects.

'ensures against race conditions' - no it does not; what about concurrent requests from other clients?! Server holds data for and serves it to many clients, concurrently; in many cases data is shared between clients. This statement is just silly, should be removed.

---

P17

Section 6.2.2.3 Unsubscribe
https://spec.graphql.org/October2021/#sec-Unsubscribe
(read the section)
what exactly it specifies? the exact method/format/request of cancelling the stream? Is it another field/method in Subscription or Mutation object? what kind of call cancels subscription? Or it is canceled by just disconnecting of long-lived connection (socket or smth)

---

P18

Section 6.2.3 Subscription
Note inlet http://spec.graphql.org/October2021/#note-79d56

In large scale subscription

Should be 'In large-scale subscription', with hyphen: https://grammarhow.com/large-scale-or-large-scale/

---

P19

Subsection Supporting Subscriptions at Scale
http://spec.graphql.org/October2021/#sec-Subscription.Supporting-Subscriptions-at-Scale

Supporting subscriptions is a significant change for any GraphQL service.

should it be 'significant challenge'?

---

P20

Subsection Delivery Agnostic
http://spec.graphql.org/October2021/#sec-Subscription.Delivery-Agnostic

Subscriptions specifies algorithms for the creation of a stream,

typo: should be 'specify'

---

P21

Section 6.3 Executing Selection Sets
Algorithm ExecuteSelectionSet
http://spec.graphql.org/October2021/#ExecuteSelectionSet()

3.b. Let fieldType be the return type defined for the field fieldName of objectType.
3.c. If fieldType is defined:

How is it possible that fieldType is not defined?! and what happens if it's not?

---

P22

Section 6.3.1 Normal and Serial execution
https://spec.graphql.org/October2021/#sec-Normal-and-Serial-Execution

Because the resolution of fields other than top-level mutation fields must always be side effect-free and idempotent, the execution order must not affect the result

1. The starting word "Because" suggests that the statement that immediately follows (the reason) - is already established, mandated, explained or proved. But in the spec, I did not find any prio mentioning of the subject, or terms effect-free or idempotent. The spec did not establish this.
2. 'fields other than top-level mutation fields' - meaning query fields and subscription (!) fields should be side effect-free and idempotent?
3. More common hyphenation of 'side effect free' appears to be 'side-effect free' or with 3 hyphens. Ex:
   https://en.wikipedia.org/wiki/Side_effect_(computer_science)#Referential_transparency (at the end of paragraph)
   https://www.sciencedirect.com/science/article/abs/pii/0096055195000089
4. Should we establish that query fields are side-effect free and idempotent? - I do not think so. Doing this will make it illegal to create APIs like stock quotes; or simply getTime(); or anything that can change over time (ex: total bid count on bidding site).
   idempotent: "... can be applied multiple times without changing the result beyond the initial application." (wikipedia).

---

P23

Examples 191, 192
http://spec.graphql.org/October2021/#example-33b6a
http://spec.graphql.org/October2021/#example-c91a3

These two are examples of mutations. They should not use a shorthand format (without operation type and name) - it is allowed only for queries; they should explicitly use 'mutation' keyword.

---

P24

Section 6.4.3 Value Completion
Subsection Merging Selection Sets
http://spec.graphql.org/October2021/#sec-Value-Completion.Merging-Selection-Sets

When more than one field of the same name is executed in parallel, their selection sets are merged together ...

1. Why it is only for 'parallel' execution? Does it mean that if server chooses to execute serially, the result would be different (not merged)?
2. It would be helpful to provide a sample result json of merged fields, with merged 'me' node

---

P25

Section 7.1.1 Data
http://spec.graphql.org/October2021/#sec-Data
The first paragraph describes the response object for query, mutation. Should also mention Subscription for completeness.

---

P26

Section 7.1.2 Errors
http://spec.graphql.org/October2021/#sec-Errors

If the data entry in the response is present (including if it is the value null), the errors entry in the response may contain any field errors that were raised during execution. If field errors were raised during execution, it should contain those errors.

The last sentence appears to be a repeat (in meaning) of the previous one.

---

P27

Subsection Request errors
http://spec.graphql.org/October2021/#sec-Errors.Request-errors

This may occur due to a parse grammar or validation error in the requested document ...

should be 'in the request document'

---

P28

Consistency in capitalization of titles of non-numbered subsections
In chapter 7 "Response", all subsection titles without numbers are capitalized with Sentence style, for ex: 'Request errors'.
In chapter 6 "Execution", subsection titles without numbers are capitalized in First-cap style, for ex: 'Event Streams'

---

P29

General observation, multiple times throughout the spec
Throughout the doc 'GraphQL' is used as an 'actor' - GraphQL validates, GraphQL executes, etc. As if it can act, as a person or software actor. But it is an abstract thing, it cannot do stuff. For real actions, we should be more explicit: GraphQL Service does this or that.
Examples of current use:

Validation: GraphQL does not just verify if a request is syntactically correct,
Execution: GraphQL generates a response from a request via execution.

[leebyron]

Following up from the working group call today, thank you for the careful read through of the spec and capturing all of these improvements.

The best way to track and address these kinds of issues are with spec PRs. These are all Editorial changes, which we typically review without the need for a live working group discussion. The non-controversial changes we'll directly merge, and any that warrant discussion we can have that within the context of a focused PR.

[rivantsov]

thank you, will do

[rivantsov]

pushed the PR with fixes for 20 issues