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

Support MSW 2 #12

Merged
merged 10 commits into from
Jun 12, 2024
Merged

Support MSW 2 #12

merged 10 commits into from
Jun 12, 2024

Conversation

IanVS
Copy link
Contributor

@IanVS IanVS commented Aug 18, 2023

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
Copy link
Contributor Author

IanVS commented Sep 12, 2023

Blocked at egoist/tsup#978 currently. May need to find a build tool other than tsup, I guess.

Nevermind, figured it out. Opened evanw/esbuild#3377 as a suggestion for improvement.

@IanVS
Copy link
Contributor Author

IanVS commented Sep 13, 2023

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
Copy link
Contributor Author

IanVS commented Sep 13, 2023

I think I've solved the "dynamic require" issue in #13, but it might still be worth considering supporting msw2.

@mrloop
Copy link
Contributor

mrloop commented Oct 26, 2023

We'd also get support for FormData mswjs/msw#1327

@IanVS
Copy link
Contributor Author

IanVS commented Oct 26, 2023

@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
Copy link
Contributor

mrloop commented Nov 1, 2023

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 mswjs/msw#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
Copy link
Contributor

mrloop commented Nov 27, 2023

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

@IanVS
Copy link
Contributor Author

IanVS commented Dec 7, 2023

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
Copy link
Contributor Author

IanVS commented Jun 12, 2024

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.

@IanVS IanVS changed the title Upgrade msw to next (2.0) Support MSW 2 Jun 12, 2024
@IanVS IanVS merged commit 786e42b into master Jun 12, 2024
2 checks passed
@IanVS IanVS deleted the update-msw branch June 12, 2024 16:29
@cah-brian-gantzler
Copy link
Collaborator

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
Copy link
Contributor Author

IanVS commented Jun 12, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants