apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.38k stars 2.66k forks source link

MockLink: update the function types returned by `ResultFunction` & `VariableMatcher` to define their own generics #11812

Closed sf-twingate closed 3 months ago

sf-twingate commented 6 months ago

Issue

A feature introduced last year (#6701) which updated MockLink's ResultFunction type and introduced a new VariableMatcher type (both referenced by MockedResponse) has caused a type issue our repository that is affecting our team's common practice of setting a generic which extends from MockedResponse[] on our test setup functions (e.g. function setup<M extends MockedResponse[]>).

This issue arises due to both types accepting a variables generic type (V), and then returning a function type which uses this generic as its own argument type:

export type ResultFunction<T, V = Record<string, any>> = (variables: V) => T;
export type VariableMatcher<V = Record<string, any>> = (variables: V) => boolean;

When these types are combined with our usage of M extends MockedResponse[], TypeScript is unable to correctly infer the variables argument type for each of of the values in M.

Example Failure

Here's an isolated example of this which will currently fail type checking:

Failing Example: ```tsx import { FetchResult, GraphQLRequest, gql } from "@apollo/client"; // The `ResultFunction`, `VariableMatcher` & `MockedResponse` types below are the current types // defined in `mockLink.ts`. type ResultFunction> = (variables: V) => T; type VariableMatcher> = (variables: V) => boolean; interface MockedResponse< TData = Record, TVariables = Record, > { request: GraphQLRequest; maxUsageCount?: number; result?: FetchResult | ResultFunction, TVariables>; error?: Error; delay?: number; variableMatcher?: VariableMatcher; newData?: ResultFunction, TVariables>; } // ---- test case below function setup(props: { mocks: M }) { const { mocks } = props; return { mocks, }; } // Type '[MockedResponse<{ a: string; }, { foo: string; }>]' does not satisfy the constraint 'MockedResponse, Record>[]'. // Type 'MockedResponse<{ a: string; }, { foo: string; }>' is not assignable to type 'MockedResponse, Record>'. // Types of property 'result' are incompatible. // Type 'FetchResult<{ a: string; }> | ResultFunction, { foo: string; }> | undefined' is not assignable to type 'FetchResult> | ResultFunction>, Record> | undefined'. // Type 'ResultFunction, { foo: string; }>' is not assignable to type 'FetchResult> | ResultFunction>, Record> | undefined'. // Type 'ResultFunction, { foo: string; }>' is not assignable to type 'ResultFunction>, Record>'. // Type 'Record' is not assignable to type '{ foo: string; }'. ts(2344) setup<[MockedResponse<{ a: string }, { foo: string }>]>({ mocks: [ { request: { query: gql` query A { a } `, variables: { foo: "bar", }, }, }, ], }); ```

Proposed Fix

I believe this can be fixed, without causing any breaking changes, by making the function types returned by ResultFunction & VariableMatcher declare their own generic type, which extends from and defaults to the V type.

This resolves the issue in the snippet above:

Example Fix: ```tsx import { FetchResult, GraphQLRequest, gql } from "@apollo/client"; // The `ResultFunction` & `VariableMatcher` types below have been updated to declare their own generics. type ResultFunction> = <_V extends V = V>( variables: _V ) => T; type VariableMatcher> = <_V extends V = V>( variables: _V ) => boolean; // This is still the existing `MockedResponse` type from `mockLink.ts`. export interface MockedResponse< TData = Record, TVariables = Record, > { request: GraphQLRequest; maxUsageCount?: number; result?: FetchResult | ResultFunction, TVariables>; error?: Error; delay?: number; variableMatcher?: VariableMatcher; newData?: ResultFunction, TVariables>; } // ---- test case below function setup(props: { mocks: M }) { const { mocks } = props; return { mocks, }; } // No type error anymore. setup<[MockedResponse<{ a: string }, { foo: string }>]>({ mocks: [ { request: { query: gql` query A { a } `, variables: { foo: "bar", }, }, }, ], }); // A type error is still _correctly_ thrown below due to the `variables` type mismatch. setup<[MockedResponse<{ a: string }, { foo: string }>]>({ mocks: [ { request: { query: gql` query A { a } `, variables: { // Object literal may only specify known properties, and 'not' does not exist in type '{ foo: string; }'. ts(2353) // types.d.ts(36, 5): The expected type comes from property 'variables' which is declared here on type 'GraphQLRequest<{ foo: string; }>' not: "found", }, }, }, ], }); ```

I've implemented the change as part of this PR and added a test case which currently fails when run against main, but passes on this branch.

Please let me know if you have any questions / concerns about the changes — thanks!

netlify[bot] commented 6 months ago

Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
Latest commit 24660b8e1cb77282c262a6cacc16518ee8294b87
changeset-bot[bot] commented 6 months ago

🦋 Changeset detected

Latest commit: 24660b8e1cb77282c262a6cacc16518ee8294b87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | -------------- | ----- | | @apollo/client | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jerelmiller commented 6 months ago

Hey @sf-twingate 👋

We'll do our best to get to this PR soon. Thanks so much for the contribution!

phryneas commented 4 months ago

I believe this might have been solved in a different way in #11848, could you please give a version >=3.10.5 a try and report back?

jerelmiller commented 3 months ago

I believe this was already solved with #11848 so we'll go ahead and close this. Thanks for the contribution!