IanVS / vitest-fetch-mock

Vitest mock for fetch, forked from jest-fetch-mock
MIT License
68 stars 9 forks source link

Invalid response status after update to 0.4.0 #25

Closed gabscogna closed 1 month ago

gabscogna commented 1 month ago

I ran in problem with the newer version. I have a test like this:

// ...
fetchMocker.mockResponseOnce('', { status: 204 });
// ...

But when I run the tests, it give me the error:

TypeError: Response constructor: Invalid response status code 204
 ❯ buildResponse node_modules/vitest-fetch-mock/src/index.ts:388:12
 ❯ node_modules/vitest-fetch-mock/src/index.ts:59:1

It should accept send null as response body here: https://github.com/IanVS/vitest-fetch-mock/blob/6b4ba2e4155143eb55432007eb6f05bf0dbf1cc0/src/index.ts#L307

type ResponseBody = string | null;

...and here: https://github.com/IanVS/vitest-fetch-mock/blob/6b4ba2e4155143eb55432007eb6f05bf0dbf1cc0/src/index.ts#L381

if (responseProviderOrBody === null || typeof responseProviderOrBody === 'string') {

So this will work:

// ...
fetchMocker.mockResponseOnce(null, { status: 204 });
// ...
dirkluijk commented 1 month ago

Took me a moment to figure out what's happening here, but I found out.

Before, we were relying on cross-fetch:

import { Response } from 'cross-fetch';

new Response('', { status: 204 })

which works fine.

However, when running new Response('', { status: 204 }) in a Node.js or browser environment without polyfills, it throws an error:

This is by design. There are two solutions:

1) rewrite your code to:

// either:
fetchMocker.mockResponseOnce(() => ({ status: 204 }));
// or:
fetchMocker.mockResponseOnce(() => new Response(null, { status: 204 }));

2) we adjust vitest-fetch-mock to accept null as response body, which makes sense to me, but note that this wasn't possible in previous versions either. We just broke it by replacing the incorrect polyfill with the correct implementation.

@IanVS let me know what you think. I'm willing to make a fix if we agree on solution 2.

dirkluijk commented 1 month ago

Aside from making the response body nullable in mockResponse(string: ResponseBody, params?: MockParams), I opened another MR which enables you to also write:

fetchMocker.mockResponseOnce({ status: 204 });
fetchMocker.mockResponseOnce(new Response(null, { status: 204 }));

To allow passing null as response body, I opened a separate MR.

dirkluijk commented 1 month ago

The fix has been released in v0.4.1.

gabscogna commented 1 month ago

Thank you @dirkluijk I updated here and everything works fine now with 0.4.1 :)