msw
msw copied to clipboard
feat(HttpResponse): add empty response helper for strict empty responses
This PR adds a helper that makes it possible to create a StrictResponse<null>
response without a content header or type casting.
Motivation
We have some scenarios where handlers return responses without a body. This appears to be best enforced by typing the response body for resolver functions with null
.
// Typing the body with "never" does not allow any response...
http.delete<never, never, never>('/resource', () => {
return new HttpResponse(); // Errors
});
// Typing the body with "undefined" (or providing nothing) does allow responses with any body...
http.delete<never, never>('/resource', () => {
return HttpResponse.json({id: 42}); // Should not be allowed since not empty
});
// Typing the body with "null" achieves what we are looking for
http.delete<never, never, null>('/resource', () => {
return HttpResponse.json(null, { status: 204 }); // Works but also sets the content-type
return new HttpResponse(null, {status: 204}) as StrictResponse<null>; // Works but needs casting
});
However, as you can see in the example, creating a response that satisfies the resolver either also sets a content-type or requires casting. Neither of these downsides should be required to create an empty response. Therefore, this PR adds HttpResponse.empty
to work around this downsides, which also sets the default status to 204 as a little bonus.
http.delete<never, never, null>('/resource', () => {
return HttpResponse.empty(); // Returns StrictsReponse<null> with 204
});
http.post<never, never, null>('/resource', () => {
return HttpResponse.empty({ status: 201 }); // Returns StrictResponse<null> with 201
});
If you agree on adding this to MSW, I can create the relevant PR for updating the documentation for HttpResponse.empty
.
Hi, @christoph-fricke 👋 That's for raising this.
// Typing the body with "never" does not allow any response...
http.delete<never, never, never>('/resource', () => {
return new HttpResponse(); // Errors
});
This looks more like a bug to me. This should compile. Maybe we can try looking into why HttpResponse
doesn't like this instead?
I can't say I like the idea of HttpResponse.empty()
. From the description of it, it's sole purpose is to satisfy TypeScript, so I suggest we attempt to fix the existing constructor to pass the never
response body generic. That should be the correct approach here.
I want to be extremely cautious with the HttpResponse.*
methods. For example, all of them must follow the Response
construct signature, but in your proposed case, supporting a response body value contradicts the HttpResponse.empty()
intention. This is the first hint we may be doing something wrong here.
Can you share what exact TypeScript error you get when using the never
response body generic (your first example)?
http.delete<never, never, never>('/resource', () => { return new HttpResponse(); // Errors });
I was briefly looking at this, and figured some sample errors help
Argument of type '() => HttpResponse' is not assignable to parameter of type 'ResponseResolver<HttpRequestResolverExtras<PathParams>, DefaultBodyType, never>'.
Type 'HttpResponse' is not assignable to type 'AsyncResponseResolverReturnType<never>'
Some sample errors here:
https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbwBYxmANHAEqsAlAUwGdIA7IgzAZRimAGMZCSJzK4AFAQxiW6i4gimACIEAZlwCuAGxgAhCABMAngBUVYdgEEiK0vWZkKzCDIBuBKIRhSopDVoC+ccVAgg4AciEB3LwBQAShoAHRKBDIEMAQAPKQEllCYCUkpiVYAfAAUXgD0UMQQdvQEXpjZAJRwALyZiAFwcIW29nAJvti4RqwU2aSyMpUBTpUA3EEhYOGR0XHcvPyCwnBikrIKyuqa7KlZuQVFJWUV1XUNTS12pO0EnThoPWz9g9VcRHA0dIxPFPEZUEyI3Gk1wMyiMViCz4XAEQlEEmkckUqkcuwBOXyhSIxSgpXKcCqtXqCEazWi11u926xGMBBeMiGcHen1oDCYtN6cS4pBUQNGE2CYIiEPmPBhcJWayRm1ROxSg0xhxxxwJRPOpMuFLaHS6j05zwGjLeHy+7N+cSNMn5IKFYRFc3+aVuzoARK6ldjcfjTsSLuTWjcHvgDRRQjEAB4wKrAwVTcGOvbJF1WBWMz1HPEnQlnElkq5tYMW0IAKxxpAZQ1jQA
@mattcosta7 Thank you for providing these sample errors! I have been out of town for the past two days and didn't have my laptop on me.
Using never
If we consider "fixing" never
what should it result to when used as the response body type?
Allow returning any Response
, so the same as undefined
? Does this really align with the semantics of never
, which represents values that are never observed? See https://www.typescriptlang.org/docs/handbook/2/functions.html#never.
Allow returning only StrictResponse<never>
? This would get weird. When Responses
are created they accept BodyInit | null | undefined
as the body. What value should a developer provide for the body when creating a StrictResponse<never>
, especially when they want to add second argument for the ResponseInit
?
Strict Empty Responses
@kettanaito I understand your hesitation towards adding more methods to HttpResponse
and I am open to other solutions if they achieve the essence of this PR. I am searching for a solution to express that a response resolver is allowed to only return responses that have no body. Best I found is using null
as the response body type, which results in StrictResponse<null>
as the response resolver return type. All other types either result in any Response
, e.g. undefined
, no allowed response at all, see never
in the sample errors, or other non-empty strict responses when some other type is provided. Examples of these can be found in my initial PR description.
Therefore, null
and thus StrictResponse<null>
look like the best option. However, I found that there is no ergonomic way of creating instances of StrictResponse<null>
without including unintended headers in the response:
// This works and is ergonomic buts also sets the content-type header
const r1: StrictResponse<null> = HttpResponse.json();
// This works but isn't ergonomic
const r2: StrictResponse<null> = new HttpResponse() as StrictResponse<null>;
// This would work and is ergonomic
const r3: StrictResponse<null> = HttpResponse.empty();
Alternatively, I could imagine having HttpResponse
implement StrictResponse
to make the intent of this PR possible without type casting:
// Would be fine for me as well when it works without errors
const r1: StrictResponse<null> = new HttpResponse();
// This should error with this approach since a string is no empty response body
const r2: StrictResponse<null> = new HttpResponse("No Content");
@mattcosta7 Thank you for providing these sample errors! I have been out of town for the past two days and didn't have my laptop on me.
Using
never
If we consider "fixing"
never
what should it result to when used as the response body type?Allow returning any
Response
, so the same asundefined
? Does this really align with the semantics ofnever
, which represents values that are never observed? See https://www.typescriptlang.org/docs/handbook/2/functions.html#never.Allow returning only
StrictResponse<never>
? This would get weird. WhenResponses
are created they acceptBodyInit | null | undefined
as the body. What value should a developer provide for the body when creating aStrictResponse<never>
, especially when they want to add second argument for theResponseInit
?Strict Empty Responses
@kettanaito I understand your hesitation towards adding more methods to
HttpResponse
and I am open to other solutions if they achieve the essence of this PR. I am searching for a solution to express that a response resolver is allowed to only return responses that have no body. Best I found is usingnull
as the response body type, which results inStrictResponse<null>
as the response resolver return type. All other types either result in anyResponse
, e.g.undefined
, no allowed response at all, seenever
in the sample errors, or other non-empty strict responses when some other type is provided. Examples of these can be found in my initial PR description.Therefore,
null
and thusStrictResponse<null>
look like the best option. However, I found that there is no ergonomic way of creating instances ofStrictResponse<null>
without including unintended headers in the response:// This works and is ergonomic buts also sets the content-type header const r1: StrictResponse<null> = HttpResponse.json(); // This works but isn't ergonomic const r2: StrictResponse<null> = new HttpResponse() as StrictResponse<null>; // This would work and is ergonomic const r3: StrictResponse<null> = HttpResponse.empty();
Alternatively, I could imagine having
HttpResponse
implementStrictResponse
to make the intent of this PR possible without type casting:// Would be fine for me as well when it works without errors const r1: StrictResponse<null> = new HttpResponse(); // This should error with this approach since a string is no empty response body const r2: StrictResponse<null> = new HttpResponse("No Content");
I think never
is probably incorrect as well
FWIW undefined
seems to work where null and never don't
http.delete<PathParams, DefaultBodyType, undefined>('/resource', () => {
return new HttpResponse()
});
this appears related to this type:
https://github.com/mswjs/msw/blob/88c45ea5295fa7a47a943311dff65c99ebb3111e/src/core/handlers/RequestHandler.ts#L30-L35
Which could be extended to account for null as well as undefined, but probably not never?
http.delete<PathParams, DefaultBodyType, undefined>('/resource', () => {
return new HttpResponse()
});
http.delete<PathParams, DefaultBodyType, undefined>('/resource', () => {
return new HttpResponse(JSON.stringify({key: 'value'}), {headers: {'Content-Type': 'application/json'}})
});
http.delete<PathParams, DefaultBodyType, undefined>('/resource', () => {
return new HttpResponse(null, {})
});
each of these works fine as an empty response, since they'll accept a valid Response
object as a return which HttpResonse
extends
I think we have two separate topics here. First one being never
as response body type and the second topic is about strictly-typed empty responses (null
).
Regarding never
: I don't think there is much we can do without some weirdness in other places. It might be changed to behave as undefined
, but they have separate meanings so combining them in the same behavior seems weird.
I am way more interested in strictly-typed empty responses, which this PR is all about:
null
is working perfectly fine. I want it to result in StrictResponse<null>
as the resolver function's return type. Combining its behavior with undefined
, i.e. resulting in Response
would break my use case.
I am approaching this topic from the angle of Openapi-MSW. I want to express that a handler can only return responses with empty content as StrictResponse<null>
expresses, and not any content as Response
expresses.
OpenAPI-MSW is all about surfacing type conflicts in MSW resolvers when they no longer match their OpenAPI specification, e.g. return a response body that does not match the defined response schema. Imagine an API endpoint that returns some response content and is now changed to return no-content with the response. This should surface a type error in MSW handlers for this endpoint since they would still return a response and have to be adjusted.
And indeed, this scenario does surface a type error when the response body generic of the MSW resolver function gets typed with null
(=> StrictResponse<null>
), which OpenAPI-MSW is doing since I merged this PR: https://github.com/christoph-fricke/openapi-msw/pull/17. This works well and is good. 👍
However, I don't thing the current way of creating strictly typed responses with no content (StrictResponse<null>
) provides a good DX, which is what I am trying to change with this PR. The code examples from my previous comments to showcase how I think the DX could be improved.
I hope that the OpenAPI scenario better explains what I am trying to achieve and why. I created a small Playground that hopefully helps explaining it even more: Link to Playground
Thanks for the detailed discussion, @christoph-fricke, @mattcosta7! Will provide my input below.
On default body types
The DefaultBodyType
type that both request and response bodies extend already allows both undefined
and null
as accepted values:
https://github.com/mswjs/msw/blob/88c45ea5295fa7a47a943311dff65c99ebb3111e/src/core/handlers/RequestHandler.ts#L13-L20
The difference is like what you'd expect:
-
undefined
, the response has no body; -
null
, the response has an empty body.
JavaScript is a bit not in our favor here because runtime has no distinction between null
and undefined
response body init, although there is a strong semantic distinction. But this is what runtime JavaScript results in:
new Response().body
// null
new Response(undefined).body
// null
new Response(null).body
// null
I'd argue that the first example must return
undefined
as thebody
value of the response because that's precisely what was given to theResponse
constructor.
On new HttpResponse()
strict body type
Right now, the HttpResponse
class directly extends the global Response
class, which has no strict body type. I believe this is why the proposal is here. And this is also the main difference and the reason why HttpResponse.text()
and HttpResponse.json()
can have strict response body type—because they return a ScriptResponse
custom type that achieves strict response body type through the symbol-based type.
What we can try to do here is to make HttpResponse
implement the StrictResponse
type. It will still be a type-only kind of response body validation but that's precisely what we're after. We can also turn the StrictResponse
into a branded type instead of introducing an internal bodyType
symbol, I think that's redundant.
But that likely won't work. TypeScript is notoriously bad with null
/undefined
distinction. Here's an example (in this example, the validate(new Foo(null))
call must type error because Foo<null>
doesn't extend the required type of Foo<undefined>
in theory; in practice, it does).
I don't think that purely type-level safety is possible in this regard. I don't think that introducing HttpResponse.empty()
is a good solution for this either. We are after type-safety, and as such, as must implement the solution on the type level.
This is also a good moment to mention that request/response body type safety is purely imaginary. In reality, nothing can guarantee what body the application will receive, so, technically speaking, bodies must always be unknown
and then explicitly narrowed both type- and runtime-wise to a particular type. This reminds me of the sentiment that @pcattori shared with me on proper type safety and how type-casting is beneficial at times. MSW may be trying to do you a favor with the StrictRequest
/StrictResponse
but, as a result of it, shooting itself in the foot.
Actually, my bad, I'm always forgetting about this TS hack to know if a generic was provided:
type X<T> = [T] extends [never] ? 'NOT PROVIDED' : 'PROVIDED'
Alas, we cannot use this for StrictResponse
because:
A class can only implement an object type or intersection of object types with statically known members.ts(2422)
🫠
@kettanaito Thank you for your input! I will have a look at this again this weekend and convert the PR from HttpResponse.empty()
to implementing a branded type or StrictResponse
for HttpResponse
directly.
This is also a good moment to mention that request/response body type safety is purely imaginary. In reality, nothing can guarantee what body the application will receive, so, technically speaking, bodies must always be unknown and then explicitly narrowed both type- and runtime-wise to a particular type.
I think this is only partially true, right? You are right that in a client/server setting, with both specifying the same request- and response-body for a communication, they can't rely on the incoming data matching the specified type. Meaning, the client can't rely on it's response body type and the server cannot rely on the request body type.
However, for the client the request body is a contract that must be fulfilled by its requests, and for the server the response body type is a contract that must be fulfilled by its response. They can make sure that the type of the data they own is matched. When both rely on the same contract types, they can make sure that they at least fulfill their own bargain to make the communication work.
Client: Network Server:
- Request Type: Contract <-----> - Request Type: Potential Lie
- Response Type: Potential Lie <-----> - Response Type: Contract
Reflecting this back onto MSW handlers: The request body type is kinda a lie that we have for convenience. But the response body type is able to act as a contract that resolvers have to fulfill to ensure that they return a response that is expected by the application (client). Of course, this doesn't matter if the real backend returns completely different responses, but this is where OpenAPI comes into play to specify a communication contract. ;)
The recent changes about a proper response body type validation can help us implement this (#2107). In fact, I can achieve this behavior that validates the empty (null
) response body correctly:
My only comment would be that it'd be nice if that type error happened on the argument to new HttpResponse()
vs on the return type of the response resolver. It does happen in the correct place for HttpResponse.text()
and HttpResponse.json()
methods, so not sure why the constructor is different. The only thing that differs it is that the constructor cannot have generic type arguments so I'm adding the argument to the class itself.
Edit: I think I know the difference that causes it. HttpResponse.text()
returns a StrictResponse<T>
type while new HttpResponse()
always returns the HttpResponse<T>
type (even if it implements StrictResponse<T>
). In other words, the return type of the constructor is not strictly compatible with the return type expected by the resolver (StrictResponse
). I think that's why TypeScript lifts that error as a return type error, not the constructor argument error.
I've opened a pull request that will add support for explicit empty response bodies by using null
as the response body type generic argument:
https://github.com/mswjs/msw/pull/2118/files#diff-cb924248fca260b733cd71049d2b17c14cf5c52ef561273ab8e420b1d5a674a5R88-R104
@christoph-fricke, could you please take a look at that pull request and confirm that it satisfies your use cases? Thanks.
I will close this in favor of #2118. It achieves the original use case without having to resort to custom response static methods. Thanks for raising this, @christoph-fricke! I can still use your wise eyes on that pull request.