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

Allow marking a key as deprecated #51

Open
smyrick opened this issue Sep 11, 2024 · 2 comments
Open

Allow marking a key as deprecated #51

smyrick opened this issue Sep 11, 2024 · 2 comments

Comments

@smyrick
Copy link

smyrick commented Sep 11, 2024

Duplicate of apollographql/federation#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 apollographql/federation#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
Copy link
Contributor

glen-84 commented Sep 14, 2024

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
Copy link
Author

smyrick commented Sep 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants