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

Upgrade to MSW 2.0 #1809

Merged
merged 11 commits into from
Nov 3, 2023
Merged

Upgrade to MSW 2.0 #1809

merged 11 commits into from
Nov 3, 2023

Conversation

zephraph
Copy link
Contributor

@zephraph zephraph commented Nov 1, 2023

The v1 to v2 migration guide can be found here.

This PR depends on oxidecomputer/oxide.ts#215

Most of the changes here are fairly mechanical like renaming the rest export from msw to http. The biggest change is the removal of the compositional req API from MSW which was a pseudo request object with extra metadata associate with it and the ability to compose utility functions on top of it. It now uses the standard Request object which includes some immediate tradeoffs like not having cookies hanging off of it. Cookies are now passed to the handler and have to be piped down through the rest of the call chain. Given the removal of the compositional API, utilities like delay are now stand alone functions which necessitates actually being able to write async handlers. I've wrapped all the handlers in type-fest's Promisable (T | Promise<T>) and we were actually already awaiting the results so no changes were needed there.

Overall, not too bad. Just remember to use HttpResponse for normal responses. I've also included a json helper export from the generated API to cut down on the diff as suggested.

Copy link

vercel bot commented Nov 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Nov 3, 2023 5:53pm

@@ -63,7 +64,7 @@ export const handlers = makeHandlers({
}
db.projects.push(newProject)

return json(newProject, { status: 201 })
return HttpResponse.json(newProject, { status: 201 })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn’t say no to cutting this diff down by doing const json = HttpResponse.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a big of finagling but I got it to work. Diff is definitely smaller now.

@zephraph zephraph marked this pull request as ready for review November 2, 2023 16:36
@zephraph
Copy link
Contributor Author

zephraph commented Nov 3, 2023

For historical context, @david-crespo and I spent all of yesterday trying to debug an issue where E2E tests were failing in safari due to this browser error

FetchEvent.respondWith received an error: TypeError: Response cannot have a body with the given status.

We traced it down to a bug in a safari work around in MSW which was causing an empty array buffer to be passed to the Response constructor on a 204 response code which expects no body. See the full report here: mswjs/msw#1827

I've worked around the issue in mockServiceWorker.js: 7a930f2

Also, note to our future selves: console logs and debug output for service workers on safari are shuttled off to a different devtools instance hidden in the developer menu.

Copy link
Collaborator

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. The Firefox E2E failure is likely due to GH's current outages.

@zephraph zephraph merged commit a61ecf2 into main Nov 3, 2023
7 checks passed
@zephraph zephraph deleted the msw-v2 branch November 3, 2023 19:57
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 4, 2023
oxidecomputer/console@bd65b9d...ae8218d

* [ae8218df](oxidecomputer/console@ae8218df) bump msw to 2.0.3 for the real safari fix
* [9a3f95a9](oxidecomputer/console@9a3f95a9) reduce dev console noise about local assets not served by MSW
* [a61ecf24](oxidecomputer/console@a61ecf24) oxidecomputer/console#1809
* [50f3a5b2](oxidecomputer/console@50f3a5b2) bump recharts and react query
* [1a2b5656](oxidecomputer/console@1a2b5656) oxidecomputer/console#1808
* [4c01cd68](oxidecomputer/console@4c01cd68) oxidecomputer/console#1800
* [57cc1892](oxidecomputer/console@57cc1892) oxidecomputer/console#1802
* [47d76dbf](oxidecomputer/console@47d76dbf) oxidecomputer/console#1806
* [eee0eb2e](oxidecomputer/console@eee0eb2e) oxidecomputer/console#1805
* [60c2285e](oxidecomputer/console@60c2285e) oxidecomputer/console#1804
* [d9cf1ef1](oxidecomputer/console@d9cf1ef1) bump sort imports plugin for vuln
* [ba3a383e](oxidecomputer/console@ba3a383e) bump playwright to 1.39 (fix issue with z-index test)
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 4, 2023
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.

2 participants