-
-
Notifications
You must be signed in to change notification settings - Fork 469
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?
Changes from all commits
991a7b4
f324d33
baf112e
a6a29bc
11d3719
aac734a
8e26859
180b722
8053895
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
import { invariant } from 'outvariant' | ||
import { getCallFrame } from '../utils/internal/getCallFrame' | ||
import { isIterable } from '../utils/internal/isIterable' | ||
import type { ResponseResolutionContext } from '../utils/executeHandlers' | ||
|
@@ -57,6 +56,11 @@ export type AsyncResponseResolverReturnType< | |
MaybeAsyncResponseResolverReturnType<ResponseBodyType>, | ||
MaybeAsyncResponseResolverReturnType<ResponseBodyType> | ||
> | ||
| AsyncGenerator< | ||
MaybeAsyncResponseResolverReturnType<ResponseBodyType>, | ||
MaybeAsyncResponseResolverReturnType<ResponseBodyType>, | ||
MaybeAsyncResponseResolverReturnType<ResponseBodyType> | ||
> | ||
> | ||
|
||
export type ResponseResolverInfo< | ||
|
@@ -117,11 +121,17 @@ export abstract class RequestHandler< | |
public isUsed: boolean | ||
|
||
protected resolver: ResponseResolver<ResolverExtras, any, any> | ||
private resolverGenerator?: Generator< | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any> | ||
> | ||
private resolverGenerator?: | ||
| Generator< | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any> | ||
> | ||
| AsyncGenerator< | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any>, | ||
MaybeAsyncResponseResolverReturnType<any> | ||
> | ||
private resolverGeneratorResult?: Response | StrictResponse<any> | ||
private options?: HandlerOptions | ||
|
||
|
@@ -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 commentThe 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. |
||
// Create a response extraction wrapper around the resolver | ||
|
@@ -304,39 +317,28 @@ export abstract class RequestHandler< | |
const result = this.resolverGenerator || (await resolver(info)) | ||
|
||
if (isIterable<AsyncResponseResolverReturnType<any>>(result)) { | ||
// Immediately mark this handler as unused. | ||
// Only when the generator is done, the handler will be | ||
// considered used. | ||
// Opt-out from marking this handler as used. | ||
this.isUsed = false | ||
|
||
const { value, done } = result[Symbol.iterator]().next() | ||
const nextResponse = await value | ||
|
||
if (done) { | ||
this.isUsed = true | ||
} | ||
|
||
// If the generator is done and there is no next value, | ||
// return the previous generator's value. | ||
if (!nextResponse && done) { | ||
invariant( | ||
this.resolverGeneratorResult, | ||
'Failed to returned a previously stored generator response: the value is not a valid Response.', | ||
) | ||
|
||
// Clone the previously stored response from the generator | ||
// so that it could be read again. | ||
return this.resolverGeneratorResult.clone() as StrictResponse<any> | ||
} | ||
|
||
if (!this.resolverGenerator) { | ||
this.resolverGenerator = result | ||
} | ||
|
||
const { done, value } = await this.resolverGenerator.next() | ||
const nextResponse = await value | ||
|
||
if (nextResponse) { | ||
// Also clone the response before storing it | ||
// so it could be read again. | ||
this.resolverGeneratorResult = nextResponse?.clone() | ||
this.resolverGeneratorResult = nextResponse.clone() | ||
} | ||
|
||
if (done) { | ||
// A one-time generator resolver stops affecting the network | ||
// only after it's been completely exhausted. | ||
this.isUsed = true | ||
|
||
// Clone the previously stored response so it can be read | ||
// when receiving it repeatedly from the "done" generator. | ||
return this.resolverGeneratorResult?.clone() | ||
} | ||
|
||
return nextResponse | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
/** | ||
* @vitest-environment node | ||
*/ | ||
import { http, HttpResponse, delay } from 'msw' | ||
import { setupServer } from 'msw/node' | ||
|
||
const server = setupServer() | ||
const toJson = (response: Response) => response.json() | ||
|
||
beforeAll(() => { | ||
server.listen() | ||
}) | ||
|
||
afterEach(() => { | ||
server.resetHandlers() | ||
}) | ||
|
||
afterAll(() => { | ||
server.close() | ||
}) | ||
|
||
it('supports generator function as response resolver', async () => { | ||
server.use( | ||
http.get('https://example.com/weather', function* () { | ||
let degree = 10 | ||
|
||
while (degree < 13) { | ||
degree++ | ||
yield HttpResponse.json(degree) | ||
} | ||
|
||
degree++ | ||
return HttpResponse.json(degree) | ||
}), | ||
) | ||
|
||
// Must respond with yielded responses. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(11) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(12) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(13) | ||
// Must respond with the final "done" response. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(14) | ||
// Must keep responding with the final "done" response. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(14) | ||
}) | ||
|
||
it('supports async generator function as response resolver', async () => { | ||
server.use( | ||
http.get('https://example.com/weather', async function* () { | ||
await delay(20) | ||
|
||
let degree = 10 | ||
|
||
while (degree < 13) { | ||
degree++ | ||
yield HttpResponse.json(degree) | ||
} | ||
|
||
degree++ | ||
return HttpResponse.json(degree) | ||
}), | ||
) | ||
|
||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(11) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(12) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(13) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(14) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(14) | ||
}) | ||
|
||
it('supports generator function as one-time response resolver', async () => { | ||
server.use( | ||
http.get( | ||
'https://example.com/weather', | ||
function* () { | ||
let degree = 10 | ||
|
||
while (degree < 13) { | ||
degree++ | ||
yield HttpResponse.json(degree) | ||
} | ||
|
||
degree++ | ||
return HttpResponse.json(degree) | ||
}, | ||
{ once: true }, | ||
), | ||
http.get('*', () => { | ||
return HttpResponse.json('fallback') | ||
}), | ||
) | ||
|
||
// Must respond with the yielded incrementing responses. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(11) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(12) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(13) | ||
// Must respond with the "done" final response from the iterator. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual(14) | ||
// Must respond with the other handler since the generator one is used. | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual( | ||
'fallback', | ||
) | ||
expect(await fetch('https://example.com/weather').then(toJson)).toEqual( | ||
'fallback', | ||
) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { it } from 'vitest' | ||
import { http, HttpResponse } from 'msw' | ||
|
||
it('supports generator function as response resolver', () => { | ||
http.get<never, never, { value: number }>('/', function* () { | ||
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.8)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.9)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.0)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.1)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 5 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.2)resolver-generator.test-d.ts > supports generator function as response resolver
|
||
yield HttpResponse.json({ value: 1 }) | ||
yield HttpResponse.json({ value: 2 }) | ||
return HttpResponse.json({ value: 3 }) | ||
}) | ||
|
||
http.get<never, never, { value: string }>('/', function* () { | ||
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.8)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.9)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.0)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.1)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 11 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.2)resolver-generator.test-d.ts > supports generator function as response resolver
|
||
yield HttpResponse.json({ value: 'one' }) | ||
yield HttpResponse.json({ | ||
// @ts-expect-error Expected string, got number. | ||
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.8)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.9)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.0)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.1)resolver-generator.test-d.ts > supports generator function as response resolver
Check failure on line 14 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (5.2)resolver-generator.test-d.ts > supports generator function as response resolver
|
||
value: 2, | ||
}) | ||
return HttpResponse.json({ value: 'three' }) | ||
}) | ||
}) | ||
|
||
it('supports async generator function as response resolver', () => { | ||
http.get<never, never, { value: number }>('/', async function* () { | ||
Check failure on line 22 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports async generator function as response resolver
|
||
yield HttpResponse.json({ value: 1 }) | ||
yield HttpResponse.json({ value: 2 }) | ||
return HttpResponse.json({ value: 3 }) | ||
}) | ||
|
||
http.get<never, never, { value: string }>('/', async function* () { | ||
Check failure on line 28 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports async generator function as response resolver
|
||
yield HttpResponse.json({ value: 'one' }) | ||
yield HttpResponse.json({ | ||
// @ts-expect-error Expected string, got number. | ||
Check failure on line 31 in test/typings/resolver-generator.test-d.ts GitHub Actions / typescript (4.7)resolver-generator.test-d.ts > supports async generator function as response resolver
|
||
value: 2, | ||
}) | ||
return HttpResponse.json({ value: 'three' }) | ||
}) | ||
}) | ||
|
||
it('supports returning nothing from generator resolvers', () => { | ||
http.get<never, never, { value: string }>('/', function* () {}) | ||
http.get<never, never, { value: string }>('/', async function* () {}) | ||
}) | ||
|
||
it('supports returning undefined from generator resolvers', () => { | ||
http.get<never, never, { value: string }>('/', function* () { | ||
return undefined | ||
}) | ||
http.get<never, never, { value: string }>('/', async function* () { | ||
return undefined | ||
}) | ||
}) |
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:
Reproduction steps
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:
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.