fix: support async generators as response resolvers#2108
Conversation
| // 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.
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.
src/core/handlers/RequestHandler.ts
Outdated
| MaybeAsyncResponseResolverReturnType<ResponseBodyType>, | ||
| MaybeAsyncResponseResolverReturnType<ResponseBodyType> | ||
| > | ||
| | AsyncGenerator< |
There was a problem hiding this comment.
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.
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.
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.
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.tsThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
FWIW I did compile and test locally, and it all seemed to work, though I did not acutally test the full range of TS versions (but I can't imagine it wouldn't work, maybe I'm jinxing myself)
There was a problem hiding this comment.
Happy to give this a try once it's here 👀 Installing different typescript dependency versions triggers the test:ts into using that version for testing. The CI will also cover all the ranges for us. So excited we may actually ship this!
There was a problem hiding this comment.
Sent #2213.
FWIW if you're looking for multi-version testing and aren't too tied to vitest, https://github.com/tstyche/tstyche is one option I've heard of which automatically runs multiple versions of TS without having to install them explicitly.
There was a problem hiding this comment.
That's nice! Something to consider in the setup.
|
The fix from @jakebailey (#2213) seems to fix the compatibility with older TypeScript versions! 🎉 Super job here. Any objections to merge this? |
|
This technically now enables passing in arrays, maps, etc; is that desirable? Should it be tested? |
|
@jakebailey as the return value of the resolver? It would be nice to guard against that on runtime at least. |
|
Actually, the old code may have already allowed that, at least for like .values()... |
|
It's probably fine as is, I'm just paranoid given I know nothing about this codebase! |
|
@jakebailey, well, now you know a little bit :) I'm infinitely grateful for your work on this. Looks like I know another TypeScript wizard now. I will give that false matching types scenario a try before releasing this. |
|
Yeah, @jakebailey, you're right. As long as the return type satisfies the Iterator type, there's no type error: http.get('/user/:a/:b/:b', ({ params }) => {
return []
// ^ This compiles OK while it must error.
}) |
|
I guess the question is whether or not that's bad or not; I can just as easily pass in a generator which never yields today, right? |
|
That's certainly agains the established contract. Response resolvers either return/yield a |
|
This only seems to be a problem when returning an empty array Huh. |
|
Oddly enough, I'm beginning to like the array stuff. Since arrays are iterable, one can return an array of mocked responses at once (no need for a generator). If that happens, MSW will exhaust that array as it does a generator function, producing mocked responses in order, and stopping on the last response to then use it forever. I wouldn't document this behavior but I like it! |
Released: v2.3.3 🎉This has been released in v2.3.3! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
Uh oh!
There was an error while loading. Please reload this page.