-
Notifications
You must be signed in to change notification settings - Fork 81
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
Adding msw integration #830
Conversation
Note: These PR is mostly for discussion and will probably end up in it's own repo eventually. |
// We could make this more consistent with the exposed bypass() function provided by msw | ||
// IFF we could specify interceptors on a request by request basis. At least in theory, bypass() | ||
// works on a standard Request object so we'd need a converter from the interceptor UnaryRequest/StreamRequest | ||
// to a standard request to pass it to bypass() and out again. | ||
// Otherwise, we just need to make sure this keeps in sync with https://github.com/mswjs/msw/blob/8855956f561ad4afd4ce7ae6500ca706757bb5a9/src/core/bypass.ts | ||
"x-msw-intention": "bypass", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timostamm wanted to get your thoughts on this way of adding headers to a bunch of requests. Basically this header exists to tell msw to have a request bypass all mock handlers. This can be applied to any client request (as opposed to passthrough, which is designed to just the incoming request). It would be nice to have a way to intercept the Request before it gets sent to fetch so we can more seamlessly fit with the bypass()
function, which just says a Request and returns a new Request.
Not really blocking, just wanted to raise it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is bypass for? I see this comment in the source:
Unlike "passthrough()", bypass is meant for performing additional requests within pending request resolution.
If I intercept a request, and need to make some requests to provide a mocked response, do I need to set this header to prevent MSW from intercepting these requests as well?
That's pretty straight-forward with:
client.say({sentence: "hi"}, { headers: { "x-msw-intention": "bypass" } });
Users could also set up a transport specifically for this use case, with a connect interceptor that always sets this header.
Seems to me that this just needs good examples in our documentation, but please let me know if I'm missing something.
MSW might be able to provide slightly convenience by providing a fetch
function to response resolvers that always sets this header, and potentially inherits headers and cookies - similar to Svelte.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I intercept a request, and need to make some requests to provide a mocked response, do I need to set this header to prevent MSW from intercepting these requests as well?
Yes that is correct.
Seems to me that this just needs good examples in our documentation, but please let me know if I'm missing something.
Yeah maybe docs are sufficient. I wanted to raise it to see if there was any feature requests to allow interceptors on a per request basis. I've pushed up a custom fetch that can be passed. It's a little cleaner than a custom interceptor and can still be wrapped if someone needs an additional custom fetch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to raise it to see if there was any feature requests to allow interceptors on a per request basis.
Do you mean connect interceptors as described here? I think it would be a bad design choice to allow the per request because it would soften the separation of concerns, and opens up questions about order of interceptors. If you want to run interceptors conditionally, #841 is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah the lack of clarity on ordering makes sense. An easy way to work around that would be to instead allow a custom fetch to be defined on a per request basis, which wouldn't introduce any ordering confusion. But for now, a custom transport to handle bypasses will suffice. I don't think it's worth much api modification just to smooth out things like msw bypass.
Great news! MSW v2.0.0 has been released. Do we have an estimated timeline for shipping the connect-msw package? 😄 |
The PR has been open for some time now, with v2 around the corner, we should solve this for that. |
This is a PoC of what a msw integration could look like.
Fixes #825
Caveats