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

[bug] - Pnpm not working with expo #4286

Open
Olyno opened this issue Jan 27, 2022 · 16 comments
Open

[bug] - Pnpm not working with expo #4286

Olyno opened this issue Jan 27, 2022 · 16 comments

Comments

@Olyno
Copy link

Olyno commented Jan 27, 2022

pnpm version:

6.28.0

Code to reproduce the issue:

expo init pnpm_expo # Choose Typescript tab template
cd pnpm_expo
rm -rf node_modules yarn.lock
pnpm i
pnpm start
# Go to the expo app, and try it

Expected behavior:

Be able to run an expo app without any issue.

Actual behavior:

Error: EISDIR: illegal operation on a directory, read
    at Object.readSync (node:fs:723:3)
    at tryReadSync (node:fs:433:20)
    at Object.readFileSync (node:fs:479:19)
    at UnableToResolveError.buildCodeFrameMessage (/home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:347:17)
    at new UnableToResolveError (/home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:333:35)
    at ModuleResolver.resolveDependency (/home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:211:15)
    at DependencyGraph.resolveDependency (/home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/node-haste/DependencyGraph.js:413:43)
    at /home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/lib/transformHelpers.js:317:42
    at /home/olyno/Documents/projects/pnpm_expo/node_modules/.pnpm/metro@0.64.0/node_modules/metro/src/Server.js:1471:14
    at Generator.next (<anonymous>)
› Stopped server

Additional information:

  • node -v prints: 16.13.2
  • Windows, macOS, or Linux?: Linux, Deepin
@zkochan
Copy link
Member

zkochan commented Jan 27, 2022

Try with the node-linker=hoisted option https://pnpm.io/npmrc#node-linker

@Olyno
Copy link
Author

Olyno commented Jan 27, 2022

It works this way, but isn't it contrary to the principle of operation of pnpm? The purpose of pnpm is to use symlinks to reduce the size of node_modules, isn't it?

@zkochan
Copy link
Member

zkochan commented Jan 27, 2022

Right. But we can't fix issues in third party libs. The global store is still used. Even with hoisted linker

@Olyno
Copy link
Author

Olyno commented Jan 27, 2022

The global store is still used. Even with hoisted linker

This is interesting!
Also, do we know why some libs are not compatible with pnpm? Maybe it could have a way to fix them?

@rhyek
Copy link

rhyek commented Feb 6, 2022

Can someone explain further? When using node-linker=hoisted in root/.npmrc and running pnpm i in the worspace root, expo files are not found in the react native app's node_modules, but rather only in the root:

image

Running the app this way immediately complains about not finding node_modules/expo/AppEntry.js.

Expo: 44
PNPM: 6.29.1
OS: MacOS Monterey

@rhyek
Copy link

rhyek commented Feb 6, 2022

Relevant expo/expo#16104.

@byCedric
Copy link

byCedric commented Feb 9, 2022

That's not an issue related to pnpm, or Expo. It's just how Metro explains it can't find packages. I got it working with the --shamefully-hoist flag, and going over the steps in Working with Monorepos, in case it helps. byCedric/expo-monorepo-example#19

@rhyek
Copy link

rhyek commented Feb 9, 2022

Thanks. I actually looked into that just now and these are the specific changes necessary for pnpm:
rhyek/expo-monorepo-issue@pnpm...pnpm-fixed

@karlhorky
Copy link

karlhorky commented Jan 27, 2024

@byCedric now that Expo SDK 50 uses React Native 0.73 (which made the Metro symlinks support stable, which has been reported as working by the community without Expo), would it be considered a bug in Expo for pnpm support to not work out of the box (without node-linker=hoisted or any other workarounds) with a new create-expo app?

The way I understand it, the work involved on Expo's side for supporting symlinks would be refactoring the following:

  • relative paths from files within node_modules
  • any other code that relies on a fixed node_modules structure

Would be great to be able to use Expo + the new Metro symlink support without the node-linker=hoisted workaround!

@karlhorky
Copy link

@zkochan alternatively, if Expo will not do the work above to support symlinks, would pnpm consider adopting into core some public-hoist-pattern[] hoisting exceptions config for Expo and related packages which rely on a fixed node_modules directory structure?

Some users have reported public-hoist-pattern[] configurations working for them:

eg. the following (not tested as working yet)

public-hoist-pattern[]=*expo*
public-hoist-pattern[]=*react-native-gradle-plugin*
public-hoist-pattern[]=*@react-native-community/cli-platform-android*
public-hoist-pattern[]=*@react-native-community/cli-platform-ios*

