Resolving promise in RequestHandler vs in MockLink

[baleeds]

Have you considered adding Promise.resolve() to the MockLink instead of having to resolve in each RequestHandler?

My nitpick about resolving in the handler is that typescript is not inferring that the return of my resolve() is the type provided to TData in HandleResponse, so I have to provide the TData argument to both places, which looks like this:

const handler: RequestHandler<DogQuery, DogQueryVariables> = () =>
    Promise.resolve<DogQuery>({ data: { dog: { id: 1, name: 'Rufus', breed: 'Poodle' } } }))

If the link resolved the handler:

const handler: RequestHandler<DogQuery, DogQueryVariables> = () =>
    ({ data: { dog: { id: 1, name: 'Rufus', breed: 'Poodle' } } }))

[Mike-Gibson]

I tried your first example without the second DogQuery type , and TypeScript seemed to infer the types correctly:

const handler: RequestHandler<DogQuery, DogQueryVariables> = () =>
  Promise.resolve({ data: { dog: { id: 1, name: 'Rufus', breed: 'Poodle' } } });

This seemed to work in TypeScript 3.5 (version in project) and 3.8 (latest in VS Code). What version are you using?

It's a reasonable suggestion to allow a non-promise to be passed in to the handler. I've found that when I'm testing components which are loading data, I've always wanted to test loading states, so I've always needed to control when the promise is resolved (or rejected). If you're finding you don't need to control the promise, I don't think it would be too hard to change the library to allow that. If you've got time, it would be great if you could have a go at implementing it? Happy to help if you'd like.

[baleeds]

Yeah, maybe I used the wrong wording. The issue I had was actually that VS Code wouldn't suggest fields in the resolved object, even though TS knew that I was returning that promise. I think I would've gotten a compilation error.

I see your point about loading states. It is nice to have that power in the resolvers! I figured you had solid reasoning but felt I should suggest it.

I have made that change, it's basically just one line in the link. But its not clearly better for every scenario, and would be a breaking change, so it's probably not a good idea to change the library.

[Mike-Gibson]

I tried in the mock apollo client repo and intellisense for the example seemed to work:

I have experienced VS Code and the TypeScript language server sometimes getting confused (and giving up) in large code bases and also when there are lots of generics involved though. So could be project-specific.

As for the change - I think the library could be extended to support both cases - so:

RequestHandler<TData = any, TVariables = any> =
  (variables: TVariables) =>
    Promise<RequestHandlerResponse<TData>>;

becomes

RequestHandler<TData = any, TVariables = any> =
  (variables: TVariables) =>
    (Promise<RequestHandlerResponse<TData>> | RequestHandlerResponse<TData>);

Then the mock link could look at the returned object and if it's not a promise-like object, could wrap the result in a resolved promise.

Maybe we can see if anyone else has a similar suggestion/requirement?

Thanks for your input.