Add OptInFeatures.md

[martinbonnin]

Rendered proposal

This is a work in progress centralising the latest state of graphql/graphql-spec#943 in a more reader-friendly way than the Github thread.

Comments are welcome here or at graphql/graphql-spec#943

[jturkel]

Do tools also need a mechanism for listing all of the @optIn features in a schema? I'm imagining tools like GraphiQL showing users a list of possible features they can opt into rather than requiring users to somehow obtain this information via documentation or some other non-introspection based mechanism.

[martinbonnin]

Good point 👍 If types are not included in introspection in the first place, there's no way of knowing what possible values are for includeOptIn...

That'd be an argument for marker directives because these user-defined directive would be returned no matter what.

The other way to look at this would be to return all the fields all the time (no more includeOptIn) and let the tooling (GraphiQL, Playground, etc...) use the logic they find appropriate to show them in autocomplete (or hide them)? I feel like that'd be my favorite solution. I never really understood why some tools would set includeDeprecated: false for an example. Is there a place where there is more context about this?

[jturkel]

Could @optIn also apply to type definitions since you might want to expose entirely new types in an experimental feature?

[martinbonnin]

Certainly? I copy/pasted this list from @deprecated because I started with @experimental being the counterpoint of @deprecated but as we're moving away from this I think it makes a lot of sense.

I guess it has implications in terms of validation though. If a type is @optIn("foo"), should all the usages be optIn("foo") too? That can make a lot of directives?

[tomgasson]

I'm not sure about type definitions.

You can query over a union or interface, which could mean you get one of many types. Specifying what a server would have to do if you hadn't opt-ed in based on the runtime value makes this much more complex.
I think having a decidable result based only on schema & operation (and regardless of the value) is desirable.

For the same reason, I'm also not sure about ENUM_VALUE

ARGUMENT_DEFINITION and INPUT_FIELD_DEFINITION make sense from a static validation perspective, but I'm unsure if there's real-world need for them.

At the field level it's often things like "this it's hitting a t1.nano and not ready for 1 million users". Experimental input fields and arguments are more likely about business logic and validation correctness. If they are truely doing something newly experimental they could create a new field instead. I'd love to see some examples of situations where this would be a useful addition

[tomgasson]

I'm having trouble understanding this mechanism. Here's what I gather:

The spec defines a directive named @optIn (and in doing so introduces the need to be able to apply directives to directives)

directive @optIn on DIRECTIVE_DEFINITION

Services create a directive for each distinct opt-in feature they want in their schema

directive @experimentalDeploymentApi on FIELD_DEFINITION @optin

type Query {
 deployment: Deployment @experimentalDeploymentApi
}


enum WorkspaceKind {
 CROSS_PROJECT
 CROSS_COMPANY
}
directive @workspaces(kind: WorkspaceKind) on FIELD_DEFINITION @optin

type Deployment {
 workspaces: [Workspace] @workspaces(kind: CROSS_COMPANY)
}

Is my understanding correct?

• Do clients do something to consume this? Or is it just documenting?

• It's not clear what extra information would be useful to capture. Some examples might help clarify

• This appears to hinder the ability to do out-of-band feature opt-ins (HTTP headers) because you don't have an identifier to use. E.g.

headers: {
  "X-ExperimentalApi": "experimentalDeploymentApi"
}

[martinbonnin]

Is my understanding correct?

100% correct. I took the liberty of reusing your wording as it's much clearer 👌 and pushed it to the proposal, I hope you don't mind?

Do clients do something to consume this? Or is it just documenting?

Simply documenting. See also my other comment.

It's not clear what extra information would be useful to capture. Some examples might help clarify

To me, the main advantage is that tools can get the list of opt-in features by scanning the directive definitions. They don't have to scan each field and infer unique feature names. So it's a bit easier on the tooling side.

As a side effect, it also allows users to add arguments to their directives (your WorkspaceKind above) although at this point, I'm not sure at all there are use cases for this.

This appears to hinder the ability to do out-of-band feature opt-ins (HTTP headers) because you don't have an identifier to use. E.g.

You can always use directive name. Your example would actually work

headers: {
  "X-ExperimentalApi": "experimentalDeploymentApi"
}

[tomgasson]

Alternative proposal: @requiresOptIn and @OptIn

Similar to the Proposed solution, except the schema uses @requiresOptIn instead

directive @requiresOptIn(feature: String!) on FIELD_DEFINITION 

type Session {
  id: ID!
  title: String!
  startInstant: Instant @requiresOptIn(feature: "experimentalInstantApi")
  endInstant: Instant @requiresOptIn(feature: "experimentalInstantApi")
}

