rnx-kit icon indicating copy to clipboard operation
rnx-kit copied to clipboard

metro-resolver bug: resolve logic assumes fromModule name does not have a path separator

Open SlyryD opened this issue 3 years ago • 1 comments

What happened?

metro-resolver unable to resolve package.json browser fields pointing to scoped packages, when using rnx-kit. I opened this bug on metro first, but they threw it over here. The bug and the linked PR here have relevant information: https://github.com/facebook/metro/issues/860

Affected Package

@rnx-kit/metro-resolver-symlinks

Version

0.1.21

Which platforms are you seeing this issue on?

  • [ ] Android
  • [ ] iOS
  • [ ] macOS
  • [X] Windows

System Information

info Fetching system and libraries information...
System:
    OS: Windows 10 10.0.22621
    CPU: (16) x64 Intel(R) Core(TM) i9-9900 CPU @ 3.10GHz
    Memory: 47.81 GB / 63.84 GB
  Binaries:
    Node: 16.14.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.5.0 - C:\Program Files\nodejs\npm.CMD
    Watchman: Not Found
  SDKs:
    Android SDK: Not Found
    Windows SDK: Not Found
  IDEs:
    Android Studio: Not Found
    Visual Studio: 17.3.32901.215 (Visual Studio Community 2022)
  Languages:
    Java: Not Found
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.2 => 17.0.2 
    react-native: 0.68.51-microsoft.0 => 0.68.51-microsoft.0 
    react-native-windows: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to Reproduce

  1. Clone https://github.com/SlyryD/metro-repro
  2. Run yarn at root
  3. Run yarn run bundle at packages/baz

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

SlyryD avatar Oct 17 '22 06:10 SlyryD

Is this the error you're seeing:

error Unable to resolve module util from /~/packages/car/node_modules/@foo/bar/lib/index.js:

None of these files exist:
  * node_modules/@foo/lib/polyfills/util.js(.native|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)
  * node_modules/@foo/lib/polyfills/util.js/index(.native|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)
> 1 | import { promisify } from "util";
    |                            ^
  2 |
  3 | export function printCallback(thing, cb) {
  4 |   console.log(thing, "from printCallback");.
Error: Unable to resolve module util from /~/packages/car/node_modules/@foo/bar/lib/index.js:

I reduced the metro.config.js to:

module.exports = {
  transformer: {
    getTransformOptions: async () => ({
      transform: {
        experimentalImportSupport: true,
        inlineRequires: true,
      },
    }),
  },
};

And copied @foo/bar into packages/car/node_modules and can still reproduce it. It doesn't look like it's related to @rnx-kit/metro-resolver-symlinks at all. In the issue you linked, Rob says:

However, the non-standard object browser field isn't something Metro supports at all.

It's quite clear that the browser field is not supported. Is this an ask to support the browser field?

tido64 avatar Oct 18 '22 08:10 tido64

That's a different bug. The bug was due to me supplying "browser" in "resolverMainFields" and attempting to use it to replace specific files according to this spec (in this case "browser": { "util": "./lib/polyfills/util.js" }). It seems like metro partially recognized this and attempted to resolve the replacement file; however, the resolution failed because the file is in a scoped package; I think it would have worked otherwise!

SlyryD avatar Oct 27 '22 00:10 SlyryD

This field, as defined in the spec, is not supported by Metro, and thus cannot be used with React Native. There are a couple of alternatives:

  1. You can use the remapModule option in @rnx-kit/metro-resolver-symlinks: https://github.com/microsoft/rnx-kit/tree/main/packages/metro-resolver-symlinks#options
    • This should give you the equivalent to path remapping. But the downside is that any consumers of your package will have to copy this config.
  2. Keep the browser field, but also create a platform-forked utility module. This is the standard solution and will work across platforms. On web, Webpack reads util.ts and correctly resolves the browser field. On native, Metro will pick up util.native.ts with the polyfill.
    • Example:
      // src/util.ts
      export { promisify } from "util";
      
      // src/util.native.ts
      export function promisify(fn) {
        ...
      }
      

tido64 avatar Oct 27 '22 07:10 tido64

Closing due to inactivity. If you think this is a mistake, please reply and we'll reopen it.

tido64 avatar Feb 09 '23 13:02 tido64