Skip to content

[Pre-RFC] Support gradual upgrade/rename of Input types and Scalars in field arguments for values with compatible wire format #526

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

Closed
kestred opened this issue Oct 26, 2018 · 5 comments

Comments

@kestred
Copy link

kestred commented Oct 26, 2018

I'm hoping to discuss solutions for the use case of upgrading field argument input type / scalars from less-strict to more strict types; consider:

# Current graph api
type Query { person(id: string): Person }
type Mutation {
    addPerson(person: PersonArgs)
    setPersonType(id: String, type: String)
}

# Desired eventual graph api (after deprecation lifecycle)
type Query { person(id: PersonId): Person }  # compatible Scalar
type Mutation {
    addPerson(person: NewPerson) # compatible InputObject
    setPersonType(id: PersonId, type: PersonType) # multiple compatible Scalars
}  

Where there isn't a reasonable way to make this change over time, because any change to the graph will break queries that use variables:

# This id variable triggers a validation error, even though the wire format is the same
query GetPerson($id: String) {  
    person(id: $id)
}

Motivation

This feels like (and certainly is for me) a very common use case where, you may start with a less specific type (frequently a String scalar) and would like to upgrade to a well-typed value over time (e.g. a BigDecimal, a DateTime, an enum, etc).

Similarly, you may want to change the name of an InputObject, even if the structure of the argument hasn't changed to be able to provide a consistent and predictable API to consumers as business needs grow and change (separately, you'd of course like to be able to deprecate fields on InputObjects).

I feel this is an especially frequent issue for rapidly prototyped graph APIs, that you'd like to harden and polish after they've already been deployed into a production environment.

The current way I'd make this change is an extraordinarily painful process both for the provider and consumers of an API:

  1. Add a new field with the new arguments (or old ones; doesn't really matter) and
    deprecate the previous field:

    type Query {
        person(id: String): Person @deprecated
    
        "A temporary field; will be renamed back to 'Person'"
        personByPersonId(id: PersonId)
    }
  2. Wait for all consumers to switch off the old field.

  3. Change the original field and de-deprecate it, deprecating the temporary field

    type Query {
        person(id: PersonId): Person
        personByPersonId(id: PersonId): Person @deprecated
    }
  4. Eventually remove the temporary, now deprecated, field.

Hopefully, an RFC that make this problem easier to solve reduces the amount of friction involved in gradual enhancement of a graph schema without being forced to choose between having a well-typed schema vs. having a predictable schema.

Ideas/Suggestions

I will add ideas from the thread here as they are discussed (Last Edited: Oct 25, 2018)

A. Overloading fields

The spec could allow fields to be overridden; the executor would choose which variant to resolve through a combination of the name and number of arguments and the type of input variables.

There would be ambiguity when NOT using variables, which could be resolved by:

  1. Always choosing the first overload
  2. Requiring that a default overload by specified by an annotation (e.g. @defaultOverload)

The @deprecated attribute could be applied as normal to any of the overloads

type Query {
    field person(id: String): Person @deprecated
    field person(id: PersonId): Person
}

B. Anonymous Input Unions (aka. sum types) w/ deprecatable variants

This is similar to many of the InputUnion suggestions, with the key difference being in how
variables are validated. With an anonymous input union, a variable should be specified
using any of the variant names, rather than the a new InputUnion name.

type Query {
    field person(id: String @deprecated | PersonId): Person
}

This suffers from all of the same variant discrimination problems as previous work on input unions:
see #395, #488,
and #202.

C. Less strict type inference of variables using ofType

We could allow scalar variables types to be valid if the wire representation of the scalar is passed as the type of the variable instead of the more strict type; relying on something like #521.

For example,

scalar PersonId(String)   # NOTE: not actual syntax, but use whatever machinery is provided in PR-521.

type Query {
    field person(id: PersonId): Person
}

# This now validates successfully.
query GetPerson($id: String) {  
    person(id: $id)
}

Unfortunately, by only relying on that change, this only converting builtin scalars to well typed scalars (but not vice-versa) and doesn't provide a solution for renaming InputObjects.

D. Allowing explicit type coercions

We could come up with a way to annotate that type coercion between any two types is supported.
This is somewhat of an extension of #521.

@convert(from String)
scalar PersonId

input Point2D {
  x: Float
  y: Float 
}

@convert(from Point2D)
input Point3D {
  x: Float
  y: Float
  z: Float
}

Currently this idea is my favorite (syntax and details to be worked out).

@kestred kestred changed the title [Pre-RFC] Supporting gradual "upgrade" or "rename" of InputTypes and Scalars in GraphQL APIs for values with compatible wire format [Pre-RFC] Support gradual upgrade/rename of Input types and Scalars in field arguments for values with compatible wire format Oct 26, 2018
@IvanGoncharov
Copy link
Member

@kestred Sorry for the delay.

Variant D is currently discussed as #509 and is currently Stage 1 proposal.
Note: it initially started as a proposal to allow migration from a scalar to array but latter was expanded in scope to include your use case.

Also, note that in this proposal you don't need @convert since custom scalars can consume any value.
Note: RFC #521 allows you to restrict possible values of scalars by defining them as scalar PersonId as String.

Do you want this issue to stay open or combination of #509 + #521 will solve your problem?

@kestred
Copy link
Author

kestred commented Jan 20, 2019

@IvanGoncharov - they look mostly on point; only lingering questions for clarity.

The key in both cases for me is how input arguments are validated against where
they are later used in the query.

Rather than how the value itself it is validated. The assertion being that in these examples
no actual "coercion" of the value needs to occur from a "json-like" perspective, but that
the validator needs to accept that the Input type can be cast into the argument type without error.

wrt. #509

Suppose some schema:

input Point2D {
  x: Float
  y: Float 
}

input Point3D {
  x: Float
  y: Float
  z: Float
}

type Mutation {
    addPoint(point: Point3D)
}

Would this query execute without errors given how #509 is currently proposed?

mutation ($myPoint: Point2D) {
    addPoint(point: $myPoint)
}

wrt. #521

Suppose some schema:

scalar PersonId ofType String;
type Person { .. }
type Query { person(id: PersonId!): Person } 

Would this query execute without errors given how #521 is currently proposed?

query ($id: String!) {
    person(id: $id)
}

@IvanGoncharov
Copy link
Member

Would this query execute without errors given how #509 is currently proposed?

@kestred Good question, I'm not sure since we were focused only on scalar to list coercion and coercion between different scalar types.
Personally, I think it's impossible to support such implicit conversion since any two input objects with only optional fields would always be compatible.

Would this query execute without errors given how #521 is currently proposed?

With only #521 accepted it would produce errors. But with #521 + #509 it would valid query.
Note that String => PersonId would be legal but not the PersonId => String.

@kestred
Copy link
Author

kestred commented Jan 22, 2019

With only #521 accepted it would produce errors. But with #521 + #509 it would valid query.
Note that String => PersonId would be legal but not the PersonId => String.

For scalars, this behavior seems great!

Personally, I think it's impossible to support such implicit conversion since any two input objects with > only optional fields would always be compatible.

Yeah, I agree that implicit conversion is sketchy here. In the ideas proposed above there were a variety of explicit mechanisms, however after this conversation I think a different mechanism for handling graceful upgrade might be more suitable (assuming #521 and #509 for scalars).

Suppose we had some schema:

input XY {
  x: Float
  y: Float 
}

type Mutation {
    addPoint(point: XY)
}

I'd like to have a way to change the name of XY, without breaking existing users.
We could allow a server to specify type/input aliases, such that an "updated" schema could be:

input Point {
  x: Float
  y: Float
  z: Float
}

input XY = Point   # or `input XY: Point`? `alias XY: Point`?

type Mutation {
    addPoint(point: Point)
}

Overall, I think typeof is a substantial improvement to scalars, but it would be nice to have some upgrade path that exists for composite types.

@IvanGoncharov
Copy link
Member

Overall, I think typeof is a substantial improvement to scalars, but it would be nice to have some upgrade path that exists for composite types.

@kestred In general, we need to balance features and added complexity.
Lee gave a good overview of this topic in his talk: https://youtu.be/mePT9MNTM98?t=1175

Not sure if it fully covers your scenario but #525 allows you to @deprecate args and input fields. This is a universal mechanism that will allow to first deprecate field and then remove it after all clients updated.

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