Operations use @optIn

directive @optIn(feature: String!) on FIELD 

query {
  session {
    id
    title
    startInstant @optIn(feature: "experimentalInstantApi")
    endInstant @optIn(feature: "experimentalInstantApi")
  }
}

Adding @requiresOptIn to an existing field would need to be detected as a schema breaking change.
Removing @requiresOptIn is safe, and an operation using @optIn on a field without @requiresOptIn would continue to work.
Tools can use introspection values to provide code actions to add @optIn where necessary
Matching the feature prevents misuse, for example:

query {
  session @optIn {
    id @optIn
    title @optIn
    startInstant @optIn
    endInstant @optIn
  }
}

For introspection, unlike the Proposed solution the schema is telling you about the opt-in features, rather than you having to provide them

 type __Type {
   # [...] other fields omitted for clarity
   fields(
     includeDeprecated: Boolean = false,
+    includeRequiresOptIn: Boolean = false
   ): [__Field!]
 }
 
 type __Field {
   name: String!
   # [...] other fields omitted for clarity
   isDeprecated: Boolean!
   deprecationReason: String
+  requiresOptIn: Boolean!
+  optInFeature: String
 }

Pros:

• Self-documenting. There's no set of feature names defined somewhere else
• Usage is colocated with consumption
• No grammar changes
• No need to enumerate all opt in features

Cons:

• Only uses a string, no capability for providing more type information
• Goes further than just being documentation to somewhat imply behaviour (although that could still be for server to decide)
• No place to document a feature itself (i.e one feature might apply to many fields, and want a high-level description of the feature itself)

[martinbonnin]

Alternative proposal: @requiresOptIn

Woops, sorry I missed this earlier today, I actually just pushed @requiresOptIn, I like this a lot, it plays well with @optIn and is more explicit

Operations use @optIn

Do we need to put that in the spec? This has execution implication and different teams might want to implement that differently.

I initially envisioned the proposal as a way to "communicate" opt-in requirements (like @deprecated communicates the deprecated status). "Enforcing" it seems another steps with a wide range of implications.

For introspection, unlike the Proposed solution the schema is telling you about the opt-in features, rather than you having to provide them

Not sure I get this one. The current proposed solution already exposes the opt-in features:

type __Field {
  name: String!
  isDeprecated: Boolean!

  # [...] 

  # list of @requiresOptIn features required to use this field
  requiresOptIn: [String!]

We can use an additional Boolean but it feels like duplication because we can tell if there's the need for duplication if the list is not empty.

On a separate (but related) note, I wish isDeprecated would be replaced by checking for deprecationReason != null.

[tomgasson]

Should these be be repeatable? I'm not sure if that's necessary and it could complicate things

[martinbonnin]

Fair point. Since there are different features I figured out it was possible to require multiple of them and it didn't feel too complex but we can start simple and add repeatable down the road if we want to I guess. Although that circles back to the introspection discussion: if we add repeatable, then we need to model features as a list which might be breaking, etc...

[tomgasson]

I'm not sure about type definitions.

You can query over a union or interface, which could mean you get one of many types. Specifying what a server would have to do if you hadn't opt-ed in based on the runtime value makes this much more complex.
I think having a decidable result based only on schema & operation (and regardless of the value) is desirable.

For the same reason, I'm also not sure about ENUM_VALUE

ARGUMENT_DEFINITION and INPUT_FIELD_DEFINITION make sense from a static validation perspective, but I'm unsure if there's real-world need for them.

At the field level it's often things like "this it's hitting a t1.nano and not ready for 1 million users". Experimental input fields and arguments are more likely about business logic and validation correctness. If they are truely doing something newly experimental they could create a new field instead. I'd love to see some examples of situations where this would be a useful addition

[benjie]

@martinbonnin Ping me when you want me to merge this. In general RFC documents get merged quickly so that others can raise PRs against them. Also please add yourself as the champion near the top of the document 👍

[martinbonnin]

so that others can raise PRs against them

Ah, that makes a lot of sense 👍 . Feel free to merge for me.

@tomgasson I already included some of your feedbacks there (Atlassian prior work and @requiresOptIn). Let me know if you want me to revert these commits so that you can open a PR and get proper attribution.

[benjie]

Since @tomgasson thumbs-up'd your comment I'm taking that as a signal to merge 👍

[LouisCAD]

For the case of Kotlin, I think you mentioned @OptIn in place of @RequiresOptIn, which is the replacement for @Experimental.

[martinbonnin]

@LouisCAD good catch! #1013 fixes it