Skip to content
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

MSW doesn't seem to be working with URQL or Vite correctly #1593

Closed
4 tasks done
RIP21 opened this issue Apr 12, 2023 · 12 comments
Closed
4 tasks done

MSW doesn't seem to be working with URQL or Vite correctly #1593

RIP21 opened this issue Apr 12, 2023 · 12 comments
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser

Comments

@RIP21
Copy link

RIP21 commented Apr 12, 2023

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 14 or higher

Browsers

Chromium (Chrome, Brave, etc.)

Reproduction repository

https://codesandbox.io/p/sandbox/brave-voice-5yd8ut

Reproduction steps

I have problems (it doesn't work no matter what I tried) with URQL + Vite. Probably due to some smart-ass preflight calls or so.

Will be glad to get help here or report a bug. A reproduction is a bit tricky on codesandbox, so I suppose downloading the archive and running it locally going to be the best way to test it.

Current behavior

Not a single request to /graphql works despite handlers being registered.

Expected behavior

Everything works perfectly with all requests getting intercepted returning data.

@RIP21 RIP21 added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser labels Apr 12, 2023
@mayteio
Copy link

mayteio commented Apr 13, 2023

Reproducible demo - this is just CRA with storybook, urql and msw/storybook-addon-msw.

  1. clone
  2. npm install
  3. npm run storybook
  4. navigate to the "header -> logged in" story and check the console - MSW is enabled but the graphql call is not being intercepted

+1 - we've noticed this since updating to urql's latest version (4.x.x). We've noticed it specifically in storybook, but it seems it's an issue for a handful of people across jest, storybook, vite so I'm posting the issue here. I've used storybook in my demo because it pertains to our issues.

Screenshot 2023-04-14 at 9 12 36 am

Screenshot 2023-04-14 at 9 10 31 am

@RIP21
Copy link
Author

RIP21 commented Apr 13, 2023

@mayteio I think it's related to URQL getting smarter and shipping some cool stuff such as multipart etc. support out of the box without special exchanges and whatnot.

E.g this
urql-graphql/urql#3114

But it's not a BUG of URQL, not at all, I think it's just MSW isn't compatible with these things or so. I think this will be sorted with the next major release of MSW, unsure if addressable now which is a bit pity.

@mayteio
Copy link

mayteio commented Apr 14, 2023

Yep - makes total sense. Thanks for the info! I'd love to see continued support.

@tnyo43
Copy link

tnyo43 commented Apr 14, 2023

Since version 4.0.0, urql started to support for 'text/event-stream' response (urql-graphql/urql#3050).
Because msw just bypasses server sent event, urql requests are not intercepted.

// Bypass server-sent events.
if (accept.includes('text/event-stream')) {
return
}

Actually it's not a technical issue of msw but an issue of Service Worker (#156 (comment)).

As a solution for now to use both urql and msw together, I recommend you to add fetchOptions to your urql client (I created a PR for @mayteio 's demo mayteio/storybook-msw-urql#1).

export const graphqlClient = createClient({
  url: 'http://localhost/graphql',
  exchanges: [fetchExchange],
  fetchOptions: { headers: { accept: '*/*' } } // <- overwrite accept of the request header
});

@mayteio
Copy link

mayteio commented Apr 14, 2023

Thanks @tnyo43! What are the implications of accepting /? Would a regular application/json suffice?

@tnyo43
Copy link

tnyo43 commented Apr 15, 2023

* is just a wild card and accept: */* means it accepts any content types. (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept#directives)

@scottrippey
Copy link

This issue seems to affect all users of Urql v4 ... which seems like a pretty big deal.

Around 2 months ago, URQL added these headers by default:
https://github.com/urql-graphql/urql/blob/1470a9c7474c75b65303b503c0a3a51e895f1a38/packages/core/src/internal/fetchOptions.ts#L125-L126

Accept: application/graphql-response+json, application/graphql+json, application/json, text/event-stream, multipart/mixed

It seems to me like msw should be looking for an Accept type that it likes (eg. application/graphql-response+json, application/graphql+json, application/json), rather than skipping just because it finds one it doesn't like. I'd be happy to create a PR, if this seems reasonable?

@kettanaito
Copy link
Member

Hi, @scottrippey. As far as I understand the issue, URQL sends the Accept: text/event-stream request header by default, which causes MSW to skip those requests in the browser because we don't have the event stream support at the moment. I am not aware of any other logic that would skip or ignore an outgoing request based on the Accept request header.

@scottrippey
Copy link

@kettanaito You're correct, the presence of text/event-stream is why MSW skips these requests.
However, URQL's Accept header has five acceptable formats, not just text/event-stream -- it also lists application/json, and other GraphQL-spec ones.

Unfortunately, MSW's logic says "if text/event-stream is present, MSW cannot handle this request".
I'd suggest that the logic should be improved. For example, "if any of the following accept types is present, MSW CAN handle this request: application/json, text, etc..."

@naorz
Copy link

naorz commented Jun 7, 2023

Hi,

I've encountered a specific issue while using msw to mock my GraphQL requests. Despite my efforts, I'm receiving responses that appear to originate from my actual server, rather than the expected mock responses. It seems that the mockServiceWorker is not intercepting the requests as intended.

To gain further insights into the problem, I decided to experiment with REST mocks. Unfortunately, I found that the worker is not functioning as expected in this scenario either.
However, I can confirm that the service worker itself is being successfully loaded.

Here are the details of my environment:

  • Node version: v18.12.1
  • npm version: 9.2.0

To reproduce the issue, I created an isolated environment using the command npm create vite@latest with React, TypeScript, and SWC. I installed msw, and in the attached image, you'll find all the code changes I made.
Additionally, the second image demonstrates that the web service is indeed loaded successfully according to the web console, and it appears to be functioning correctly.

Despite these observations, I'm still encountering a 404 error.
It's worth mentioning that I've double-checked the accept header, ensuring it includes application/json and */*, and I've also verified that the handler is correctly sending the expected content type in the response. However, the issue persists.

I kindly request your assistance and guidance in resolving this matter. Any help would be greatly appreciated.

code

web

@kettanaito
Copy link
Member

@naorz, I have some time to look into this. Please send me the link to that public reproduction repo. Thanks!

@kettanaito
Copy link
Member

I'm going to close this. All the references in this issue concern MSW 1.x, which has reached EOL. Please update to msw@latest, the issue should be gone there. If it's not, submit a new issue on GitHub and attach a reproduction repository. Thanks.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser
Projects
None yet
Development

No branches or pull requests

6 participants