apollographql / apollographql/federation

(Federation) Proposal: Require that selection sets in @requires directives only use fields that are explicitly defined in the schema

Open
#402 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
725
Forks
276
Avg merge
1h 47m
Merged PRs (30d)
1

Description

During gateway composition and validation, we don't currently enforce that all fields found in a `@requires` selection set are defined in the schema, which allows us to write schemas that look like this:
```graphql
extend type Test @key(fields: "id") {
id: String! @external
item: Item @external
location: String @requires(fields: "item { name owner { firstname lastname } }")
}

extend type Item {
name: String! @external
}
```
Here the `@requires` directive assumes that the `item` type will have an `owner` field, but this field nor its type are explicitly defined in the schema (even though they may be defined in the base schema where the `Test` base type is defined). Though this isn't a problem in and of itself, it can lead to bad practices when writing resolvers, since the type of `item.owner` is implicitly assumed in the resolver and cannot be enforced by static type checking. It also means that we would have no way of catching when the `item.owner` type changes in the base schema and becomes out of sync with what the `Test.location` resolver assumes its type is.

These problems can be avoided by following best practices, namely by requiring that selection sets in @requires directives **only** use fields that are explicitly defined in the schema, but this is subtle and hard to follow on one's own. I think it'd be much better if we made this a requirement in the federation spec and implemented validation for it, so that we are forced to follow this best practice. That way, every parent type passed to any resolver will always be explicitly defined, making the schema self-contained.

This requirement will force us to write more boilerplate, but it comes with the benefit of guaranteed type safety. We'll always know when a resolver's parent type becomes out of sync with the base schema type, since the gateway will see that the types between these two schemas are inconsistent and fail to start up. We'll no longer be able to work with implicitly unknown types in resolvers which can be the source of major headaches in production, especially when coordinating between multiple services that often change independently.

For this added guarantee of type safety, I think we should make it an explicit requirement in the federation spec itself as opposed to merely suggesting it as a best practice. If we all agree then I can start working on adding it to the spec and then begin looking at implementing validation for it. Let me know what you think!

Contributor guide

Open the contributing guide

Research direction

No files, tests, or entry points are named. Start by reviewing the federation specification and gateway composition-validation code, then define the validation behavior and tests needed to reject @requires selections that reference fields not explicitly defined in the schema.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, typescript
Domain
api, backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.