mswjs / msw

Seamless REST/GraphQL API mocking library for browser and Node.js.
https://mswjs.io
MIT License
15.69k stars 503 forks source link

React Native support is broken #622

Closed wwdrew closed 3 years ago

wwdrew commented 3 years ago

This code was taken directly from the React Native GraphQL branch in the examples repository (https://github.com/mswjs/examples/tree/examples/gql-react-native/examples/graphql-react-native-apollo). I've been unable to get that to work at all for other reasons, so I thought I'd create a new repository using the latest versions of everything to demonstrate the error I'm receiving.

https://github.com/wwdrew/mswjs-react-native-failing

Environment

Name Version
msw 0.27.0
browser n/a
react-native 0.63.4
OS MacOS 11.2.1

Current behavior

Metro crashes with this stack trace:

error: Error: Unable to resolve module os from <obscured>/AnotherTest/node_modules/msw/native/lib/index.js: os could not be found within the project.

If you are sure the module exists, try these steps:
 1. Clear watchman watches: watchman watch-del-all
 2. Delete node_modules and run yarn install
 3. Reset Metro's cache: yarn start --reset-cache
 4. Remove the cache: rm -rf /tmp/metro-*
  4 |
  5 | var XMLHttpRequest = require('node-request-interceptor/lib/interceptors/XMLHttpRequest');
> 6 | var os = require('os');
    |                   ^
  7 | var tty = require('tty');
  8 | var events_1 = require('events');
  9 | var nodeRequestInterceptor = require('node-request-interceptor');

Expected behavior

Expecting the API call to be successful and not crash the bundler.

Screenshots

msw-error
wwdrew commented 3 years ago

This problem is a serious blocker for me, if anyone has any idea what might be causing it I'd be happy to try to track it down. Any kind of guidance would be really helpful.

kettanaito commented 3 years ago

Hey, @wwdrew. Thanks for reporting this.

This issue is caused by how node-request-interceptor library is bundled (and also how MSW bundles library within itself). Looks like it shouldn't include os (and possibly other modules) that don't exist in React Native.

If this issue blocks your work, I suggest cloning MSW and experimenting with its build configuration. Here's how we bundle the library for React Native:

https://github.com/mswjs/msw/blob/10cccc03f18e462dfaa12d42ded6ccc0a9e432f9/rollup.config.ts#L121-L155

The first thing I'd try is omitting the os from the external array in the config.

I don't have experience with React Native to provide much guidance at this time.

wwdrew commented 3 years ago

@kettanaito Thanks so much for the reply, that's fantastic I'll start taking a look at it and see what I can come up with.

wwdrew commented 3 years ago

From my past experience, the last version that hasn't failed this way was 0.22.2. I've gone back to check out the difference and, sure enough, between 0.22.2 and 0.22.3 something was added that includes 'os' and 'tty', and it appears to be the 'bold' function from the chalk package.

To verify this is the cause I've removed it from createSetupServer.ts and the resulting build file no longer includes 'os' or 'tty' - result! However, it's still pulling in 'events' and 'url' so there's still more to solve, but I feel like I'm making progress at last.

wwdrew commented 3 years ago

I've had a bit more time this evening to work on this and have managed to get it working a little further, but with some big caveats.

First of all, in order to import 'events' I've had to install the 'events' package, otherwise it fails trying to load that package. This is only used in StrictEventEmitter, I haven't tracked down where this is imported so I'm not sure yet how to overcome this one. Since it's referring to EventEmitter, I'm hoping I can change the import to bring in the RN version rather than going through the events package. If someone out there has any suggestions on this I'd be really happy to hear them.

The next problem is the usage of 'url' in the getPublicUrlFromRequest function. Just as a hack I've removed the function call to 'format' entirely and there are no more runtime errors - result, again! However, the call now fails with a 'Network request failed'. According to the TS annotations the format function is deprecated anyway and should probably be updated, but that still won't solve the missing 'url' package. Again, not sure how to overcome this one.

I do have some good news though, I've added 'chalk' to the list of external modules in the rollup config and it's cleared up the 'tty' and 'os' imports.

Hopefully someone else out there has some ideas on how to fix these issues?

wwdrew commented 3 years ago

I've finally had a bit of spare time lately and have spent a bit of time looking into this again and have managed to get it working - though I don't have a solution yet.

I was digging into the createSetupServer code line-by-line and was able to confirm it was working up until the createInterceptor call. Within the resolver, it turns out the mockedRequest isn't being returned by parseIsomorphicRequest. Digging into it even further, it seemed to fail silently when trying set the request cookies. A bit more digging and a try/catch statement unearthed this:

[ReferenceError: Can't find variable: localStorage]

Basically, it appears any time the code tries to work with the cookies, it's trying to access localStorage which doesn't exist in React Native.

I've commented out the cookie set/retrieve code in the shortest path and I've confirmed it works! However, I'm not sure how to proceed with updating the cookie store to work with React Native. My first guess would be to replace the calls with ones to asyncStorage, I'll give it a go to see what happens and come back with the results.

The url issue I outlined above is still a problem, too.

In the meantime, it brought me such joy to see the mocked result appear in my app! Feeling like it's really close to being solved now.

Edit:

Just to confirm, this is using v0.30.1.

So far my results are:

It might be worthwhile submitting a PR to fix the first two issues at the very least. The localStorage issue appears to reside in the @mswjw/cookies package so I'm not entirely sure how to proceed with that one.

wwdrew commented 3 years ago

Actually, it turns out solving the last step was actually really easy.

@mswjs/cookies@0.1.6 has changed how it checks for whether localStorage exists, which is compatible with React Native. After testing it on my local setup (with the two patches I've submitted above) it's now working.

I've submitted a third PR that solves the last hurdle for React Native support.

kettanaito commented 3 years ago

Thank you for the amazing work done with this investigation and preparing the fixes, @wwdrew!

However, I'm not sure how to proceed with updating the cookie store to work with React Native.

There should be no need for that. @mswjs/cookies fallbacks to using a singleton in the case there is no localStorage found. I believe that's the appropriate strategy for React Native applications.

wwdrew commented 3 years ago

Yep, it turns out there's no need to update the cookie store at all. The update to the cookies package is all that was needed.

kettanaito commented 3 years ago

A gigantic thank you to @wwdrew for fixing React Native compatibility issues! The fix has been released in 0.31.0 and should enable React Native support back.

Please, @wwdrew, would you have some time to set up a React Native usage example in the examples repository? That'd help us catch any regressions, as all those examples are also smoke tests for MSW.

wwdrew commented 3 years ago

Absolutely, would be glad to help!

kettanaito commented 3 years ago

Thank you, @wwdrew!

Take a look at the contribution guidelines on adding a new example to the repository. Let me know if you have any questions.

Also, if you don't mind, our Twitter account has mentioned you in the latest release tweet. We appreciate your contribution and would love to collaborate more in the future with you!

jorgemasta commented 3 years ago

Really nice work @wwdrew !!

taylorkline commented 3 years ago

@wwdrew not to step on your toes, but I've opened mswjs/examples#60 with a Rest React Native example.

therynamo commented 2 years ago

Hey there folks - I know this issue is closed - but I have a related issue that brought me here.

If you are sure the module exists, try these steps:
 1. Clear watchman watches: watchman watch-del-all
 2. Delete node_modules and run yarn install
 3. Reset Metro's cache: yarn start --reset-cache
 4. Remove the cache: rm -rf /tmp/metro-*
   7 |  */
   8 |
>  9 | import http from 'node:http';
     |                   ^
  10 | import https from 'node:https';
  11 | import zlib from 'node:zlib';
  12 | import Stream, {PassThrough, pipeline as pump} from 'node:stream';

I went through the RN Example Repo above - and I got tests working great! I then went to start up the app and saw this. I then came across this Stack Overflow answer - and gave it a shot. Downgrading to node-fetch: 2.6.7 worked like a charm.

It made me curious why it wasn't working in Development but worked fine in test. So I started looking at the migration docs that node-fetch put together.

This module was converted to be a ESM only package in version 3.0.0-beta.10. node-fetch is an ESM-only module - you are not able to import it with require. We recommend you stay on v2 which is built with CommonJS unless you use ESM yourself. We will continue to publish critical bug fixes for it.

So clearly there is something with esmodules happening here - where the default preset for our app

  presets: [
    [
      "module:metro-react-native-babel-preset",
      { useTransformReactJSXExperimental: true,  }
    ]

Do not transform the upgraded esm node-fetch in versions 3.0+.

Furthermore - we're ignoring node-fetch in the jest-config which was necessary to get the tests working.

{
    "node_modules/(?!((jest-)?(@react-native|react-native)|react-clone-referenced-element|expo(nent)?|@expo(nent)?/.*|@react-navigation/.*|@sentry|@react-native-community/.*|node-fetch|data-uri-to-buffer|fetch-blob|formdata-polyfill))"
}

In our app - we are not doing anything special with esmodules.

Unfortunately I do not know enough about the module:metro-react-native-babel-preset to know whether or not it should transform these modules or not. And their NPM docs don't offer much help either:

If you get stuck configuring Babel, please ask a question on Stack Overflow or find a consultant for help. If you discover a bug, please open up an issue.

It'd be awesome if someone more familiar with those presets would be able to jump in and verify that nothing extra should have to be done to have esm working in an RN app.


This isn't a blocker anymore as downgrading worked - but its more of a bandaid because node-fetch is on v4 beta now - and v2 support will likely be dropped at some point.