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
fix: support async generators as response resolvers #2108
base: main
Are you sure you want to change the base?
Conversation
kettanaito
commented
Mar 25, 2024
•
edited
edited
- Fixes "Cannot read properties of undefined (reading 'get')" when using generators/async generators as resolvers #2109
- Adds failing integration tests for using a generator (and async generator) function as response resolvers.
- Supports async generators as response resolvers.
- Adds missing type tests for generators and async generators as response resolvers (needs fix: accept a narrower response body type by default #2107 to properly pass).
@@ -256,6 +266,9 @@ export abstract class RequestHandler< | |||
return null | |||
} | |||
|
|||
// Preemptively mark the handler as used. | |||
// Generators will undo this because only when the resolver reaches the | |||
// "done" state of the generator that it considers the handler used. | |||
this.isUsed = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's important to mark the handler as used immediately, even if using generators then opts-out from this behavior. This is what we promise right now, so let's keep that promise.
@@ -57,6 +56,11 @@ export type AsyncResponseResolverReturnType< | |||
MaybeAsyncResponseResolverReturnType<ResponseBodyType>, | |||
MaybeAsyncResponseResolverReturnType<ResponseBodyType> | |||
> | |||
| AsyncGenerator< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older versions of TypeScript (4.7 - 5.2) have trouble digesting this type:
FAIL resolver-generator.test-d.ts > supports async generator function as response resolver
TypeCheckError: Argument of type '() => AsyncGenerator<StrictResponse<JsonBodyType>, StrictResponse<JsonBodyType>, any>' is not assignable to parameter of type 'HttpResponseResolver<never, never, { value string; }>'.
Type 'AsyncGenerator<StrictResponse<JsonBodyType>, StrictResponse<JsonBodyType>, any>' is not assignable to type 'AsyncResponseResolverReturnType<{ value string; }>'.
Type 'AsyncGenerator<StrictResponse<JsonBodyType>, StrictResponse<JsonBodyType>, any>' is not assignable to type 'AsyncGenerator<ResponseResolverReturnType<{ value string; }>, ResponseResolverReturnType<{ value string; }>, ResponseResolverReturnType<{ value string; }>>'.
The types returned by 'next(...)' are incompatible between these types.
Type 'Promise<IteratorResult<StrictResponse<JsonBodyType>, StrictResponse<JsonBodyType>>>' is not assignable to type 'Promise<IteratorResult<ResponseResolverReturnType<{ value string; }>, ResponseResolverReturnType<{ value string; }>>>'.
Type 'IteratorResult<StrictResponse<JsonBodyType>, StrictResponse<JsonBodyType>>' is not assignable to type 'IteratorResult<ResponseResolverReturnType<{ value string; }>, ResponseResolverReturnType<{ value string; }>>'.
Type 'IteratorYieldResult<StrictResponse<JsonBodyType>>' is not assignable to type 'IteratorResult<ResponseResolverReturnType<{ value string; }>, ResponseResolverReturnType<{ value string; }>>'.
Type 'IteratorYieldResult<StrictResponse<JsonBodyType>>' is not assignable to type 'IteratorYieldResult<ResponseResolverReturnType<{ value string; }>>'.
Type 'StrictResponse<JsonBodyType>' is not assignable to type 'ResponseResolverReturnType<{ value string; }>'.
Reproduction steps
- TypeScript 4.7
pnpm test:ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @Andarist! Sorry for troubling you once again. If you have a minute, could you please lend me a hand with this type issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would need to see a slimmed-down repro. The one that I tried put together in a rush errors in the current TS version too: TS playground
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for giving it a try, @Andarist. Here's a minimal repro:
pnpm install
pnpm build
pnpm test:ts test/typings/resolver-generator.test-d.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not exactly minimal ;p it still depends on everything in the MSW's codebase. I could really use a self-contained TS playground here.