Skip to content

Editorial: Type kinds #970

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

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

Editorial: Type kinds #970

wants to merge 1 commit into from

Conversation

leebyron
Copy link
Collaborator

@leebyron leebyron commented Jun 27, 2022

This provides definitions at the introduction of "Type System" for leaf type, composite type, and abstract type. Then, these definitions are used throughout the spec.


Inspired by #957 which had included clarifications around "leaf" types and fields. This uses definition syntax to formally define the terms so they can be used.

This ended up being a pretty big change, so I'd love a review

This provides definitions at the introduction of "Type System" for _leaf type_, _composite type_, and _abstract type_. Then, these definitions are used throughout the spec.
@leebyron leebyron requested a review from benjie June 27, 2022 19:05
@netlify
Copy link

netlify bot commented Jun 27, 2022

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit 3b00fdb
🔍 Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/62b9ff6434c4f2000956e6be
😎 Deploy Preview https://deploy-preview-970--graphql-spec-draft.netlify.app/draft
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

enumerable. GraphQL offers an `Enum` type in those cases, where the type
specifies the space of valid responses.
:: A _leaf type_ is a kind of type representing a primitive value which cannot
be further selected and thus form the leaves in a response tree. GraphQL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something does not match; if subject is 'kind' (singular) then the verb should be 'forms' (singular)

Copy link
Contributor

@rivantsov rivantsov Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General suggestion
I think 'kind of type' is confusing, since we have already TYPE_KIND, different kind. What we introduce here are groups of types, sort of, purely for easier references in the spec; these groups or categories do not appear anywhere at runtime or in graphql artifacts - purely for shorter and simpler spec references.

So instead of
A Leaf type is a kind of type representing ...

we can say simply
Leaf types are types representing

without even explicitly calling it a group or category

(Edit) PS - actually this way (without kind) is phrased in other cases, so we would need to just change this spot.


GraphQL supports two abstract types: interfaces and unions.
:: A _composite type_ is a type composed of other types via a set of named
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need consistency in capitalizing - if 'Leaf type' then 'Composite type'

A `Union` defines a list of possible types; similar to interfaces, whenever the
type system claims a union will be returned, one of the possible types will be
returned.
:: An _abstract type_ allows a GraphQL schema to introduce polymorphism; where a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be comma after polymorphism, not semicolon.

of which yield a value of a specific type. Object values should be serialized as
ordered maps, where the selected field names (or aliases) are the keys and the
result of evaluating the field is the value, ordered by the order in which they
appear in the selection set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might mention here that field names might repeat in this output map, as a result of field merging

differ from interfaces in that Object types declare what interfaces they
implement, but are not aware of what unions contain them.
GraphQL Unions are an _abstract type_ representing one of a list of GraphQL
Object possible types, but provides for no guaranteed fields between those
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provides -> provide ? since Unions (plural)

out in `possibleTypes`.
Interfaces are an _abstract type_ where there are common fields declared. Any
type that implements an interface must define all the fields with names and
types exactly matching. The implementations of this interface are explicitly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we require exact matching of type for implementing field. It can be derived/compatible type
Like interface declares:
boo : IBoo
while implementing type can declare
boo: Boo # implements IBoo

possible types of the scope and the spread, the spread is considered valid.
In the scope of an _abstract type_ (interface or union), a fragment spread of
another _abstract type_ can be used as long as there exists at least _one_
object type that exists in the intersection of the possible types of the scope
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there exists... that exists ...

suggest to rephrase

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good; great readability/cross-referencing improvements I think 👍

Comment on lines +270 to +271
:: A _leaf type_ is a kind of type representing a primitive value which cannot
be further selected and thus form the leaves in a response tree. GraphQL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit wordy, but...

Suggested change
:: A _leaf type_ is a kind of type representing a primitive value which cannot
be further selected and thus form the leaves in a response tree. GraphQL
:: A _leaf type_ is a kind of type representing a primitive value which cannot
be further selected, thus the type of each leaf in a response tree is a leaf type. GraphQL

