search

miragejs / mirage-msw

4 stars 4 forks source link

Support MSW 2 #12

Closed IanVS closed 5 months ago

IanVS commented 1 year ago

This updates the plugin to use MSW version 2, which relies more directly on standard fetch apis. The migration doc is here: https://github.com/mswjs/msw/blob/feat/standard-api/MIGRATING.md.

I think at this point, it's best to support only MSW 2+, to keep our maintenance burden lower, but I'd be happy to hear other opinions on it.

IanVS commented 1 year ago

~Blocked at https://github.com/egoist/tsup/issues/978 currently. May need to find a build tool other than tsup, I guess.~

Nevermind, figured it out. Opened https://github.com/evanw/esbuild/issues/3377 as a suggestion for improvement.

IanVS commented 1 year ago

One of the reasons I explored this in the first place is that I'm not able to use the current build output in my app, as I get an error:

Dynamic require of "events" is not supported

This seems to be because msw is not currently ESM-compatible. It relies on an older version of @mswjs/interceptors, and only in version 0.21.0 did they start to distribute ESM.

The alternative, if we want to keep using msw 1, is to try to resolve by polyfilling events I think. I'll see what can be done.

IanVS commented 1 year ago

I think I've solved the "dynamic require" issue in https://github.com/miragejs/mirage-msw/pull/13, but it might still be worth considering supporting msw2.

mrloop commented 1 year ago

We'd also get support for FormData https://github.com/mswjs/msw/issues/1327

IanVS commented 1 year ago

@mrloop are you using mirage-msw currently? It sounds like you're in favor of moving to msw2.

I think we might also need to make Mirage's createServer an async call, because whereas pretender is synchronous, msw takes a bit to start up, and does it asynchronously. Do you have any thoughts on a change like that?

mrloop commented 1 year ago

I tried out the branch, thanks! There's an issue around FormData. Pretender, MSWv1 and MSWv2 handle form data differently.

Using mirage/pretender I can access a FormData instance on the request.requestBody Using mirage/MSWv1 I can access a flat Object instance containing data on the request.body which I can convert into a FormData object. The request.requestBody is a JSON string of this Object. Using mirage/MSWv2 I can access a the raw form data string on the request.requestBody, I'd need to parse using something like parse-multipart-data

There's a bit of discussion about form data handling here https://github.com/mswjs/msw/issues/1327. Notably request.body is deprecated and no longer in MSWv2, and there is a new request.formData() method.

How should a mirage/MSWv2 user handle form data?

  1. are they expected to parse it themselves, this seems quiet erroneous of the mirage-msw user and could be a road block for adoption.
  2. do we provide a helper function to parse the 'raw' requestBody form data for them?
  3. do we use the MSWv2 formData() somehow and maybe add formData to mirage Request?
mrloop commented 11 months ago

Any thoughts on this @IanVS. I can probably add option 2) next week

IanVS commented 11 months ago

Sorry @mrloop I'm not currently in this headspace, but I'm happy to let you experiment and come up with a solution that meets your needs. Feel free to open a PR into this PR, if you'd like. I'll try to return to this some time in the next few weeks.

IanVS commented 5 months ago

I've been using this in my own app, and with the fixes I've made here, it's working well. I'm going to merge this, and can continue to iterate on it in subsequent PRs.

cah-brian-gantzler commented 5 months ago

Thanks for keeping this up. Way back when I had two tests that failed when I used MSW, will have to try again maybe they work now with all your changes. Would definitely like to experiment with it again.

IanVS commented 5 months ago

@cah-brian-gantzler thanks. I've merged a PR that uses this in my app at work, so I'm hopeful that it's in a pretty good spot. I did have some issues with the latest versions of msw, and had to pin to ~2.0, but hopefully that will be resolved eventually. I'm going to cut a new release of this package, and I think the next thing to do will be to support its usage in node. That should be entirely possible, but it will probably require some API changes. But honestly I think that's fine since this is so experimental soon.

Right now I'm running a patched version of miragejs that adds some async startup, but that will also require a breaking change at some point, I'd suggest doing it when we're confident that this msw support is in good shape.