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

[RFC] Custom Scalar Specification URIs #635

Open
eapache opened this issue Nov 1, 2019 · 11 comments
Open

[RFC] Custom Scalar Specification URIs #635

eapache opened this issue Nov 1, 2019 · 11 comments
Labels
📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md)

Comments

@eapache
Copy link
Member

eapache commented Nov 1, 2019

(This RFC grew out of the discussion around #579 and is championed by @eapache and @andimarek.)

This RFC proposes attaching an optional URI to custom scalar definitions pointing to a document holding data-format, serialization, and coercion rules for the scalar.

Problem

The set of scalars provided within the main GraphQL specification is limited to primitive types (Int, String, etc) plus ID. In the wild, most schemas end up implementing several additional custom scalars for types like Date and URL; in fact the GraphQL specification currently recommends those two as examples of motivating use cases for custom scalars, and there have been proposals to add them to the main GraphQL specification (e.g. #315, #579). There are several closely-related problems with the world that has grown around the usage of these custom scalars:

  • Schema authors have to pack the entire format of custom scalars into the description field which often results in scalar formats being seriously underspecified. Compare the built-in scalar types, which each take several hundred words to define in the current spec.
  • Clients and tools have no way to introspect which custom scalar conforms to which format, and thus no way to dynamically provide additional functionality based on that information (for example, a date-picker widget for date-based inputs).
  • Many subtly different variations of each scalar exist, depending on the precise name and formatting chosen (for example, there are numerous different ways to represent a date and time). This makes interoperability very difficult for clients or tools that have to consume multiple schemas by different authors.

Solution

Allow attaching a URI to custom scalar definitions, in both the SDL and via introspection. If present, the URI is expected to point to a definition of the data-format, serialization, and coercion rules for the scalar. This provides value because:

  • The linked page now provides ample space for schema authors to properly specify the format of the scalar.
  • Clients and tools can treat the URI as an opaque key and use it to reliably identify the data format being used (for example, recognizing that a scalar linking to RFC 3339 is a DateTime in some format, regardless of its name) in order to provide additional functionality. The URI is considered opaque in this context because client tooling need not be able to follow the link or understand the contents of the linked document, they need only be able to map the URI itself (e.g. https://tools.ietf.org/html/rfc3339) to some internal behaviour (e.g. a date-picker UI).
  • Support for additional scalar formats by common tools (e.g. GraphiQL) and the constraint of providing a specific URI defining the format will hopefully result in a gradual consolidation of the ecosystem around common formats for things like DateTime, without constraining schema authors who want to do something different.

Out of Scope

The following problems are closely related, but considered out of scope for this RFC. They could be tackled in later RFCs if there is interest:

  • A standard way of attaching format or specification compliance information to non-scalar types (e.g. "this object is a Relay-style Connection"). This RFC is limited to custom scalar definitions.
  • Defining standards for how a scalar format should be specified, what serialization data and information are required, etc. This RFC is limited to the ability to attach a spec to a custom scalar, and does not deal with what that spec should contain.

Alternatives and Variations

  • Define more common scalars (DateTime, URL, etc) in the main GraphQL specification alongside Int, String, ID, etc.
    • Pro: a much stronger standardization effect on the ecosystem.
    • Con: forcing the many production schemas who have existing scalars in the same area to choose between incompatibility, or a breaking schema change for their users.
    • Con: it becomes contentious to determine when a format is sufficiently common to bake into the spec (and thus deserves better tooling etc) vs when it isn’t, as there is no middle ground.
  • Allow attaching an array of URIs to custom scalar definitions.
    • Pro: many RFCs and standards formats evolve through the addition of new RFCs and formats which build upon the past. An array of URIs would allow schemas to indicate support for the proper set of standards rather than trying to pick the single correct document.
    • Con: complexity for the majority of cases which don’t need this.

Cost

The primary costs of this RFC are:

  • Complexity in the grammar and implementation.
  • It is an incompatible change to introspection: clients which adopt the RFC and query the new introspection field will not work with older servers which do not provide it.

Illustrative example

This example specifies a new “DateTime” scalar based on RFC3339. It should be published and hosted somewhere on graphql.org distinct from the main GraphQL spec.

The upstream RFC can be found here: https://www.ietf.org/rfc/rfc3339.txt.
And the errata here: https://www.rfc-editor.org/errata/rfc3339.

RFC3339 has some ambiguities or unclear sections. This spec aims to clear up these ambiguities without introducing new semantics or changing any of the semantics established in RFC3339.

The format for input and output for this scalar is the same.

This DateTime scalar represents a “date-time” as specified in section 5.6.
While RFC3339 allows also other formats (see Errata of the RFC) only “date-time” is accepted in this scalar.

The allowed separation character between “full-date” and “full-time” is “T”.
In RFC3330 “time-secfrac” is optional, and not limited to a specific amount of digits. This scalar requires it to be present with exactly three digits representing milliseconds.

This scalar also doesn’t allow “-00:00” as offset to describe an unknown offset (See 4.3. Unknown Local Offset Convention in the RFC)

Example of valid DateTime scalars:

2011-08-30T13:22:53.108Z
2011-08-30T13:22:53.108-03
2011-08-30T13:22:53.108+03:30

Example of invalid DateTime scalars:

2011-08-30T13:22:53.108912Z  -> too many secfrac digits 
2011-08-30T13:22:53.108 -> no offset
2011-08-30 -> no time
2011-08-30T13:22:53.108-00:00 -> negative offset representing unknown offset not allowed

Spec edits

#649

Concerns, challenges, and drawbacks

  • How do we specify a URL in the spec grammar? The two main options would be to a) simply reference the grammar from RFC3986 or b) only specify it as a string, and let the fact that it is a URL be mostly convention.
  • What is the best grammar for specifying the extra field in the SDL. Should we add a keyword, or reuse “implements”, or something?
@leebyron leebyron added the 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) label Nov 7, 2019
@leebyron
Copy link
Collaborator

leebyron commented Nov 7, 2019

Here's an alternate spec grammar and introspection idea for you:

scalar DateTime @specified(by: "https://tools.ietf.org/html/rfc3339")
Fields
* `kind` must return `__TypeKind.SCALAR`.
* `name` must return a String.
* `specifiedBy` may return an RFC3986-compliant URI or {null}.
* `description` may return a String or {null}.
* All other fields must return {null}.

This makes the SDL grammar change non-breaking, just defines a well-known directive (similar to @deprecated)

@michaelstaib
Copy link
Member

@leebyron I think that feels great and aligns good with @deprecated

@fotoetienne
Copy link
Contributor

fotoetienne commented Nov 7, 2019

An idea: A specified format for scalar specification documents.

Example: uri points to a json document that contains specified fields such as name, description etc.
This would allow for:

  • tooling can follow the link and create uniform documentation
  • extensibility. We could later add an optional field such as implementations

@eapache
Copy link
Member Author

eapache commented Nov 11, 2019

@leebyron adding a new directive seems just as breaking as changing the grammar? What happens to existing schemas that have already defined a custom directive with that name? Should we call it @__specified or something?

@eapache
Copy link
Member Author

eapache commented Nov 11, 2019

Other than the grammar/format, I have updated the main RFC and draft specification to address all of the comments from the working group meeting.

@leebyron
Copy link
Collaborator

leebyron commented Nov 11, 2019

Technically almost any change to GraphQL can be determined to be breaking since ultimately someone somewhere can theoretically be relying on the behavior we're about to change. In practice we need to determine the sort of breakage and how likely it is to effect real consumers.

For a grammar change, this will affect all current GraphQL SDL parsers. Upon seeing grammar they do not yet understand (eg. a consumer has not yet upgraded to a version of their tool which supports the new grammar) the tool will present a syntax error and no value can be had from this point. This would be likely to affect a large number of people who use the SDL and encounter services which start to support this feature.

Directives were added for exactly this reason. We can extend without creating syntactically breaking changes (tools which don't understand the directive can ignore it). Of course your point is totally correct that any newly specified directive has some probability of colliding with someone's existing custom directive. There's no way to get this probability to zero, only to make it smaller and smaller by picking directive names less and less likely to already be used. The tradeoff is that names that are less likely to be used are probably also less understandable and useable by consumers. So we enter a balancing act. We should seek to pick the best name possible while breaking the fewest (ideally none) existing customers.

Since custom directives are not something casual consumers of GraphQL are using (really only a small number of sophisticated tools), this seems like something we could do a quick check or poll for.

@eapache
Copy link
Member Author

eapache commented Nov 11, 2019

Makes sense. I'm not sure how to poll for something like that other than to ask it at the next working group; if there's a better venue I'm open to suggestions.

Longer-term it might make sense to pre-reserve a directive namespace for future spec usage the same way we reserve types and fields that start with __. I'm not sure what that would look like though, and if custom directives are as unused as you suggest then it may not matter.

@leebyron
Copy link
Collaborator

Technically this limitation is already in place https://graphql.github.io/graphql-spec/draft/#sec-Type-System.Directives.Validation as a consistency with type and field naming limitation, but no directives actually make use of it.

I think the __ made sense as being intentionally somewhat odd for optionally tapping into less often used meta information as part of introspection. It's a good way to say "these things are less important than your schema, so they're getting out of the way with an odd namespace" but maybe a less good way to express common or important concepts like this one, or @deprecated which don't expect to be competing with your schema.

@leebyron
Copy link
Collaborator

I filed #646 since I think this is worth elaborating on a bit in the spec itself.

@eapache
Copy link
Member Author

eapache commented Nov 20, 2019

I've finished the switch to a directive, and filed #649 with what should be a complete draft of the specification edits needed for this RFC. At the next working group meeting we can poll for any last concerns (particularly around the compatibility of adding a directive) but barring any objections I believe this is ready to move to Draft (RFC 2).

m14t added a commit to m14t/graphql-js that referenced this issue Dec 6, 2019
This in an implementation for a spec proposal:

* Spec proposal: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#649)
* Original issue: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#635)

Please let me know if I forgot something.
m14t added a commit to m14t/graphql-js that referenced this issue Dec 6, 2019
This in an implementation for a spec proposal:

* Spec proposal: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#649)
* Original issue: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#635)
@eapache
Copy link
Member Author

eapache commented Dec 10, 2019

We polled for concerns at the working group meeting, and found a few minor ones which I've addressed with tweaks to the spec PR. @m14t has been kind enough to create a draft implementation for graphql-js (graphql/graphql-js#2276) so I believe this is actually ready for Draft/stage2 now.

@IvanGoncharov IvanGoncharov added the 🚀 Next Stage? This RFC believes it is ready for the next stage label Dec 11, 2019
m14t added a commit to m14t/graphql-js that referenced this issue Dec 31, 2019
This in an implementation for a spec proposal:

* Spec proposal: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#649)
* Original issue: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#635)
@leebyron leebyron added 📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md) and removed 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) 🚀 Next Stage? This RFC believes it is ready for the next stage labels Jan 9, 2020
m14t added a commit to m14t/graphql-js that referenced this issue Mar 4, 2020
This in an implementation for a spec proposal:

* Spec proposal: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#649)
* Original issue: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#635)
m14t added a commit to m14t/graphql-js that referenced this issue Apr 25, 2020
This in an implementation for a spec proposal:

* Spec proposal: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#649)
* Original issue: [[RFC] Custom Scalar Specification URIs](graphql/graphql-spec#635)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

5 participants