ardatan / ardatan/graphql-tools

Improve handling of selections that would only consist of `__typename`

Open
#6,456 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
5.4k
Forks
830
Avg merge
10h 59m
Merged PRs (30d)
45

Description

**Is your feature request related to a problem? Please describe.**

In 33e8146e33aa17790ee76d14e52f62c684ee1b16, you implemented a change that aborted the query with an error if it would habe `__typename` in its selection set. That is good, as it finally allows us to handle a long-standing issue with our setup, but we would suggest to further refine the behavior.

In our project, we use a custom solution based on `stitchSchemas` and especially `delegateToSchema` to implement a strangler pattern where we want to gradually replace an old service A with a new service B behind a common GraphQL gateway.
Here, the schema of B is mostly a subset of the schema of A. If `stitchSchemas` determines that a top-level field is supported by both services, then we use `delegateToSchema` to send the request to both services and then implement a custom merge logic to merge the results.

There is one problem though. Assume we have these schemas:

```graphql
# Service A
type Query {
foo: Foo
}

type Foo {
first: String
second: String
}

# Service B
type Query {
foo: Foo
}

type Foo {
first: String
}
```

And this query:

```graphql
query {
foo {
__typename
second
}
}
```

`stichSchemas` will determine that both services implement `Query.foo` and we will therefore pass the query to `delegateToSchema`, which in turn will find out that we want to send this query to B:

```graphql
query {
foo {
__typename
}
}
```

With the old logic, this generated a lot of useless requests while the new logic now throws an error that we can catch and handle! (Technically, this also means that this patch release was a breaking change for us, but we do not mind, we pinned the dependency for now)

However, catching this error means we need to rely on the exact wording of the error message as we still want to handle other errors as before. Also, using errors as expected control flow like this probably also carries some performance penalty (though we did not test this).

**Describe the solution you'd like**

We want a clean way to ignore requests that would only select `__typename` of any field. Ideally, these fields would return `null`. It would be ok for this behavior to be opt-in via option. Alternatively, there could be a hook to handle these cases.

**Describe alternatives you've considered**

If we want to stick with throwing errors, it would also help to add a stable error code to the error that we can test for.

**Additional context**

See above. If you think our approach could be improved in some way that would avoid this problem differently, let us know!

Contributor guide

Open the contributing guide

Research direction

Start by reading the existing handling in stitchSchemas and delegateToSchema, including the change from commit 33e8146e33aa17790ee76d14e52f62c684ee1b16. Reproduce the example where delegation leaves only __typename in the selection, then determine how an opt-in null result, hook, or stable error code should behave without relying on error-message wording.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.