Fix bug with type extension of empty type definition #1821
Conversation
👷 Deploy request for apollo-federation-docs pending review.Visit the deploys page to approve it
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| @@ -878,6 +878,30 @@ describe('@core/@link handling', () => { | |||
| // Just making sure this don't error out. | |||
| buildAndValidate(doc); | |||
| }); | |||
|
|
|||
| it('allows defining a repeatable directive as non-repeatable but validates usages', () => { | |||
There was a problem hiding this comment.
Reviewer notes: this test is unrelated to the issue and it wasn't meant to be here, but I just wrote it earlier (to prove to myself that the behaviour of the test does work as the test shows) and didn't noticed it was there. But seeing it now, I'm thinking: "it's a nice test to have and is it really worth opening a separate PR for this?". Happy to remove though if it bothers someone.
| type = schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value)); | ||
| } else if (type.preserveEmptyDefinition) { | ||
| // Note: we reuse the same error message than graphQL-js would output | ||
| throw new GraphQLError(`There can be only one type named "${definitionNode.name.value}"`); |
There was a problem hiding this comment.
Reviewer note: writing this, I realised that we weren't validating this properly. That is:
type Foo {
bar: Int
}
type Foo {
baz: Int
}was not erroring out, even though it should as it's not valid graphQL. So took the liberty to fix as well (and there is a test for it).
pcmanus
force-pushed
the
type-extension-bug
branch
from
f67a0a7
to
77ea570
Compare
It is valid graphQL to do:
and while that's uncommon, this could be the convenient when mechanically concatenating files.
This wasn't handled correctly however as the code was not preserving the empty type definition and was thus considering the schema as just:
which, when trying to compose this, may lead into an error complaining that the type is extended but has no definition (which is obviously incorrect).