facebook / metro

🚇 The JavaScript bundler for React Native
https://metrobundler.dev
MIT License
5.24k stars 626 forks source link

Importing Node-API .node modules in Expo Router API Routes? #1377

Open karlhorky opened 3 weeks ago

karlhorky commented 3 weeks ago

Companion issue (expo/expo): https://github.com/expo/expo/issues/32350

Do you want to request a feature or report a bug?

Request a feature:

Documentation about Node-API native addons usage in Metro with platform: 'web', which Expo Router API Routes uses.

What is the current behavior?

Metro does not allow for importing Node-API native addons in platform: 'web' code, meaning that npm package containing native addons (such as @node-rs/bcrypt) cannot be used in Expo Router API Routes.

For example:

app/hash+api.ts

import { hashSync } from '@node-rs/bcrypt-darwin-arm64';

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

Out of the box, Expo Router API Routes will not be able to bundle this, because the .node extension is not supported (as also mentioned by @EvanBacon here):

Android Bundled 10400ms node_modules/expo-router/entry.js (1132 modules)
λ Bundling failed 16ms app/hash+api.ts (1 module)

Metro error: Failed to bundle API Route: hash+api.ts

While trying to resolve module `@node-rs/bcrypt-darwin-arm64` from file `/Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/app/hash+api.ts`, the package `/Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/node_modules/@node-rs/bcrypt-darwin-arm64/package.json` was successfully found. However, this package itself specifies a `main` module field that could not be resolved (`/Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/node_modules/@node-rs/bcrypt-darwin-arm64/bcrypt.darwin-arm64.node`. Indeed, none of these files exist:

  * /Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/node_modules/@node-rs/bcrypt-darwin-arm64/bcrypt.darwin-arm64.node(.web.ts|.ts|.web.tsx|.tsx|.web.js|.js|.web.jsx|.jsx|.web.json|.json|.web.cjs|.cjs|.web.mjs|.mjs|.web.scss|.scss|.web.sass|.sass|.web.css|.css)
  * /Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/node_modules/@node-rs/bcrypt-darwin-arm64/bcrypt.darwin-arm64.node/index(.web.ts|.ts|.web.tsx|.tsx|.web.js|.js|.web.jsx|.jsx|.web.json|.json|.web.cjs|.cjs|.web.mjs|.mjs|.web.scss|.scss|.web.sass|.sass|.web.css|.css)

Changing sourceExts to also contain .node (adding sourceExts.push('node') in node_modules/@expo/metro-config/build/ExpoMetroConfig.js) yields the following (trying to load the .node binary file):

Android Bundled 11972ms node_modules/expo-router/entry.js (1132 modules)
λ Bundling failed 191ms app/hash+api.ts (3 modules)

Metro error: Failed to bundle API Route: hash+api.ts

SyntaxError: /Users/k/p/repro-expo-router-api-routes-node-rs-bcrypt-sha1-not-computed/node_modules/@node-rs/bcrypt-darwin-arm64/bcrypt.darwin-arm64.node: Unexpected character '�'. (1:0)

> 1 | ����
          ���__TEX__text__TEXT�?��?�__stubs__TEXT�����
                                                      __stub_helper__TEXT������__const__TEXT���`��__gcc_except_tab__TEXT�8p�8__cstring__TEXTV*
                                                                                                                                              V__unwind_info__TEXTDb�Db__eh_frame__TEXT@z��@z
                          h8__DATA_CONST@@__got__DATA_CONST0h__mod_init_func__DATA_CONST0X0     __const__DATA_CONST��4�x__DATA@�@@__la_symbol_ptr__DATA@@@n__data__DATA@C�@C__thread_vars__DATA�Z��Z__thread_data__DATA�[(�[__thread_bss__DATA�[A�[__bss__DATA@\�&__common__DATA�
    | ^
  2 | �H__LINKEDIT�
  3 | @�x2
  4 | �/Users/runner/work/node-rs/node-rs/target/aarch64-apple-darwin/release/deps/libnode_rs_bcrypt.dylib"�0����x��    ��(��p���
                                                                                                                                 Pn���[�k@�q>����f"�(2 

                                                                                                                                                       *
                                                                                                                                                        8/usr/lib/libiconv.2.dylib
               8xA/usr/lib/libSystem.B.dylib&��)���������o

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

Unclear how to add "externals" in Metro

Thinking through this, one idea came up to use something similar to "externals" - code that Metro will not touch / bundle, but rather just load at runtime.

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

Maybe this feature doesn't exist in Metro?

If such a feature were to exist, this would also help with usage of the standard bcrypt package in Expo Router API Routes:

Metro Configuration

I believe this is the Metro configuration for Expo Router API Routes:

https://github.com/expo/expo/blob/48d64e9c285871e513f11238933e4abe7d7d1d9a/packages/%40expo/cli/src/start/server/metro/MetroBundlerDevServer.ts#L554-L577

 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, 
 }; 

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

I'm not sure this is a bug, but just a gap in the documentation - that Node-API native addons are not intended to be supported in the platform: 'web' configuration.

What is the expected behavior?

It's documented (and a workaround is provided, if possible)

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

robhogan commented 3 weeks ago

Hi @karlhorky - I'm not certain of all the details for how bundling is set up for Expo server-side routes - but at first glance I'd try assetExts and not sourceExts for .node files. Sources will be run through the transformer, which will choke on binary files.

If that doesn't help I'll give this a closer look later in the week.

(Generalised externals support is planned, but not there yet.)

marklawlor commented 3 weeks ago

bcrypt has a dependency on @mapbox/node-pre-gyp, which is trying to import the problem module. There are two issues with this package

  1. It lists mock-aws-s3 as a development dependency, but tries to require it in production code
  2. The guard around the require('mock-aws-s3') is an if () statement checking for an env value. From a bundler perspective, it does not know if a random env value is a flag for production or not. Env values can be changed at runtime.

Metro does have the means to handle this, but the require() needs to be wrapped in the try {} catch {} block.

karlhorky commented 3 weeks ago

@marklawlor This issue https://github.com/facebook/metro/issues/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.

So the notes about @mapbox/node-pre-gyp, mock-aws-s3, etc. actually don't belong in this issue.

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