Make GraphQLResponse generic for e.g. type-safe integration tests

[juergenzimmermann]

I'd recommend to make GraphQLResponse generic so that e.g. integration tests will be type safe, i.e. at https://github.com/apollographql/apollo-server/blob/main/packages/apollo-server-types/src/index.ts#L102:

export interface GraphQLResponse<T = any> {
  data?: Record<string, T> | null;
  ...

Then integration tests with e.g. Axios could be written in TypeScript as follows:

const response: AxiosResponse<GraphQLResponse<MyRecord> = await axios.post(url, requestBody, config);
const myRecord = response.data.data; // Type MyRecord

[glasser]

I think in practice folks will want to use a code generator such as GraphQL Code Generator or (not actively maintained) Apollo Codegen to generate response types rather than relying on GraphQLResponse. GraphQLResponse is more designed for writing a server that has to handle any GraphQL operation rather than use with specific operations.

It's also only sorta typesafe, in the sense that you're just basically asserting at compile time that the data you got back matches MyRecord without any runtime checks, right? So it's not much better than just doing an as X in your code.

It does appear that changing any to unknown in the data and extensions field definitions is a change that typechecks at least in the context of the apollo-server repo. I think that might be a more accurate type.

I also think the change you suggested doesn't quite make sense. Different keys under data will have different types. Did you mean something more like:

export interface GraphQLResponse<Data extends Record<string, unknown> = Record<string, unknown>> {
  data?: Data | null;

[juergenzimmermann]

@glasser Yes, your code fragment is actually what I was looking for.

[glasser]

Hmm. So one problem with this is that all the uses of GraphQLResponse in the ApolloServer API would either still be hardcoded to the default Record<string, any>, or would require further genericization. (And the latter doesn't really work — eg the formatResponse callback needs to work for any operation's data, right?) So eg we'd want to generic-ify ApolloServer.executeOperation (which is used for testing) too.

I think this is a reasonable idea and would be interested in PRs implementing it. Probably we would want to leave the default using any rather than unknown for backwards compatibility reasons (we just passed the 3.0.0 major version...)

[tobiasdiez]

For testing it would be nice if executeOperation would support graphql-typed-document-node, too. Something like

export interface GraphQLResponse<TData = any> {
  data?: TData | null;
  errors?: ReadonlyArray<GraphQLFormattedError>;
  extensions?: Record<string, any>;
  http?: Pick<Response, 'headers'> & Partial<Pick<Mutable<Response>, 'status'>>;
}

async executeOperation<TData = any, TVariables = Record<string, any>>(
    request: Omit<GraphQLRequest, 'query'> & {
      query?: string | DocumentNode | TypedDocumentNode<TData, TVariables>;
    },
    integrationContextArgument?: ContextFunctionParams,
  ): Promise<GraphQLResponse<TData>>

The typed document node is the direction the graphql code generator people are pushing, so supporting this would be awesome.

[glasser]

Yes, I think in general it would be good to move in the direction with aligning the types in Apollo Server (both for operations and resolvers) with things that can be generated by graphql-code-generator.

[mmahalwy]

Any update on this? Feels like this is a regression from apollo-testing-server where a type can be specified?

[glasser]

As mentioned above I'd be interested in PRs that implement this.

[mmahalwy]

@glasser looking for initial 👁️ on this: #5847

[trevor-scheer]

Resolved via #6960