Introduce SourceSchemaDocument and FullSchemaDocument #1049

martinbonnin · 2023-10-04T15:05:14Z

Supersedes #1036

See also graphql/graphql-wg#1401 for the glossary.

This PR is a draft of what spec edits might look like to give a more precise idea of what it could end up like. Terminology and style to be fine tuned.

netlify · 2023-10-04T15:05:20Z

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	`00a3f3a`
🔍 Latest deploy log	https://app.netlify.com/sites/graphql-spec-draft/deploys/655b9d095875c7000853e5d6
😎 Deploy Preview	https://deploy-preview-1049--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 configuration.

…ing additive for now

mjmahone

I am still a fan of this change

mjmahone · 2023-11-20T15:33:23Z

spec/Appendix B -- Grammar Summary.md

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

+FullSchemaDocument : TypeSystemDefinition+


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).

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

mjmahone · 2023-11-20T15:36:53Z

spec/Section 2 -- Language.md

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


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.

💯 I'll reword

mjmahone · 2023-11-20T15:39:18Z

spec/Section 2 -- Language.md

+
+FullSchemaDocument : TypeSystemDefinition+
+
+A {FullSchemaDocument} describes all the types in the schema, including the


"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.

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.

mjmahone · 2023-11-20T15:41:43Z

spec/Section 2 -- Language.md

+built-in ones. Tools that are not implementations may use a {FullSchemaDocument}
+to validate an operation.
+
+All _built-in definitions_ must be included in a {SourceSchemaDocument}.


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?

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.

spec/Section 2 -- Language.md

spec/Section 3 -- Type System.md

Co-authored-by: Matt Mahoney <mahoney.mattj@gmail.com>

martinbonnin

Thanks for the review 💙

martinbonnin · 2023-11-20T17:33:36Z

spec/Appendix B -- Grammar Summary.md

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

+FullSchemaDocument : TypeSystemDefinition+


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

martinbonnin · 2023-11-20T17:40:51Z

spec/Section 2 -- Language.md

+
+FullSchemaDocument : TypeSystemDefinition+
+
+A {FullSchemaDocument} describes all the types in the schema, including the


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.

martinbonnin · 2023-11-20T17:41:01Z

spec/Section 2 -- Language.md

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


💯 I'll reword

martinbonnin · 2023-11-20T17:43:37Z

spec/Section 2 -- Language.md

+built-in ones. Tools that are not implementations may use a {FullSchemaDocument}
+to validate an operation.
+
+All _built-in definitions_ must be included in a {SourceSchemaDocument}.


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.

introduce source schema document and full schema document

6916e89

martinbonnin mentioned this pull request Oct 4, 2023

Add a note about omitting introspection type definitions #1036

Closed

keep TypeSystemDocument and TypeSystemExtensionDocument, make everyth…

58a3a75

…ing additive for now

benjie mentioned this pull request Oct 6, 2023

[2023-10] Everyone - please review Full Schemas RFC graphql/graphql-wg#1415

Closed

mjmahone reviewed Nov 20, 2023

View reviewed changes

martinbonnin and others added 2 commits November 20, 2023 18:49

Update spec/Section 3 -- Type System.md

65c9e41

Co-authored-by: Matt Mahoney <mahoney.mattj@gmail.com>

code review

00a3f3a

martinbonnin commented Nov 20, 2023

View reviewed changes

benjie added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label Nov 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce SourceSchemaDocument and FullSchemaDocument #1049

Introduce SourceSchemaDocument and FullSchemaDocument #1049

martinbonnin commented Oct 4, 2023

netlify bot commented Oct 4, 2023 •

edited

mjmahone left a comment

mjmahone Nov 20, 2023

martinbonnin Nov 20, 2023

mjmahone Nov 20, 2023

martinbonnin Nov 20, 2023

mjmahone Nov 20, 2023

martinbonnin Nov 20, 2023

mjmahone Nov 20, 2023

martinbonnin Nov 20, 2023

martinbonnin left a comment

martinbonnin Nov 20, 2023

martinbonnin Nov 20, 2023

martinbonnin Nov 20, 2023

martinbonnin Nov 20, 2023

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


		FullSchemaDocument : TypeSystemDefinition+

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

Introduce SourceSchemaDocument and FullSchemaDocument #1049

Are you sure you want to change the base?

Introduce SourceSchemaDocument and FullSchemaDocument #1049

Conversation

martinbonnin commented Oct 4, 2023

netlify bot commented Oct 4, 2023 • edited

✅ Deploy Preview for graphql-spec-draft ready!

mjmahone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinbonnin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Oct 4, 2023 •

edited