I know that pnpm already has some public-hoist-pattern[] configuration in core for some other popular packages in the ecosystem which also rely on node_modules directory structure - so I'm asking whether pnpm would also consider this for Expo, if they will not do the work to refactor.

@wesleyfreit
Copy link

wesleyfreit commented Mar 15, 2024

Yes, @karlhorky is correctly, my project is working good using public-hoist-pattern[] pnpm configuration in .npmrc and other configurations in metro.config.js.

I'm using a default expo project created inside the monorepo using turborepo with pnpm.

My .npmrc:

public-hoist-pattern[] = @babel/*
public-hoist-pattern[] = *eslint* // if you use eslint
public-hoist-pattern[] = *prettier* // if you use prettier

In my expo project, I have the @rnx-kit/metro-resolver-symlinks package installed.

And my metro.config.js:

const { getDefaultConfig } = require('expo/metro-config');
const MetroSymlinksResolver = require('@rnx-kit/metro-resolver-symlinks');
const path = require('path');

// Find the workspace root
const workspaceRoot = path.resolve(__dirname, '../..');
const projectRoot = __dirname;

const config = getDefaultConfig(projectRoot);

// Watch all files within the monorepo
config.watchFolders = [workspaceRoot];

// Let Metro know where to resolve packages, and in what order
config.resolver.nodeModulesPaths = [
  path.resolve(projectRoot, 'node_modules'),
  path.resolve(workspaceRoot, 'node_modules'),
];

// Force Metro to resolve (sub)dependencies only from the `nodeModulesPaths`
config.resolver.disableHierarchicalLookup = true;

// Using the Metro Symlinks Resolver config of the `rnx-kit`
config.resolver.resolveRequest = MetroSymlinksResolver();

module.exports = config;

In my project, the typescript path mapping configuration (ex: @/), did not work with the described configuration, so I had to remove it. Maybe I should add another public pattern for typescript, but I preferred not to add it.

@watadarkstar
Copy link

watadarkstar commented Mar 26, 2024

@wesleyfreit Thanks for the above! It worked well for me with pnpm and SDK 50. I was having issues after upgrading but with your changes everything works well now.

Here is my config for anyone who wants to use Expo and pnpm without a monorepo:

.npmrc

save-exact=true
# node-linker=hoisted is needed for pnpm to work with Expo see:
# https://github.com/nrwl/nx-labs/issues/34#issuecomment-1090033598
node-linker=hoisted
strict-peer-dependencies=false
# https://github.com/pnpm/pnpm/issues/4286#issuecomment-1999957869
public-hoist-pattern[] = @babel/*
public-hoist-pattern[] = *eslint* // if you use eslint
public-hoist-pattern[] = *prettier* // if you use prettier

metro.config.js

// Learn more https://docs.expo.io/guides/customizing-metro
// https://github.com/pnpm/pnpm/issues/4286#issuecomment-1999957869
const { getDefaultConfig } = require('expo/metro-config');
const MetroSymlinksResolver = require("@rnx-kit/metro-resolver-symlinks");
const path = require('path');

const projectRoot = __dirname;

/** @type {import('expo/metro-config').MetroConfig} */
const config = getDefaultConfig(__dirname);

// Let Metro know where to resolve packages, and in what order
config.resolver.nodeModulesPaths = [
    path.resolve(projectRoot, 'node_modules'),
  ];

// Force Metro to resolve (sub)dependencies only from the `nodeModulesPaths`
config.resolver.disableHierarchicalLookup = true;

// Using the Metro Symlinks Resolver config of the `rnx-kit`
config.resolver.resolveRequest = MetroSymlinksResolver();

module.exports = config;

@karlhorky
Copy link

karlhorky commented Mar 26, 2024

@wesleyfreit @watadarkstar these two configurations are not following my suggested public-hoist-pattern[]= config above:

  1. Your config above includes Babel, ESLint and Prettier dependencies, which are unrelated to React Native / Expo hoisting problems
  2. Your config above uses node-linker=hoisted, which is a workaround for the issue and avoids the problem which should actually be fixed by Expo / React Native / pnpm (description on option: "A React Native project will most probably only work if you use a hoisted node_modules.")

The point of issues like this are to try to move past the historical limitation of React Native to not work with symlinks and pnpm's default isolated linker, now that Expo SDK 50 uses React Native 0.73, which made the Metro symlinks support stable:

@watadarkstar
Copy link

watadarkstar commented Mar 26, 2024

@karlhorky I tried it without the node-linker=hoisted and I get some weird errors:

simulator_screenshot_EFEB70BA-C4F4-4BDA-B227-E2C82E744969

simulator_screenshot_7407106F-E700-4556-893D-6DA85EECD488

Maybe I need to delete my pnpm-lock.yaml? I ran a clean which is:

