Skip to content
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

Introduce SourceSchemaDocument and FullSchemaDocument #1049

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions spec/Appendix B -- Grammar Summary.md
Expand Up @@ -248,6 +248,8 @@ TypeSystemDefinition :

TypeSystemExtensionDocument : TypeSystemDefinitionOrExtension+

SourceSchemaDocument : TypeSystemDefinitionOrExtension+

TypeSystemDefinitionOrExtension :

- TypeSystemDefinition
Expand All @@ -258,6 +260,8 @@ TypeSystemExtension :
- SchemaExtension
- TypeExtension

FullSchemaDocument : TypeSystemDefinition+
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting that we are disallowing extensions in the FullSchemaDocument. I'm not opposed but this is interesting.

That feels a little odd, as the Relay Compiler tends to overload extension to mean client-defined (this is probably wrong and we should switch to using a directive).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with no extension by default to make a full schema as simple and as easy to read as possible.

A "full schema" is probably machine written and probably machine consumed as well. It's the source of truth for a service.

Another typical use case besides validation would be someone clicking their way in IntelliJ to find a type definition. I'd argue that having the extensions merged at that point would be nicer because you don't have to remember to "merge" things.

That being said, I think it'd be OK to "extend" a full schema:

client full schema = server full schema + type extensions

We'll probably end up encouraging this in Apollo Kotlin, possibly for stuff like @nullOnlyOnError


SchemaDefinition : Description? schema Directives[Const]? {
RootOperationTypeDefinition+ }

Expand Down
94 changes: 90 additions & 4 deletions spec/Section 2 -- Language.md
Expand Up @@ -242,17 +242,19 @@ Definition :
- ExecutableDefinition
- TypeSystemDefinitionOrExtension

A GraphQL Document describes a complete file or request string operated on by a
GraphQL service or client. A document contains multiple definitions, either
executable or representative of a GraphQL type system.

### Executable document

ExecutableDocument : ExecutableDefinition+

ExecutableDefinition :

- OperationDefinition
- FragmentDefinition

A GraphQL Document describes a complete file or request string operated on by a
GraphQL service or client. A document contains multiple definitions, either
executable or representative of a GraphQL type system.

Documents are only executable by a GraphQL service if they are
{ExecutableDocument} and contain at least one {OperationDefinition}. A Document
which contains {TypeSystemDefinitionOrExtension} must not be executed; GraphQL
Expand All @@ -275,6 +277,90 @@ operations, each operation must be named. When submitting a Document with
multiple operations to a GraphQL service, the name of the desired operation to
be executed must also be provided.

### Source schema document

SourceSchemaDocument : TypeSystemDefinitionOrExtension+

TypeSystemDefinitionOrExtension :

- TypeSystemDefinition
- TypeSystemExtension

TypeSystemDefinition :

- SchemaDefinition
- TypeDefinition
- DirectiveDefinition

A {SourceSchemaDocument} describes the user types of a schema. A GraphQL
implementation may use a {SourceSchemaDocument} as input to represent the schema
and execute operations.

For brevity, and to avoid conflicting with the implementation, all _built-in
definitions_ must be omitted in a {SourceSchemaDocument}.

**Default Root Operation Type Names**

:: The _default root type name_ for each {`query`}, {`mutation`}, and
{`subscription`} _root operation type_ are {"Query"}, {"Mutation"}, and
{"Subscription"} respectively.

The type system definition language can omit the schema definition when each
_root operation type_ uses its respective _default root type name_ and no other
type uses any _default root type name_.

Likewise, when part of a {SourceSchemaDocument}, a schema definition should be
omitted if each _root operation type_ uses its respective _default root type
name_ and no other type uses any _default root type name_.

This example describes a valid complete GraphQL schema, despite not explicitly
including a {`schema`} definition. The {"Query"} type is presumed to be the
{`query`} _root operation type_ of the schema.

```graphql example
type Query {
someField: String
}
```

This example describes a valid GraphQL schema without a {`mutation`} _root
operation type_, even though it contains a type named {"Mutation"}. The schema
definition must be included, otherwise the {"Mutation"} type would be
incorrectly presumed to be the {`mutation`} _root operation type_ of the schema.

```graphql example
schema {
query: Query
}

type Query {
latestVirus: Virus
}

type Virus {
name: String
mutations: [Mutation]
}

type Mutation {
name: String
}
```

### Full schema document

FullSchemaDocument : TypeSystemDefinition+

A {FullSchemaDocument} describes all the types in the schema, including the
Copy link
Contributor

Choose a reason for hiding this comment

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

"describes all the types in the schema" I'm not sure this is guaranteed to be true. It describes all the types that are visible to the consumer.

I know this is pedantic, but I'm currently using an equivalent of the "full schema document" in a way that says, for this library, what do we want to make available for codegen and validation, even if other types/fields exist and are exposed on the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point 👍 . I guess it depends how much we want to bind this to introspection but I think it'd have some value having the FullSchemas be a super set of the introspection schema.

built-in ones. Tools that are not implementations may use a {FullSchemaDocument}
to validate an operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this singular purpose captures the depth of why FullSchemaDocument is useful. It's validation, but also code generation, IDE features, and more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

💯 I'll reword


All _built-in definitions_ must be included in a {SourceSchemaDocument}.
Copy link
Contributor

Choose a reason for hiding this comment

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

FullSchemaDocument

Though I'm not sure this is true either. Any built-in definitions not included will be unavailable to tooling, and imply the schema is not spec compliant which is not the same as "must be included".

