graphql / composite-schemas-spec

MIT License
29 stars 9 forks source link

Allow marking a key as deprecated #51

Open smyrick opened 2 months ago

smyrick commented 2 months ago

Duplicate of https://github.com/apollographql/federation/issues/2747

I want to be able to indicate to other subgraph authors that they should stop using a particular key to reference an entity and that there is another one everyone should use instead.

type Foo @key(fields: "x y") @key(fields: "x", deprecated: "Use the new x+y key to reference this now") {
  x: ID!
  y: ID!
  someField: String
}

This is not the same as marking the existing key field as deprecated, it is marking the use of the @key directive with those particular fields as deprecated


See the discussion in https://github.com/apollographql/federation/issues/2695

Current I have two subgraphs and they use Foo.x to identify the key

// Subgraph 1
type Foo @key(fields: "x") {
  x: ID!
  someField: String
}

// Subgraph 2
type Foo @key(fields: "x") {
  x: ID!
  otherField: String
}

Some time in the future, I now want to migrate from using just Foo.x as the id to Foo.x AND Foo.y. However current subgraphs are still using only Foo.x as the reference. Well we can't break those subgraphs so we need to add the new key to one subgraph that understands that

This is still valid

// Subgraph 1
type Foo @key(fields: "x") @key(fields: "x y") {
  x: ID!
  y: ID!
  someField: String
}

// Subgraph 2
type Foo @key(fields: "x") {
  x: ID!
  otherField: String
}

We want some way in the tooling that you can indicate to all subgraphs that they need update their key references from @key(fields: "x") to @key(fields: "x y"). If all subgraphs do that then we can successfully remove @key(fields: "x") entirely and only use the new x + y id.

This is also still valid

// Subgraph 1
type Foo @key(fields: "x") @key(fields: "x y") {
  x: ID!
  y: ID!
  someField: String
}

// Subgraph 2
type Foo @key(fields: "x y") {
  x: ID!
  y: ID!
  otherField: String
}

Today, that would have to be solved with communication, but I like the idea of adding a optional @key.deprecated flag so we could inform other subgraph authors with linter rules

Eventual state we want

// Subgraph 1
type Foo @key(fields: "x y") {
  x: ID!
  y: ID!
  someField: String
}

// Subgraph 2
type Foo @key(fields: "x y") {
  x: ID!
  y: ID!
  otherField: String
}
glen-84 commented 1 month ago

I would expect a scalar deprecated argument to be a boolean (or perhaps a DateTime). Maybe something like:

@key(fields: "x", deprecated: { reason: "Use the new x+y key to reference this now" })

This would also allow for later expansion:

@key(
    fields: "x",
    deprecated: {
        date: "...",
        reason: "Use the new x+y key to reference this now",
        documentationUrl: "https://example.com/"        
    }
)

And aligns better with the @deprecated directive.

smyrick commented 1 month ago

Yea, I had just proposed the string because that was the current ability of the GraphQL @deprecated

https://spec.graphql.org/draft/#sec--deprecated

But even if there is only 1 arg today, that spec has the option to add new optional args in the future.

So maybe just to start @key could be updated to this

directive @key(fields: FieldSet!, resolvable: Boolean = true, deprecated: DeprecatedInput = null) repeatable on OBJECT | INTERFACE

input DeprecatedInput {
  reason: String = ""
}

As for dates and documentation URLs, those should just be put in the reason string