-
-
Notifications
You must be signed in to change notification settings - Fork 529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Narrowing the response body type in HttpResponse.json
#2105
Comments
Hi, @Hajime-san. Thanks for this suggestion, I think it makes perfect sense. A bit of context why it allows additional fields right now. Since the response body type is provided onto the handler itself, there's no way to "drill" it to the argument type of export const handlers = [
handleSdkRequest(async ({ request }) => {
const data = await request.json()
// Body type is inferred from the argument you provide
// to the HttpResponse.json().
return HttpResponse.json({
transactionId: data.transactionId,
// `bar` should be error
data: { ok: true, foo: 'bar' },
})
}),
] The inferred body type becomes this:
This type is later compared to the return type of the entire resolver, which is inferred from your response body type argument (the narrow type). It seems, since it matches partially, it passes the validation. ReproductionHere's this issue distilled to a minimal reproduction scenario outside of MSW: |
Note that if you provide the narrow response body type to return HttpResponse.json<SdkResponse>({ ... }) Playground showcasing the "unknown property 'bar'" type error. Since Lines 79 to 82 in 16fadfe
|
HttpResponse.json
HttpResponse.json
@Andarist, please, correct me if I'm wrong, Mateusz. |
@Andarist, but this case is a bit different, isn't it? I can clearly get a proper narrow type if I provide an explicit body generic. The problem is not how the response body type is declared but how to project that type onto the // Works if the body type is explicitly provided.
// Hijacks inference from the argument value.
+return json<ResponseBodyType>({ foo: true, bar: 'xyz' })
// Breaks because now the T in json<T> gets inferred from
// a broader value and it satisfies the type.
-return json({ foo: true, bar: 'xyz' }) I get your point that a broader type satisfying a narrower type is how types work. I was mostly curious if we can make this type projection possible? Because in this case it does feel like a bug to the user. |
This is likely very close to the stuff I was investigating here. You can't easily influence how TS infers such things. You could use The essence here is likely something around those lines: the contextual return type is a lower priority inference and an inference made directly from the arguments will always "win" over it. |
TIL Are there any other implications to using |
Alas, I don't support one can check if a built-in type exists before using it. @Hajime-san, it looks like there's no capabilities in TypeScript to achieve what you want (see a fantastic explanation). You have one option: provide an explicit response body type to |
you'd break the simplest things like const j = json({ foo: 'bar'})
return j So it could lead to surprising DX.
Right. You can mimic it using this though: type NoInfer<T> = [T][T extends any ? 0 : never] |
But this would still work, would it not? const j = json<T>(value)
return j // StrictResponse<T> I'd say it's fine to have |
Yes. |
@Andarist, I'm trying your suggestion but not getting the same behavior for custom |
You only need to wrap |
I'm implementing this in #2107. I think this is a good direction to follow. |
Thank you for implementing this feature! |
Released: v2.2.11 🎉This has been released in v2.2.11! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
Scope
Improves an existing behavior
Compatibility
Reproduction steps
Versions:
"msw": "2.2.10"
Feature description
Following the documentation of Using with TypeScript, it works almost well.
But, it allows widing type of
HttpResponse.json
.Here is an modified Higher-order request handlers example of adding non-existence
foo
field toHttpResponse.json
.The text was updated successfully, but these errors were encountered: