metro icon indicating copy to clipboard operation
metro copied to clipboard

Metro neglects to check ".native.js" before ".js" when "main" ends with ".js" already

Open aleclarson opened this issue 6 years ago • 1 comments

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

What is the current behavior? When main in package.json ends with an extension (eg: .js), Metro inadvertently looks for {filePathPrefix}.js before {filePathPrefix}.native.js because its resolution algorithm is incorrect.

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.

Since there's an easy work around (exclude the extension), I can't spend time setting up a repo, but here's instructions for testing.

git clone https://github.com/alloc/react-layout-effect

# Reset to a commit where "main" ends with .js
git reset --hard 1b00d93ab956f8af8fc1fdb65dffa4ab42579f43

# Then use react-layout-effect in a RN bundle, 
# and notice how "lib/useLayoutEffect.native.js" is NOT used
???

# Reset to a commit where "main" does NOT end with any extension
git reset --hard origin/master

# Then try using it again, and notice how "lib/useLayoutEffect.native.js" is used.
???

What is the expected behavior? Check .native.js before .js when preferNativePlatform is true and main ends with .js.

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

aleclarson avatar Nov 12 '19 21:11 aleclarson

hi, are there any workarounds to the problem? it still exists in 0.63

joe06102 avatar Jun 04 '21 09:06 joe06102

Got hit by this today. Do we have the related code where to apply the fix? Would love to give it a try

orlando avatar Nov 11 '22 16:11 orlando

I'm not certain whether this is a bug or is intended behaviour (in the context of package entry points) in order to respect the exact module path defined in a package's "main" field.

{
  "name": "react-layout-effect",
  "version": "1.0.5",
  "main": "dist/cjs/useLayoutEffect", // -> dist/cjs/useLayoutEffect.js ✅

Looking forward, when we support the "exports" field, this will be explicitly the intended behaviour (if a module path is provided and is present, it will be resolved as defined in package.json).

Based on this, and given we provide the "react-native" root field pattern (which is widely used), I imagine it isn't worth making a breaking change to this piece of resolution logic this late in the game.

Workaround for projects

Resolution of this import path under Metro can be overridden by configuring resolveRequest in metro.config.js.

resolveRequest: (context, moduleName, platform) => {
  // If needed, `&& platform !== 'web'`
  if (moduleName === 'react-layout-effect') {
    return {
      filePath: './node_modules/react-layout-effect/dist/cjs/useLayoutEffect.native.js',
      type: 'sourceFile',
    };
  }

  return context.resolveRequest(context, moduleName, platform);
},

Fix for packages

We recommend using the top-level "react-native" field for packages to specify alternate entry points for React Native.

Please raise an issue/PR to the package maintainer(s) that this pattern should be used instead of .native.js when the entry point module requires platform-specific resolution.

  {
    "name": "react-layout-effect",
    "version": "1.0.5",
    "main": "dist/cjs/useLayoutEffect",
    "module": "dist/esm/useLayoutEffect",
+   "react-native": "dist/cjs/useLayoutEffectNative",

huntie avatar Nov 27 '22 14:11 huntie