`Object` types, which define a set of fields, where each field is another type
in the system, allowing the definition of arbitrary type hierarchies.
A `Scalar` represents a primitive scalar value, such as a string or number.
Oftentimes, the possible responses for a scalar field are enumerable. GraphQL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Oftentimes, the possible responses for a scalar field are enumerable. GraphQL
Oftentimes, the possible responses for a leaf field are enumerable. GraphQL

@@ -635,14 +646,14 @@ FieldDefinition : Description? Name ArgumentsDefinition? : Type
Directives[Const]?

GraphQL operations are hierarchical and composed, describing a tree of
information. While Scalar types describe the leaf values of these hierarchical
information. While _leaf types_ describe the leaf values of these hierarchical
operations, Objects describe the intermediate levels.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objects (and lists) descr...

selected field names (or aliases) are the keys and the result of evaluating the
field is the value, ordered by the order in which they appear in the selection
set.
GraphQL Objects are a _composite type_ representing a list of named fields, each
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
GraphQL Objects are a _composite type_ representing a list of named fields, each
GraphQL Objects are a _composite type_ comprised of a list of named fields, each

Comment on lines +1066 to +1068
GraphQL interfaces are an _abstract type_ which when used as the type of a field
provides polymorphism where any implementation may be a possible type during
execution.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously it was stated that an interface may implement an interface. So "any implementation" could be perceived as including object types and interfaces. Let's clarify:

Suggested change
GraphQL interfaces are an _abstract type_ which when used as the type of a field
provides polymorphism where any implementation may be a possible type during
execution.
GraphQL interfaces are an _abstract type_ which when used as the type of a field
provides polymorphism where any implementing object type may be a possible type during
execution.

Comment on lines +354 to +356
Input Objects are a _composite type_ defined as a list of named input field
values. They are only used as inputs to arguments and variables and cannot be a
field return type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to make it clear that input objects can be comprised of other input objects, and thus they are not only the full input to arguments but also parts thereof. I'm not sure if dropping the s conveys this subtlety, but...

Suggested change
Input Objects are a _composite type_ defined as a list of named input field
values. They are only used as inputs to arguments and variables and cannot be a
field return type.
An Input Object is a _composite type_ defined as a list of named input field
values. They are only used as input to arguments and variables and cannot be a
field return type.

@@ -446,7 +446,7 @@ SameResponseShape(fieldA, fieldB):
- If {typeA} or {typeB} is Scalar or Enum:
- If {typeA} and {typeB} are the same type return true, otherwise return
false.
- Assert: {typeA} and {typeB} are both composite types.
- Assert: {typeA} and {typeB} are both Object, Interface or Union.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be considered as (a is object and b is object) or (a is interface and b is interface) or (a is union and b is union). I think it's clearer without this change; alternatively use "each":

Suggested change
- Assert: {typeA} and {typeB} are both Object, Interface or Union.
- Assert: each of {typeA} and {typeB} are an Object, Interface or Union.

Comment on lines +586 to +587
A field subselection is not allowed on a _leaf field_, a field with a Scalar or
Enum unwrapped type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use parenthesis to make it clear this isn't a list:

Suggested change
A field subselection is not allowed on a _leaf field_, a field with a Scalar or
Enum unwrapped type.
A field subselection is not allowed on a _leaf field_ (a field with a Scalar or
Enum unwrapped type).

exists at least _one_ object type that exists in the intersection of the
possible types of the scope and the spread, the spread is considered valid.
In the scope of an _abstract type_ (interface or union), a fragment spread of
another _abstract type_ can be used as long as there exists at least _one_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emphasis unnecessary?

Suggested change
another _abstract type_ can be used as long as there exists at least _one_
another _abstract type_ can be used as long as there exists at least one

Comment on lines +726 to +727
When completing a field with an _abstract type_, that is an Interface or Union
return type, first the abstract type must be resolved to a relevant Object type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative wording:

Suggested change
When completing a field with an _abstract type_, that is an Interface or Union
return type, first the abstract type must be resolved to a relevant Object type.
When completing a field whose return type is an _abstract type_ (an Interface or Union), first the abstract type must be resolved to a suitable Object type.

"a suitable object type" or "the relevant object type" I think?

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.

3 participants