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

Throw GraphQL Schema error when federated schemas are not extend each other properly #830

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JamesLMilner
Copy link

Aims to resolve #826

This is currently an experimental draft PR as I get an understanding of what the desired behaviours are for merging federated schema types. The overall idea is that if a schema comes together and non value types do not correctly extend each other within that, then it should throw a GraphQL schema validation error.

I'm pretty certain the current code doesn't do exhaustive checks on ensuring entity types are identical when they haven't been extended - but hopefully at least it gives some ideas on how that might work. I know in another part of the code base for value types there is also a TODO: for ensuring equality between value types.

One other aspect I am not totally sure on is if the order of extension matters, for example if the schema extends a type before it has been defined, but it is defined later in the schema.

url: `http://localhost:${conflictingServicePort}/graphql`
}]
}
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the federation may not start correctly if the ready/listen method is not called because there is an onReady hook that is not executed

Suggested change
})
})
await gateway.ready()

await conflictingService.close()
})

t.rejects(() => gateway.register(GQL, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we check the error message?

Here an example from apollo:

This data graph is missing a valid configuration. [accounts] Order.name -> Field "Order.name" already exists in the schema. It cannot also be defined in this type extension. If this is meant to be an external field, add the `@external` directive.

})
})

test('Types with duplicate names that are value types should not throw', async (t) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this test, Apollo throws:

This data graph is missing a valid configuration. [accounts] Order -> Value types cannot be entities (using the `@key` directive). Please ensure that the `Order` type is extended properly or remove the `@key` directive if this is not an entity.

If I remove the @key all works

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

Successfully merging this pull request may close these issues.

federation gateway should throw on type's conflicts
3 participants