Skip to content

Proposal: Interface implementations should satisfy interface fields #294

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
mike-marcacci opened this issue Apr 3, 2017 · 5 comments
Closed

Comments

@mike-marcacci
Copy link
Contributor

mike-marcacci commented Apr 3, 2017

I have moved this here from graphql/graphql-js#776.


After opening graphql/graphql-js#720 some time ago, I refactored my schema to work with the current state of GraphQL interfaces. However, this continues to gnaw at me as a real missed opportunity, particularly in allowing general-purpose tools like Relay's Cursor Connection Specification to express their requirements in GraphQL.

Let's imagine the following currently-invalid schema:

interface Pet {
	id: !ID
	sex: Sex
	name: String
	mother: Pet
	father: Pet
}

type Dog implements Pet {
	id: !ID
	sex: Sex
	name: String
	mother: Dog
	father: Dog
	isVaccinated: Boolean
	hasDisplasia: Boolean
}

type Bird implements Pet {
	id: !ID
	sex: Sex
	name: String
	mother: Bird
	father: Bird
	hasTrimmedWings: Boolean
}

Intuitively, this makes sense: both Dog and Bird implement Pet. However, the mother and father of a Dog are each a Dog and not a generic Pet. This should be possible, but currently isn't.

If this were possible we could do some really cool things. We can continue to query the interface as if each pet's mother was a Pet (and not a Dog or Bird):

pets {
	id
	sex
	name
	mother {
		id
		name
	}
	father {
		id
		name
	}
}

We can also continue to use inline fragments to switch between the types:

pets {
	id
	sex
	name
	mother {
		id
		name
	}
	father {
		id
		name
	}

	... on Dog {
		isVaccinated
		hasDisplasia
	}

	... on Bird {
		hasTrimmedWings
	}
}

But it adds much more convenient and semantically clear querying of a single pet type. We can, for example, directly ask if a dog's parents have displasia, without using an inline fragment to switch on the obvious:

dog: {
	id
	name
	mother {
		id
		name
		hasDisplasia
	}
	father {
		id
		name
		hasDisplasia
	}
}

Now, of course this example is a bit oversimple, and the bennefits are marginal and mostly just erganomic. For tooling such as the Relay spec, however, this could be extremely useful.

If this has previously been considered and decided against, I would love to hear some thoughts on why, if just to scratch my itch. If it hasn't yet been considered (or has been filed away under "maybe later") I would love to explore what an implementation might look like!


I opened this issue together with #295, as they each describe a distinct expansion of the GraphQL language. However, both are necessary to create the "higher order interfaces" that open up GraphQL's type system to general purpose tools.

@calebmer
Copy link

calebmer commented Apr 3, 2017

This seems like a natural thing to allow. Do you want to open a PR against the specification, that might be the easiest thing to discuss. I don’t imagine it would be too intrusive written correctly 😊

One thing to think about is if we should give a similar treatment to unions. I imagine the best structure for may be to write some general “supertype” validation.

@stubailo
Copy link
Contributor

stubailo commented Apr 4, 2017

I don’t imagine it would be too intrusive written correctly

This would require some pretty significant changes to GraphQL implementations though, right?

@mike-marcacci
Copy link
Contributor Author

I was just looking through the spec with an eye out for lines I may need to change as part of a PR, and I came across the Object type validation rules, which seem to already describe my proposal here:

An object field type is a valid sub-type if it is an Object type and the interface field type is either an Interface type or a Union type and the object field type is a possible type of the interface field type.

It appears that @leebyron defined this behavior in 2015 in this commit with the following message:

This proposes loosening the definition of implementing an interface by allowing an implementing field to return a subtype of the interface field's return type.

This example would previously be an illegal schema, but become legal after this diff:

interface Friendly {
  bestFriend: Friendly
}

type Person implements Friendly {
  bestFriend: Person
}

Furthermore, this is the wording that appears on Facebook's official GraphQL specification.

So am I correct to assume that this is a bug in graphql-js (and likely other implementations)?

@mike-marcacci
Copy link
Contributor Author

Nope... not a bug, as the test is working as expected.

I'll go back and see what other factors may have made this error appear for me in graphql-js. I'm a bit embarrassed I spent so much time writing a proposal for a feature that already exists... but also glad I'm not the first one trying to use the type system this way!

Sorry for the noise!

@wincent
Copy link
Contributor

wincent commented Apr 7, 2017

Sorry for the noise!

Don't be sorry for the noise. We appreciate the attention to detail and the intent.

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

4 participants