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

Adding msw integration #830

Closed
wants to merge 9 commits into from
Closed

Adding msw integration #830

wants to merge 9 commits into from

Conversation

paul-sachs
Copy link
Contributor

@paul-sachs paul-sachs commented Sep 18, 2023

This is a PoC of what a msw integration could look like.

Fixes #825

Caveats

@paul-sachs paul-sachs changed the title Adding msw Adding msw integration Sep 18, 2023
@paul-sachs
Copy link
Contributor Author

Note: These PR is mostly for discussion and will probably end up in it's own repo eventually.

Comment on lines 22 to 27
// 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",
Copy link
Contributor Author

@paul-sachs paul-sachs Sep 28, 2023

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

@tunguyen-ct
Copy link

tunguyen-ct commented Oct 23, 2023

Great news! MSW v2.0.0 has been released. Do we have an estimated timeline for shipping the connect-msw package? 😄

@srikrsna-buf
Copy link
Member

The PR has been open for some time now, with v2 around the corner, we should solve this for that.

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.

Support API mocking via MSW
4 participants