mswjs / msw

Seamless REST/GraphQL API mocking library for browser and Node.js.
https://mswjs.io
MIT License
15.28k stars 479 forks source link

test: add type test for HttpResponse.error #2132

Open jacquesg opened 2 months ago

jacquesg commented 2 months ago
jacquesg commented 2 months ago

As expected, the CI is failing now due to the type error :)

kettanaito commented 1 month ago

Thanks for writing the tests, @jacquesg!

I think this hints at a more fundamental issue with how we annotate responses in MSW. See, this is the underlying type that everything depends on:

https://github.com/mswjs/msw/blob/7cf34c15ddd2edc688dc848eaf6eb89883ac6df6/src/core/handlers/RequestHandler.ts#L38-L45

And it expects a ResponseBodyType which is of type DefaultBodyType. For ResponseResolverReturnType to produce a non-strict, regular Response, the response body type argument must be undefined, which is valid per the DefaultBodyType type.

The issue happens when the response body type is supposed to be a union of types, like in case of GraphQL responses:

I don't believe that type likes the union as the argument. We also don't teach it how to handle unions by doing something like X extends [undefined] or other dark TypeScript magic.


Here's an isolated playground.

kettanaito commented 1 month ago

We can implement a fix like so and it will work.

It creates a different problem violating this expectation:

https://github.com/mswjs/msw/blob/7cf34c15ddd2edc688dc848eaf6eb89883ac6df6/test/typings/graphql.test-d.ts#L97-L103

Because the union produced to support Response.error() (return type of which is Response) now lets the HttpResponse.text() return type to pass even though its [bodyType] type is incompatible (it matches the generic Response type and since it's allowed, makes TS happy).

Preventing wrong mocked responses on the type level is a great feature to have. Unfortunately, Response.error() returns a generic Response type that cannot be discriminated against. This makes it impossible to tell apart a generic response type and an non-matching HttpResponse.* type because they come down to a generic Response anyway.