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

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

Closed
aleclarson opened this issue Nov 12, 2019 · 3 comments

Comments

@aleclarson
Copy link
Contributor

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.

  • metro@0.57.0
  • The rest are irrelevant
@joe06102
Copy link

joe06102 commented Jun 4, 2021

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

@orlando
Copy link

orlando commented Nov 11, 2022

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

@huntie
Copy link
Member

huntie commented Nov 27, 2022

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 huntie closed this as completed Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants