Skip to content

Allow fields to diverge more? #820

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

Open
andimarek opened this issue Feb 4, 2021 · 2 comments
Open

Allow fields to diverge more? #820

andimarek opened this issue Feb 4, 2021 · 2 comments

Comments

@andimarek
Copy link
Contributor

The current spec rule for overlapping rules allows the following:

Query:

  {pets {
        ... on Cat {
          friend: catFriend {
            catFriendName
      }}
        ... on Dog {
          friend: dogFriend {
            dogFriendName
      }}
  }}

Schema:

type Query {
  pets: Pet
}
interface Pet {
  name: String
}
type Cat implements Pet {
    name: String
    catValue: Int
    catFriend: CatFriend
}
type CatFriend {
  catFriendName: String
}
type Dog implements Pet {
     name: String
     dogValue: Float
     dogFriend: DogFriend
}
type DogFriend {
   dogFriendName: String
}

This query is allowed because friend can't never be executed at the same time because the fragment types are both different Object types and and execution time only one Fragment can be therefore valid.

The friend results will have two different "shapes" depending on if it is a Cat or Dog: One has dogFriendName, the other catFriendName.

But the following query is not allowed (same schema as above):

  {pets {
        ... on Cat {
          value: catValue
      }
        ... on Dog {
          value: dogValue
      }
  }}

The reason for that is that catValue is Int but dogValue is Float and the current rule doesn't allow it.

Different object shapes are already allowed: should the spec allow for more diverging?

The suggestion is to never compare the different result types for fields which can never executed at the same time and therefore allow the second query above.

I would also be interested in the historical context: the current rule was modified in this PR #120. @leebyron do you have maybe an explanation reasoning for the current rule?

@mjmahone
Copy link
Contributor

mjmahone commented Mar 4, 2021

{
  pets {
    ... on Cat {
      value: catValue
    }
    ... on Dog {
      value: dogValue
    }
  }
}

For the above query, this has different return types in the same response shape.

Both of these responses would need to be valid:

{
  "data": {
    "pets": {
      "value": 123
    }
  }
}
{
  "data": {
    "pets": {
      "value": 123.456
    }
  }
}

I believe the logic here is that the response shape needs to be unambiguous. If you have a generated parser for this query, what type should the parser use for value? Basically, we need a way to unambiguously parse responses into a "Response Shape" without depending on meta fields like __typename to disambiguate for us.

To be fair, this is only a problem if we support "Response Shape" parsing. In pseudo-Java, the above would need to produce something like:

interface Query {
  interface Pets {
    // Is this Integer or Float?
    ?? value();
  }
  Pet pets();
}

This is probably not a problem if you parse into either Type Models or Fragment Models, but those require adding meta-fields like __typename to work correctly. Response Models still need to be supported, unfortunately.

@andimarek
Copy link
Contributor Author

andimarek commented Mar 4, 2021

thanks @mjmahone for highlighting the need to parse the response unambiguously. This was the missing point to rationale the current logic.
To make it more specific: the first example above:

  {pets {
        ... on Cat {
          friend: catFriend {
            catFriendName
      }}
        ... on Dog {
          friend: dogFriend {
            dogFriendName
      }}
  }}

can be represented by one model where catFriendName and dogFriendName is nullable. This is why it is allowed, event if the shape is different per Pet.

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