-
-
Notifications
You must be signed in to change notification settings - Fork 474
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
fix: support intercepting fetch requests in React Native #2017
Conversation
src/native/index.ts
Outdated
@@ -13,5 +13,5 @@ export function setupServer( | |||
): SetupServerApi { | |||
// Provision request interception via patching the `XMLHttpRequest` class only | |||
// in React Native. There is no `http`/`https` modules in that environment. | |||
return new SetupServerApi([XMLHttpRequestInterceptor], ...handlers) | |||
return new SetupServerApi([FetchInterceptor], ...handlers) |
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.
We still need support XMLHttpRequest, do we not? Has React Native officially dropped XHR as the means to make requests? If so, I'd like to see that in their changelog.
Otherwise, we better to add FetchInterceptor
to the list here.
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.
React native does not have its own implementation of fetch. It uses whatwg-fetch above their XMLHttpRequest implementation. Because we fixed the error in fetch interceptor, xhr interceptor will still have the error. Is it OK to intercept both xhr and fetch if xhr will throw the error?
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.
Can you point me to the place where XHR interceptor would still have the error?
The problem as in accessing the request.signal
that doesn't always exist. But XHR interceptor doesn't access that property, afaik. Happy to be proven wrong.
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.
Never mind, I've just tried with both interceptors and it works
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.
@sairus2k, still a question from my side: can I write this code in React Native without any explicit polyfills on my end:
fetch('https://example.com')
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.
Yes, fetch is the first class citizen in RN https://reactnative.dev/docs/network
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.
Awesome! Then what you are proposing in this pull request definitely makes sense. Let's just add the fetch interceptor to the list, keeping XHR.
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.
One comment regarding dropping the XHR support in React Native.
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.
Fantastic! 🚀 Yet another awesome contribution.
Released: v2.1.7 🎉This has been released in v2.1.7! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
Switch to fetch interceptor to make MSW works in react-native. Relates to mswjs/interceptors#503
Thanks @willdawsonme for the solution