But for instance if there are no Float usages, does the full schema need to include the Float type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FullSchemaDocument

Indeed, thanks!

Though I'm not sure this is true either. Any built-in definitions not included will be unavailable to tooling, and imply the schema is not spec compliant which is not the same as "must be included".

Reason I went with this requirement is we're plagued with duplicate directive definitions. Latest example to date is apollographql/apollo-ios#3287. Right now, we've been adding directive definitions in order to validate documents but this really fragile to cases where the directive definitions do not match like this one.

if there are no Float usages, does the full schema need to include the Float type?

Good point. I guess it's not really required. The rule would then be "all used built-in definitions must be included", which is more flexible but also more prone to interpretation... But could be cool indeed.


A {SourceSchemaDocument} must exactly one {SchemaDefinition}. That
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
{SchemaDefinition} must include a _root operation type definition_ for each
supported operation.

## Operations

OperationDefinition :
Expand Down
78 changes: 5 additions & 73 deletions spec/Section 3 -- Type System.md
Expand Up @@ -5,8 +5,6 @@ used to determine if a requested operation is valid, to guarantee the type of
response results, and describes the input types of variables to determine if
values provided at request time are valid.

TypeSystemDocument : TypeSystemDefinition+

TypeSystemDefinition :

- SchemaDefinition
Expand All @@ -21,21 +19,14 @@ to provide utilities such as client code generation or service boot-strapping.
GraphQL tools or services which only seek to execute GraphQL requests and not
construct a new GraphQL schema may choose not to allow {TypeSystemDefinition}.
Tools which only seek to produce schema and not execute requests may choose to
only allow {TypeSystemDocument} and not allow {ExecutableDefinition} or
only allow {TypeSystemDefinition} and not allow {ExecutableDefinition} or
{TypeSystemExtension} but should provide a descriptive error if present.

Note: The type system definition language is used throughout the remainder of
this specification document when illustrating example type systems.

## Type System Extensions

TypeSystemExtensionDocument : TypeSystemDefinitionOrExtension+

TypeSystemDefinitionOrExtension :

- TypeSystemDefinition
- TypeSystemExtension

TypeSystemExtension :

- SchemaExtension
Expand All @@ -47,8 +38,8 @@ a local service to represent data a GraphQL client only accesses locally, or by
a GraphQL service which is itself an extension of another GraphQL service.

Tools which only seek to produce and extend schema and not execute requests may
choose to only allow {TypeSystemExtensionDocument} and not allow
{ExecutableDefinition} but should provide a descriptive error if present.
choose to only allow {TypeSystemDefinition} and {TypeSystemExtension} and not
allow {ExecutableDefinition} but should provide a descriptive error if present.

## Descriptions

Expand Down Expand Up @@ -139,6 +130,8 @@ All types and directives defined within a schema must not have a name which
begins with {"\_\_"} (two underscores), as this is used exclusively by GraphQL's
introspection system.

A document must include at most one {SchemaDefinition}.
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved

### Root Operation Types

:: A schema defines the initial _root operation type_ for each kind of operation
Expand Down Expand Up @@ -189,9 +182,6 @@ mutation {
}
```

When using the type system definition language, a document must include at most
one {`schema`} definition.

In this example, a GraphQL schema is defined with both a query and mutation
_root operation type_:

Expand All @@ -210,55 +200,6 @@ type MyMutationRootType {
}
```

**Default Root Operation Type Names**

:: The _default root type name_ for each {`query`}, {`mutation`}, and
{`subscription`} _root operation type_ are {"Query"}, {"Mutation"}, and
{"Subscription"} respectively.

The type system definition language can omit the schema definition when each
_root operation type_ uses its respective _default root type name_ and no other
type uses any _default root type name_.

Likewise, when representing a GraphQL schema using the type system definition
language, a schema definition should be omitted if each _root operation type_
uses its respective _default root type name_ and no other type uses any _default
root type name_.

This example describes a valid complete GraphQL schema, despite not explicitly
including a {`schema`} definition. The {"Query"} type is presumed to be the
{`query`} _root operation type_ of the schema.

```graphql example
type Query {
someField: String
}
```

This example describes a valid GraphQL schema without a {`mutation`} _root
operation type_, even though it contains a type named {"Mutation"}. The schema
definition must be included, otherwise the {"Mutation"} type would be
incorrectly presumed to be the {`mutation`} _root operation type_ of the schema.

```graphql example
schema {
query: Query
}

type Query {
latestVirus: Virus
}

type Virus {
name: String
mutations: [Mutation]
}

type Mutation {
name: String
}
```

### Schema Extension

SchemaExtension :
Expand Down Expand Up @@ -407,9 +348,6 @@ referenced built-in scalars must be included. If a built-in scalar type is not
referenced anywhere in a schema (there is no field, argument, or input field of
that type) then it must not be included.

When representing a GraphQL schema using the type system definition language,
all built-in scalars must be omitted for brevity.

**Custom Scalars**

GraphQL services may use custom scalar types in addition to the built-in
Expand Down Expand Up @@ -1949,12 +1887,6 @@ schema.
GraphQL implementations that support the type system definition language should
provide the `@specifiedBy` directive if representing custom scalar definitions.

When representing a GraphQL schema using the type system definition language any
_built-in directive_ may be omitted for brevity.

When introspecting a GraphQL service all provided directives, including any
_built-in directive_, must be included in the set of returned directives.

**Custom Directives**

:: GraphQL services and client tooling may provide any additional _custom
Expand Down