Make GraphQL handlers endpoint-first

[mattcosta7]

Description

This is an RFC for a breaking change to handling graphql requests, where msw would drop the raw graphql handlers graphql.query graphql.mutation and graphql.operation in favor of forcing graphql parsing to first call graphql.link and define handlers using the scoped methods returned from it.

Background and reasoning

The two ways we can mock graphql requests today are shown below

import {graphql} from 'msw'

// define a specific endpoint to intercept
const specificGraphqlEndpoint = graphql.link('https://some.example.com/graphql')

const handlers = [
   // intercept all requests that contain a GetUser query
   graphql.query('GetUser'),
   // intercept  all requests to specific endpoint that contain a GetUser query
   specificGraphqlEndpoint.query('GetUser'),
]

Note, there's a subtle bug here, because all requests that contain a GetUser query parsable as a graphql query will trigger this handler, so the more specific handler can never be matched. In this simple example it's easy to spot his error, but in a more real world scenario, it's much less clear why this might happen

const worker = setupWorker(...) // or setupServer, either one

worker.use( 
  graphql.query('GetUser')
)

Here, i'll now always intercept the global GetUser, even if it maybe shouldn't because a more specific handler is implemented elsewhere.

By deprecating the raw graphql handler methods, and forcing calls to graphql.link first, we'll bind http requests to the specific endpoint the graphql server is located on

Pros/Cons

Pros:

• Some simplification in msw codebase, and allow us to more strictly handle failed graphql parsing ¹
• MSW no longer needs to parse requests to endpoints that can't possibly match²
• Users can uncover accidental typos in their codebase³

  fetch('https://example.com', {method: 'post', body: { query: 'query GetUser { id }' }
  fetch('https://exmple.com', {method: 'post', body: { query: 'query GetUser { id }' }

Today, the raw handlers would not help a user uncover this, leading to passing tests and runtime exceptions

Cons:

• Users need to add an additional bit of configuration before intercepting calls to graphql

Anticipated rollout plan

• Immediately deprecate the raw handlers in code and in docs, warning users that they should first call graphql.link(endpoint) and use the scoped handlers returned from that to define their handlers instead of the raw ones.
• Provide a description of this rollout plan in the deprecation messages, helping users to transition
• In version 3.x.x remove the raw apis entirely

Footnotes

1. Particularly on the parse issue, we had to move to logging the errors instead, which don't result in a 500 from the mock, even though it probably should. We had to do this to avoid returning 500s for cases where non-graphql requests were parsed as graphql requests - https://github.com/mswjs/msw/pull/740 ↩

2. https://github.com/mswjs/msw/pull/1871 ↩

3. Most users don't handle graphql calls direction, as frameworks extract it, but they do still need to configure an endpoint, somewhere, that requests are made to ↩

[kettanaito]

+1 for this change is the fact that graphql.subscription() would require you to provide an endpoint because it would be implemented on top of ws.link(), which requires you to provide an endpoint. I think moving the GraphQL interception to be endpoint-based is a good change.