search

hey-api / openapi-ts

🚀 The OpenAPI to TypeScript codegen. Generate clients, SDKs, validators, and more. Support: @mrlubos
https://heyapi.dev
Other
1.37k stars 106 forks source link

Make MSW and @hey-api/client-fetch talk together #1111

Open BjorneOma opened 1 month ago

BjorneOma commented 1 month ago

Description

Hey! Thanks for this project! I am using it in my new remix app and finding it very useful :)

I am trying to get the fetch-client and msw to talk together. Msw is client-agnostic and should work with any kind of http requests. Before I used @hey-api/client-fetch (and the functions generated in services.gen.ts) msw had no issues intercepting the requests and retuning mock data, but now I get this error: image

I am using msw on the server side of my remix app since all my api calls go through it.

Could I be doing something wrong or are msw and @hey-api/client-fetch just not compatible?

Versions: msw: 2.3.4 @hey-api/openapi-ts: 0.53.0 @hey-api/client-fetch: 0.2.4

mrlubos commented 1 month ago

Hey @BjorneOma, are you able to pinpoint which is the last version that worked? Same issue if you upgrade both to the latest version? It's hard to tell what's wrong right now, will need a bit of info before being able to diagnose

BjorneOma commented 1 month ago

I get the same error when running on latest msw (2.4.9) and @hey-api/client-fetch (0.4.0).

To clarify: I have not gotten these two to work together in any version. I used msw without @hey-api/client-fetch at first, and that worked smoothly. It was only after starting to use @hey-api/client-fetch that I faced issues.

mrlubos commented 1 month ago

I see. Did you use another client before with Hey API or did you not use Hey API at all? Are you able to put together a small example reproducing the issue?

BjorneOma commented 1 month ago

I used fetch directly. I'll try to create a codesandbox reproducing the error :)

BjorneOma commented 1 month ago

I tried to reproduce the issue, but in my repro everything just works... This is sort of good news, but I am still struggling to find out why it doesn't work in my project.

This is the minimal example with Remix + MSW + @hey-api/client-fetch if you are interested: https://codesandbox.io/p/devbox/fclkyw

I have figured out that the error is linked to calling client.setConfig({ baseUrl: 'http://localhost:8998', }); in my entry.server.tsx. This works in the codesandbox, so I have no idea why it gives me an error. The versions of msw, remix and @hey-api/client-fetch are the same in the sandbox and my project. If I instead add the baseUrl to the specific service function calls everything works like a charm, but I would prefer not to. Any ideas why it could cause the error I first posted?

mrlubos commented 1 month ago

What I was going to suggest first is verify tsconfig and package.json have the same configuration. Is there anything different in module resolution or which version gets used?

BjorneOma commented 1 month ago

I updated tsconfig and package.json config to be the same. Still same issue.

I tried moving client.setConfig into a loader and then it somehow works. This is also not ideal, as I need to set the config in a central spot that every user reaches. I honestly have no clue what could be wrong at this point 😅

tomgodber commented 1 month ago

Hi, it may be unrelated but I just found this thread because when I upgraded from 52.11 to 53.9 (with the same 0.40 @hey-api/fetch-client) all of my tests that rely on vitest-fetch-mock started failing - possibly this has something to do with how dependencies are resolved, as I noticed that the bundled openapi-ts library now uses import where before it used require?

mrlubos commented 1 month ago

@tomgodber did you also find a workaround? It sounds like it's now using ESM modules whereas before it resolved to CJS. This change was introduced in 0.53.3, so I think if you upgrade to 0.53.2 everything will still work, but 0.53.3 breaks it, can you confirm? I'd like to know which file in dist folder it resolved to before vs now.

tomgodber commented 1 month ago

@mrlubos I thikn something strange is going on in my setup - I rolled back to 0.53.2, after yarn installing it apparently installed 0.53.2 - and yet the top of the index.js file still looked like 0.53.9's (whereas I confirmed on npm it should use require, as you say). Manually cleared out that folder in node_modules, reinstalled, same thing again. I'll have to look at this tomorrow, sorry!

ScottGuymer commented 1 month ago

I am seeing the same behaviour in a project I am working on now too.

Did you ever track down the issue?

Its worth noting that it was working fine using the axios client. Switching to the fetch one broke all the msw tests.

It feels like something is happening with the way that the client is created and exposed that means that MSW is not able to add the interceptors it needs to be able to intercept the calls.

Therefore the calls made via the generated API are not being intercepted and going to the "outside world"

tomgodber commented 4 weeks ago

I'm afraid I never found a workaround - got dragged onto something else and had to leave the upgrade unfinished, probably won't get back to it for a few weeks but interested if anyone can solve it

tomgodber commented 4 weeks ago

Explaining this to a colleague, I think I may know the cause - previously the reusable client library was written out every time in codegen alongside the API-specific files, so it ran within the same context as your local test code etc. In the latest version it is pulled in from node_modules as a reusable library - I don't know enough about the Javascript security model to be sure, but I imagine there's some sort of separation of context there where the mock you create in local code is segregated from the version of fetch the library can see?

BjorneOma commented 4 weeks ago

I started looking into this again today because I am starting to feel the pain of needing to pass the config for every api call.

The problem

In my case, the problem arises when I call client.setConfig. I logged the output of client.getConfig to see what changes when I call client.setConfig().

When I don't call client.setConfig(): msw_openapi_no_setconfig

When I call client.setConfig(): image msw_openapi_setconfig

As you can see, baseUrl and throwOnError are updated as expected, but I also noticed that fetch and body have changed slightly too.

The (a lot nicer) workaround

image When I specify fetch it somehow just works. I assume this is a bug, but I am really happy to have found a nicer workaround :)

mrlubos commented 4 weeks ago

Thanks for investigating @BjorneOma, I'll review this thread once I reach the MSW plugin release

tomgodber commented 4 weeks ago

Cool, that would align with what I was thinking - pass in a fetch created in your own context and you can mock it, leave the library to its own devices and it pulls one in inside its own security context which will be unaware of any mocking? I'm guessing this is done deliverately for security reasons but it may instead be some symptom of dependency resolution

zgrybus commented 1 week ago

@mrlubos any news on that?