fix: inference of ResponseType<HttpRoute>
#34
Conversation
bitgopatmcl
force-pushed
the
fix-response-inference
branch
from
ad533fa to
f466304
Compare
Instead of `never` this should be something like the union of all responses with `any` or `unknown` body.
bitgopatmcl
force-pushed
the
fix-response-inference
branch
from
f466304 to
81431c8
Compare
|
This is primarily for correctness. The intention of |
|
I scheduled a meeting to discuss this change in more detail; I think I'm too hazy on the specifics but I have a concern I'd love to run past you in a higher-bandwidth medium before approving |
| body: res.body, | ||
| original: res.original, | ||
| }; | ||
| return res as ExpectedDecodedResponse<Route, StatusCode>; |
There was a problem hiding this comment.
To be honest I actually don't know why the type of res isn't narrowed by control flow here. Maybe just because of some complexity limit?
| const ensureAllResponsesDefined = <T extends { [K in Status]: number }>( | ||
| responseCodes: T, | ||
| ) => responseCodes; | ||
|
|
||
| export const HttpResponseCodes = ensureAllResponsesDefined({ |
There was a problem hiding this comment.
It seems like a smell to use this identity function (with penalty in code size and execution time) to enforce a type-level relationship, what about using the following patch?
export const HttpResponseCodes = {
ok: 200,
invalidRequest: 400,
unauthenticated: 401,
permissionDenied: 403,
notFound: 404,
rateLimitExceeded: 429,
internalError: 500,
serviceUnavailable: 503,
} as const;
// Create a type-level assertion that the HttpResponseCodes map contains every key
// in the Status union of string literals, and no unexpected keys. Violations of
// this assertion will cause compile-time errors.
//
// Thanks to https://stackoverflow.com/a/67027737
type ShapeOf<T> = Record<keyof T, any>
type AssertKeysEqual<X extends ShapeOf<Y>, Y extends ShapeOf<X>> = never
type _AssertHttpStatusCodeIsDefinedForAllResponses = AssertKeysEqual<{ [K in Status]: number }, HttpResponseCodes>;There was a problem hiding this comment.
Fixed in 311d8a0, but did it in a test file as an assignment check.
bitgopatmcl
force-pushed
the
fix-response-inference
branch
from
7d1eb57 to
311d8a0
Compare
This changes the way that `HttpResponse` is declared. The language server now offers actual helpful autocomplete suggestions, and type inferencing behaves correctly as far as I can tell.
bitgopatmcl
force-pushed
the
fix-response-inference
branch
from
311d8a0 to
01f9c88
Compare
|
🎉 This PR is included in version 1.0.0-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 0.2.0-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 0.1.1-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 0.2.0-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
2 participants
Instead of
neverthis should be something like the union of allresponses with
anyorunknownbody.Broken:
Fixed:
