-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Schema Coordinates #794
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
base: main
Are you sure you want to change the base?
Schema Coordinates #794
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your hard work on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me; a few optional tweaks suggested.
spec/Section 3 -- Type System.md
Outdated
Schema Coordinates are always unique. Each type, field, argument, enum value, or | ||
directive may be referenced by exactly one possible Schema Coordinate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels like something worth highlighting and being clear about as a guarantee in the spec
spec/Section 3 -- Type System.md
Outdated
For example, the following is *not* a valid Schema Coordinate: | ||
|
||
```graphql counter-example | ||
Entity.Business | ||
``` | ||
|
||
In this counter example, both `Entity.Business` and `Business` would refer to | ||
the `Business` type. Since it would be ambiguous what the "primary key" should | ||
be in an application that uses Schema Coordinates to reference types, this is | ||
not allowed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this waffle? thought it might be helpful to have a little explanation/demonstration of this property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could just be
- Since it would be ambiguous what the "primary key" should be in an application that uses Schema Coordinates to reference types, this is not allowed.
+ Such ambiguity is disallowed (and hence why this is is invalid syntax).
(More terse, but doesn't walk the reader through why this is bad)
Voting lines are now open!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this counter example,
Entity.Business
is redundant sinceBusiness
already uniquely identifies theBusiness
type. Such redundancy is disallowed by this spec - every type, field, field argument, enum value, directive, and directive argument has exactly one canonical Schema Coordinate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thanks benjie, stealing this wording
Happy new year! 🥳 It's possible we still want more time for the RFC to marinate, but I'll attend so I can be available to folks to discuss the schema coordinates rfc/spec edit in person. graphql/graphql-spec#794 Thanks!
At Indeed we log usage of schema elements to 1) support deprecation and 2) understand usage from a product management perspective (i.e. is this API we built adding value?). We defined our own coordinate system that is not quite as complete as this spec. A nice benefit of formalizing a coordinate system is that shared tools can be built to solve these and other use cases. 👍 👍 👍 |
4f854af
to
c0b82d5
Compare
I made some fairly significant edits to the prose and examples section in an attempt to simplify. @magicmark please take a look and let me know what you think. |
81ef020
to
4299d8c
Compare
IIRC the only thing holding this from Approval stage is a landed GraphQL.js PR? |
This PR is applied to the `schema-coordinates` branch PR here: #3044 Implements schema coordinate spec changes per the June 5th 2025 WG discussion graphql/graphql-spec#794 - Add support for meta-fields (e.g. `__typename`) - Add support for introspection types - Revert back from FieldCoordinate+ValueCoordinate -> MemberCoordinate cc @benjie
cb28329
to
802d378
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really excited that we're getting close to this being ready to launch!
We still need to reach a consensus on the "meta fields" and "introspection types" question. I think the real issue comes down to "resolution" and I think it's implementation-defined whether or not you can "resolve" a meta field or a member of the introspection schema (since these may be implemented in completely different ways to regular fields/types/etc) - and thus we should exclude them from schema element explicitly, and let the rest of the logic flow from there. This doesn't make them "invalid" so much as "implementation defined behavior".
Co-authored-by: Benjie <benjie@jemjie.com>
Applied - makes sense and looks a bit cleaner. Thanks @benjie! The spec as written does now make it clear that these are implementation-defined, so that's the current proposal which hopefully aligns with the majority opinion. |
Co-authored-by: Benjie <benjie@jemjie.com>
@magicmark Here's my original diff that I drafted up locally: diff --git a/spec/Section 2 -- Language.md b/spec/Section 2 -- Language.md
index e422d9a..7a6dc1f 100644
--- a/spec/Section 2 -- Language.md
+++ b/spec/Section 2 -- Language.md
@@ -113,6 +113,8 @@ Comments are {Ignored} like white space and may appear after any token, or
before a {LineTerminator}, and have no significance to the semantic meaning of a
GraphQL Document.
+Comments are forbidden within a _schema coordinate_.
+
### Insignificant Commas
Comma :: ,
@@ -154,25 +156,27 @@ Ignored ::
- Comma
{Ignored} tokens are used to improve readability and provide separation between
-lexical tokens, but are otherwise insignificant and not referenced in
-syntactical grammar productions.
+lexical tokens in a GraphQL document, but are otherwise insignificant and not
+referenced in syntactical grammar productions. {Ignored} are forbidden within a
+_schema coordinate_.
-Any amount of {Ignored} may appear before and after every lexical token. No
-ignored regions of a source document are significant, however {SourceCharacter}
-which appear in {Ignored} may also appear within a lexical {Token} in a
-significant way, for example a {StringValue} may contain white space characters.
-No {Ignored} may appear _within_ a {Token}, for example no white space
-characters are permitted between the characters defining a {FloatValue}.
+Any amount of {Ignored} may appear before and after every lexical token in a
+GraphQL document. No ignored regions of a source document are significant,
+however {SourceCharacter} which appear in {Ignored} may also appear within a
+lexical {Token} in a significant way, for example a {StringValue} may contain
+white space characters. No {Ignored} may appear _within_ a {Token}, for example
+no white space characters are permitted between the characters defining a
+{FloatValue}.
**Byte Order Mark**
UnicodeBOM :: "Byte Order Mark (U+FEFF)"
-The _Byte Order Mark_ is a special Unicode code point which may appear at the
+:: The _Byte Order Mark_ is a special Unicode code point which may appear at the
beginning of a file which programs may use to determine the fact that the text
stream is Unicode, and what specific encoding has been used. As files are often
concatenated, a _Byte Order Mark_ may appear before or after any lexical token
-and is {Ignored}.
+and is {Ignored}. {UnicodeBOM} is forbidden within a _schema coordinate_.
### Punctuators Ultimately I decided that section 2 of the spec is about the GraphQL language (for GraphQL documents) which does not specifically relate to schema coordinates, and so instead of talking about schema coordinates in section 2 before they are introduced, I would instead have schema coordinates reference section 2 and apply the exception on top. I'm not sure if this is the right decision - @leebyron would be able to better weigh in here - but if we are to comment in section 2 then here are the kind of changes that I would have made (i.e. adding "in a GraphQL document" to differentiate such from a "schema coordinate"). |
This reverts commit f4eec91.
e4bdce8
to
c8375f0
Compare
…ordinates_spec_edit
c8375f0
to
4c39cda
Compare
ack @benjie
makes sense, I was wondering about this too. seems overkill I agree, but something may be in order. Anyway I've removed it from this PR for now, and made a separate follow-up PR that can be merged on top of this, should we decide this clarification (or similar) is appropriate. magicmark#2 |
👋
I've pulled out the proposed spec edits for Schema Coordinates (issue: #735) into this PR.
(The RFC now lives in the original PR: https://github.com/graphql/graphql-spec/pull/746/files)
@leebyron you mentioned it might be possible to simplify the grammar - I played around a bit since the original PR, but I think I need some help here. What did you have in mind?
As suggested, tagging @eapache @mjmahone as additional reviewers as we had some good conversation in last WG meeting about this.
Thanks!
GraphQL.js PR: graphql/graphql-js#3044