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

React Native support is broken #622

Closed
wwdrew opened this issue Mar 1, 2021 · 15 comments
Closed

React Native support is broken #622

wwdrew opened this issue Mar 1, 2021 · 15 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed scope:react-native

Comments

@wwdrew
Copy link
Contributor

wwdrew commented Mar 1, 2021

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 wwdrew added bug Something isn't working scope:browser Related to MSW running in a browser labels Mar 1, 2021
@kettanaito kettanaito added scope:react-native and removed scope:browser Related to MSW running in a browser labels Mar 1, 2021
@wwdrew
Copy link
Contributor Author

wwdrew commented Mar 5, 2021

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
Copy link
Member

kettanaito commented Mar 5, 2021

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:

msw/rollup.config.ts

Lines 121 to 155 in 10cccc0

const buildNative = {
input: 'src/native/index.ts',
external: [
'tty',
'os',
'util',
'events',
'node-request-interceptor',
/**
* Exclude Node.js request interceptors from being compiled.
* @see https://github.com/mswjs/node-request-interceptor/issues/52
*/
'node-request-interceptor/lib/interceptors/ClientRequest',
'node-request-interceptor/lib/interceptors/XMLHttpRequest',
],
output: {
file: 'native/lib/index.js',
format: 'cjs',
},
plugins: [
json(),
resolve({
browser: false,
preferBuiltins: true,
extensions: ['.js', '.jsx', '.ts', '.tsx'],
}),
typescript({
useTsconfigDeclarationDir: true,
tsconfigOverride: {
outDir: './native/lib',
},
}),
commonjs(),
],
}

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
Copy link
Contributor Author

wwdrew commented Mar 5, 2021

@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
Copy link
Contributor Author

wwdrew commented Mar 10, 2021

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
Copy link
Contributor Author

wwdrew commented Mar 10, 2021

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?

@kettanaito kettanaito added the help wanted Extra attention is needed label Mar 24, 2021
@wwdrew
Copy link
Contributor Author

wwdrew commented Jul 8, 2021

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:

  • os/tty import error is solved by adding chalk to rollup.config.ts
  • url error still exists (though I've experimented with replacing it using the WHATWG URL spec and it fixes the issue)
  • Any interaction using localStorage for the cookie store breaks RN

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
Copy link
Contributor Author

wwdrew commented Jul 8, 2021

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

@mswjs/[email protected] 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
Copy link
Member

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
Copy link
Contributor Author

wwdrew commented Jul 9, 2021

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
Copy link
Member

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
Copy link
Contributor Author

wwdrew commented Jul 9, 2021

Absolutely, would be glad to help!

@kettanaito
Copy link
Member

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
Copy link

Really nice work @wwdrew !!

@taylorkline
Copy link

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

@therynamo
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed scope:react-native
Projects
None yet
Development

No branches or pull requests

5 participants