"clean": "rm -rf node_modules && pnpm store prune && pnpm install && watchman watch-del-all && rm -fr $TMPDIR/haste-map-* && rm -rf $TMPDIR/metro-cache && pnpm types:generate",
    "clean:ios": "rm -rf ios && pnpm clean",

@watadarkstar
Copy link

@wesleyfreit @watadarkstar these two configurations are not following my suggested public-hoist-pattern[]= config above:

  1. Your config above includes Babel, ESLint and Prettier dependencies, which are unrelated to React Native / Expo hoisting problems
  2. Your config above uses node-linker=hoisted, which is a workaround for the issue and avoids the problem which should actually be fixed by Expo / React Native / pnpm (description on option: "A React Native project will most probably only work if you use a hoisted node_modules.")

The point of issues like this are to try to move past the historical limitation of React Native to not work with symlinks and pnpm's default isolated linker, now that Expo SDK 50 uses React Native 0.73, which made the Metro symlinks support stable:

You are correct. I had node-linker=hoisted so it was irrelevant what I had for public-hoist-pattern[] because the former was a catch all.

Nonetheless if I get rid of node-linker=hoisted I still get errors so maybe there were more packages that needed to have public-hoist-pattern[].

Anyway, I actually prefer this since it catches all packages even though its not super efficient:

.npmrc

save-exact=true
# node-linker=hoisted is needed for pnpm to work with Expo see:
# https://github.com/nrwl/nx-labs/issues/34#issuecomment-1090033598
node-linker=hoisted
strict-peer-dependencies=false

metro.config.js

// Learn more https://docs.expo.io/guides/customizing-metro
// https://github.com/pnpm/pnpm/issues/4286#issuecomment-1999957869
const { getDefaultConfig } = require("expo/metro-config")
const MetroSymlinksResolver = require("@rnx-kit/metro-resolver-symlinks")
const path = require("path")

const projectRoot = __dirname

/** @type {import('expo/metro-config').MetroConfig} */
const config = getDefaultConfig(__dirname)

// Let Metro know where to resolve packages, and in what order
config.resolver.nodeModulesPaths = [path.resolve(projectRoot, "node_modules")]

// Force Metro to resolve (sub)dependencies only from the `nodeModulesPaths`
config.resolver.disableHierarchicalLookup = true

// Using the Metro Symlinks Resolver config of the `rnx-kit`
config.resolver.resolveRequest = MetroSymlinksResolver()

module.exports = config

@LouisHaftmann
Copy link

LouisHaftmann commented May 2, 2024

I'm working in a pnpm monorepo and running the app on iOS worked for me with just shamefully-hoist=true but it would always fail on Android. My fix was adding a custom resolveRequest function to metro.config.js:

config.resolver.resolveRequest = (context, moduleName, platform) => {
  if (
    platform === 'android' &&
    // replace `packages/app` with the path to your app directory in monorepo
    context.originModulePath.endsWith('packages/app/.') &&
    // some weird edge cases, there might be a better way to do this
    !moduleName.startsWith('/') &&
    !moduleName.startsWith('../../') &&
    !moduleName.startsWith('./../../')
  ) {
    // just tell metro to look in the root of the monorepo where pnpm stores all node_modules
    moduleName = `../../${moduleName}`
  }

  // use default
  return context.resolveRequest(context, moduleName, platform)
}

This is a bit hacky but it works great in a pnpm workspaces monorepo with just shamefully-hoist=true (no other npm options needed).

metro.config.js:

const path = require('node:path')

const { getDefaultConfig } = require('expo/metro-config')

const workspaceRoot = path.resolve(__dirname, '../..')
const projectRoot = __dirname

const config = getDefaultConfig(projectRoot)

config.watchFolders = [workspaceRoot]

config.resolver.nodeModulesPaths = [
  path.resolve(projectRoot, 'node_modules'),
  path.resolve(workspaceRoot, 'node_modules'),
]

config.resolver.disableHierarchicalLookup = true
config.resolver.sourceExts.push('cjs')

config.resolver.resolveRequest = (context, moduleName, platform) => {
  if (
    platform === 'android' &&
    // replace `packages/app` with the path to your app directory in monorepo
    context.originModulePath.endsWith('packages/app/.') &&
    // some weird edge cases, there might be a better way to do this
    !moduleName.startsWith('/') &&
    !moduleName.startsWith('../../') &&
    !moduleName.startsWith('./../../')
  ) {
    // just tell metro to look in the root of the monorepo where pnpm stores all node_modules
    moduleName = `../../${moduleName}`
  }

  // use default
  return context.resolveRequest(context, moduleName, platform)
}

module.exports = config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants