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

"Unable to resolve module mock-aws-s3" with Expo Router API Routes + bcrypt #32383

Closed
karlhorky opened this issue Oct 27, 2024 · 10 comments
Closed
Labels
needs review Issue is ready to be reviewed by a maintainer Router expo-router

Comments

@karlhorky
Copy link
Contributor

karlhorky commented Oct 27, 2024

Minimal reproducible example

https://github.com/karlhorky/repro-expo-router-api-routes-bcrypt

Which package manager are you using? (Yarn is recommended)

npm

If the issue is web-related, please select the bundler (web.bundler in the app.json)

metro

Summary

We are trying to use bcrypt in Expo Router API Routes and we're running into an error Unable to resolve module mock-aws-s3:

$ npm start

...

Android Bundled 13112ms node_modules/expo-router/entry.js (1137 modules)
λ Bundling failed 1196ms app/hash+api.ts (138 modules)

Metro error: Unable to resolve module mock-aws-s3 from /Users/k/p/repro-expo-router-api-routes-bcrypt/node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js: mock-aws-s3 could not be found within the project or in these directories:
  node_modules/@mapbox/node-pre-gyp/node_modules
  node_modules
  41 |     // here we're mocking. node_pre_gyp_mock_s3 is the scratch directory
  42 |     // for the mock code.
> 43 |     const AWSMock = require('mock-aws-s3');
     |                              ^
  44 |     const os = require('os');
  45 |
  46 |     AWSMock.config.basePath = `${os.tmpdir()}/mock`;

 ERROR  [SyntaxError: JSON Parse error: Unexpected character: <]

Screenshot 2024-10-27 at 15 07 02

Adding the dependencies mock-aws-s3, aws-sdk, nock which are required by node-pre-gyp (PR) allows the bundling to succeed but this leads to a further error at runtime, because node-pre-gyp looks for bcrypt's package.json at runtime:

Android Bundled 1326ms node_modules/expo-router/entry.js (1134 modules)
λ Bundled 3977ms app/hash+api.ts (1607 modules)
λ  WARN  Require cycle: node_modules/@mapbox/node-pre-gyp/node_modules/semver/classes/comparator.js -> node_modules/@mapbox/node-pre-gyp/node_modules/semver/classes/range.js -> node_modules/@mapbox/node-pre-gyp/node_modules/semver/classes/comparator.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle. 
  factory (node_modules/@mapbox/node-pre-gyp/node_modules/semver/classes/range.js:219:20)
  factory (node_modules/@mapbox/node-pre-gyp/node_modules/semver/classes/comparator.js:141:15)

Metro error: /Users/k/p/repro-expo-router-api-routes-bcrypt/app/package.jsondoes not exist

  17 | exports.find = function(package_json_path, opts) {
  18 |   if (!existsSync(package_json_path)) {
> 19 |     throw new Error(package_json_path + 'does not exist');
     |           ^
  20 |   }
  21 |   const prog = new npg.Run({ package_json_path, argv: process.argv });
  22 |   prog.setBinaryHostProperty();

Call Stack
  Object.find (node_modules/@mapbox/node-pre-gyp/lib/pre-binding.js:19:11)
  factory (node_modules/bcrypt/bcrypt.js:5:31)
  loadModuleImplementation (node_modules/metro-runtime/src/polyfills/require.js:277:5)
  guardedLoadModule (node_modules/metro-runtime/src/polyfills/require.js:184:12)
  require (node_modules/metro-runtime/src/polyfills/require.js:92:7)
  factory (app/hash+api.ts:1)
  loadModuleImplementation (node_modules/metro-runtime/src/polyfills/require.js:277:5)
  guardedLoadModule (node_modules/metro-runtime/src/polyfills/require.js:177:21)
  metroRequire (node_modules/metro-runtime/src/polyfills/require.js:92:7)
  Object.<anonymous> (/Users/k/p/repro-expo-router-api-routes-bcrypt/app/hash+api.ts.bundle:919731:18)
 ERROR  [SyntaxError: JSON Parse error: Unexpected character: <]

Cause of Error

The Unable to resolve module mock-aws-s3 and .../package.json does not exist errors originates from bcrypt's dependency node-pre-gyp, caused by node-pre-gyp not supporting bundlers:

Recommendation: "externals"

The recommendations in related bcrypt discussions is to exclude these modules from the bundle (using a technique often called "externals"):

Unclear how to add "externals" in Metro

But I can't see how I can modify the Metro configuration for Expo Router API Routes, to add something similar to "externals".

Looking through the Expo source code, I can see this file, which has Metro options, but not sure how to alter them from userland:

const opts: ExpoMetroOptions = {
// TODO: Possibly issues with using an absolute path here...
mainModuleName: convertPathToModuleSpecifier(filePath),
lazy: false,
asyncRoutes: false,
inlineSourceMap: false,
engine: 'hermes',
minify: false,
bytecode: false,
// Bundle in Node.js mode for SSR unless RSC is enabled.
environment: this.isReactServerComponentsEnabled ? 'react-server' : 'node',
platform: 'web',
mode: 'development',
//
...this.instanceMetroOptions,
// Mostly disable compiler in SSR bundles.
reactCompiler: false,
baseUrl,
routerRoot,
isExporting,
...specificOptions,
};

Reproduction

Creation steps:

  1. mkdir repro-expo-router-api-routes-bcrypt && cd repro-expo-router-api-routes-bcrypt
  2. npx create-expo-app@latest .
  3. rm -r ./node_modules && npm install (for EMFILE error)
  4. Remove extra files
  5. Configure app.json with expo.web.output = "server" and expo.plugins[0][1] = { "origin": "http://localhost:8081" }
  6. Create a file app/hash+api.ts, with an Expo Router API Route, using hashSync from bcrypt
  7. Inside app/(tabs)/index.tsx, add a fetch() of the API Route inside useFocusEffect
  8. 💥 npm start and observe the error after API Route bundling

Environment

expo-env-info 1.2.0 environment info:
    System:
      OS: macOS 15.0.1
      Shell: 5.9 - /bin/zsh
    Binaries:
      Node: 20.18.0 - /opt/homebrew/bin/node
      Yarn: 1.22.22 - /opt/homebrew/bin/yarn
      npm: 10.8.2 - /opt/homebrew/bin/npm
    Managers:
      CocoaPods: 1.11.3 - /usr/local/bin/pod
    SDKs:
      iOS SDK:
        Platforms: DriverKit 24.0, iOS 18.0, macOS 15.0, tvOS 18.0, visionOS 2.0, watchOS 11.0
    IDEs:
      Android Studio: 2021.1 AI-211.7628.21.2111.8309675
      Xcode: 16.0/16A242d - /usr/bin/xcodebuild
    npmPackages:
      expo: ~51.0.28 => 51.0.38
      expo-router: ~3.5.23 => 3.5.23
      react: 18.2.0 => 18.2.0
      react-dom: 18.2.0 => 18.2.0
      react-native: 0.74.5 => 0.74.5
      react-native-web: ~0.19.10 => 0.19.13
    Expo Workflow: managed
@karlhorky karlhorky added needs validation Issue needs to be validated Router expo-router labels Oct 27, 2024
@expo-bot expo-bot added needs review Issue is ready to be reviewed by a maintainer and removed needs validation Issue needs to be validated labels Oct 27, 2024
@karlhorky
Copy link
Contributor Author

karlhorky commented Oct 27, 2024

Potentially relevant background from @EvanBacon on Expo Router API Routes not supporting all Node.js packages:

#32350 (comment)

This is partially mentioned in the docs under the known limitations. docs.expo.dev/router/reference/api-routes#no-dynamic-imports

Our immediate goal is to support workerd and Cloudflare runtimes as opposed to the entire Node.js/Lambda runtime.

@karlhorky
Copy link
Contributor Author

Workaround (use bcryptjs instead of bcrypt)

Instead of bcrypt, use bcryptjs (GitHub repo: https://github.com/dcodeIO/bcrypt.js), which is a JavaScript implementation of bcrypt instead of a native implementation (slower):

Pull request: karlhorky/repro-expo-router-api-routes-bcrypt#2

npm remove bcrypt
npm add bcryptjs
-import { hashSync } from 'bcrypt';
+import { hashSync } from 'bcryptjs';

export function GET(request: Request) {
  const hash = hashSync('hello', 10);
  return Response.json({ hash: hash });
}

Results:

Android Bundled 1169ms node_modules/expo-router/entry.js (1090 modules)
λ Bundled 946ms app/hash+api.ts (3 modules)
 LOG  {"hash": "$2a$10$h8yYtmr.E866KjgKGlNf4.i/mBhIPawhUwB4AwNcAM5rgyw5fS/3u"}

Screenshot 2024-10-27 at 16 28 34

@marklawlor
Copy link
Contributor

I'm going to close this issue and comment here facebook/metro#1377

@karlhorky
Copy link
Contributor Author

karlhorky commented Oct 31, 2024

@marklawlor facebook/metro#1377 is for a slightly different package actually - @node-rs/bcrypt instead of bcrypt - which does not have the same issues with @mapbox/node-pre-gyp.

But if you think I should open a separate issue in Metro for bcrypt itself, then happy to do that too.

@marklawlor
Copy link
Contributor

marklawlor commented Nov 3, 2024

As I explained that comment, this is an issue with how bcrypt is implemented, as it uses non-standard behaviour. This isn't an issue with Metro or Expo.

@karlhorky
Copy link
Contributor Author

karlhorky commented Nov 4, 2024

I think I understand your general sentiment, but these assertions are not entirely true:

  1. 1377 is *not* about bcrypt, it's about a different package called @node-rs/bcrypt which doesn't have the issues you mentioned
  2. Metro lacks native Node.js compat and externals features, so both @node-rs/bcrypt and bcrypt are good examples of popular packages which users will have use cases to be supported

Because Expo Router contains Metro configuration, this also is relevant for Expo.

@marklawlor
Copy link
Contributor

I understand, thats why I marked my comment in 1377 as off-topic. In regards to this issue, the comment about bcyrpt still stands. bcrypt depends on a module that is implemented in a way that bundler's cannot perform static analysis on their runtime dependencies. That is not an issue with Expo's Metro config.

@karlhorky
Copy link
Contributor Author

karlhorky commented Nov 4, 2024

Got it - mostly agreed here.

One last nit from my side would be that in the future (once Metro has support for native Node.js addons), the issues I reported with both @node-rs/bcrypt and bcrypt will potentially represent an issue in Expo's Metro config, for example when either of these occur:

  • @node-rs/bcrypt: When Metro supports native Node.js addons (if any additional Metro config is needed)
  • bcrypt: When Metro supports "externals" (additional Metro config will most likely be needed)

@marklawlor
Copy link
Contributor

marklawlor commented Nov 4, 2024

@karlhorky Your comments are mixing up between bcrypt and @node-rs/bcrypt. These are two seperate issues and its confusing to talk about them in the same issue.

For the issue with bcrypt, I'm keeping this as "won't fix" as:

  • There is a working workaround with bcryptjs
  • When Metro does support externals, you don't need Expo to update its config for it to work. You can just add this yourself in metro.config.js.

Since Metro does not support externals right now, whether Expo should or shouldn't add it to the config is not a "issue", rather a feature request that should be posted on our feature request board. https://expo.canny.io/

@karlhorky
Copy link
Contributor Author

karlhorky commented Nov 4, 2024

Your comments are mixing up between bcrypt and @node-rs/bcrypt. These are two seperate issues and its confusing to talk about them in the same issue.

Yeah, sorry about that. I'll try to keep them separated into their separate issues.

  • When Metro does support externals, you don't need Expo to update its config for it to work. You can just add this yourself in metro.config.js.

Ah interesting, wasn't aware that the modifying Metro config in metro.config.js also applied to the Expo Router API Routes, nice! 👀

Maybe useful for us at some point...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Issue is ready to be reviewed by a maintainer Router expo-router
Projects
None yet
Development

No branches or pull requests

3 participants