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

Pass custom-placed metro.config.js through --config to run-android and run-ios commands #1371

Closed
ElviraBurchik opened this issue Mar 6, 2021 · 15 comments
Labels
bug Something isn't working stale

Comments

@ElviraBurchik
Copy link

Describe the Feature

At Shopify, we are using the React Native CLI in a monorepo setup:

react-native-packages/
  node_modules/
  fixture/
      node_modules/
      metro.config.js
      android/
      ...
  packages/
  ...

Since the CLI launches the packager taking the directory where React Native lives as the working directory, it causes the packager command to try reading the Metro configuration from the wrong directory - react-native-packages/node_modules/react-native.

I suppose that it could be solved by adding an option to provide a path to the custom metro.config.js when running run-ios and run-android commands.

Possible Implementations

I spent some time understanding the code base and find some possible implementations, but I struggle to find a place where Metro CLI actually gets called.

How I understand the process so far: runAndroid command runs launchPackager.command to start metro server. launchPackager.command calls react-native-community/cli run, and from here I can't connect the dots how we run Metro CLI and start the server.

I'd be happy to submit a pr for this feature request if somebody could point me out the right place for introducing these changes 😄🤞

Related Issues

facebook/metro#588
#1358

@ElviraBurchik
Copy link
Author

@grabbou moving the discussion from this pr here since we are going to implement it withing this issue :)

Right now, we obtain Metro configuration programatically here https://github.com/react-native-community/cli/blob/master/packages/cli/src/tools/loadMetroConfig.ts#L152 and we set cwd to root from Config. By design, Metro will look for a config there.

However, we also pass a second object, options, which - inside start and bundle commands, has historical config property ->

name: '--config [string]',
description: 'Path to the CLI configuration file',
parse: (val: string) => path.resolve(val),

You can use it to set a path to custom Metro configuration. The name probably needs to be updated, as historically, Metro/CLI config used to be a single file. While refactoring, we stopped relying on that property inside CLI, but kept it for Metro.

So, in general, I would say there are two ways going forward:

  • either add --config to missing commands
  • remove --config entirely and extend Config with metroConfig property. Users would have to create react-native.config.js file and define this property

While second option is more aligned with our long-term plan, I suspect it would break some tooling such as Expo. So as a short-term hack, I'd just stick to adding --config here and there.

One thing to consider on top of that would be to rename --config to --metro-config. While doing so, I would use Commander feature to define multiple names for an option and accept both occurrences. We don't want to break existing tooling, at the same time, we want to promote less confusing naming.

@ElviraBurchik
Copy link
Author

@pepibumur maybe we could close this pr for now?

@flextremedev
Copy link

Hey @ElviraBurchik, is this issue in work or what's the current status?

@ElviraBurchik
Copy link
Author

Hey @flextremedev 👋 It's on hold for now

@flextremedev
Copy link

So nobody's working on it right now? If so I could have a look at this as we're having the same problem.
It works if I run react-native start before run-android but not if the cli starts the metro server as you mentioned.

@ElviraBurchik
Copy link
Author

It works if I run react-native start before run-android but not if the cli starts

Right, this is our temporary solution too.

Nobody is working on it right now on our side, but we could pair on this issue together if you want 🙂

@flextremedev
Copy link

Sure 🙂

@grabbou
Copy link
Member

grabbou commented Oct 4, 2021

It works if I run react-native start before run-android but not if the cli starts the metro server as you mentioned.

I think it's a bug then. Rather than adding a metro-config option as per #1461 PR, we should figure why this does not work as a part of run-android and provide a fix for it.

The desired behaviour should be to read the config from the cwd where CLI was called and traverse upwards. There's little we want to do around that to not tightly couple the codebase with Metro.

That's what happens when you run start command.

The fix would be to update this function

function startServerInNewWindow(
which starts a new Metro instance to use a proper cwd. It probably uses the location of node_modules, since it's called here:
startServerInNewWindow(
args.port,
args.terminal,
config.reactNativePath,
);

So TL;DR passing ctx.root should do it, or process.cwd.

@grabbou grabbou added bug Something isn't working and removed feature request labels Oct 4, 2021
@fortmarek
Copy link
Contributor

Thanks for the response @grabbou!

However, I'm not sure if your suggestion works for our case. The issue is that we use cwd. The reason we wanted to pass custom path to metro config is because we run run-android from the root of our project but the metro.config.js is located one directory below in a rootPath/fixture directory. Therefore, I believe the PR I opened still has its merit but maybe there is a better way I'm not seeing.

@duailibe
Copy link

duailibe commented Nov 9, 2021

@grabbou I'm having a similar problem, I believe the problem is that launchPackager.command (and packager.sh) from react-native assumes the project root differently from the CLI. It assumes the root is the folder where react-native is installed and cd's to it.

https://github.com/facebook/react-native/blob/e973b3afc274f892a0e5a6fdea9004dc5d84eb2b/scripts/packager.sh#L10-L23

My folder structure is:

~/Code/project/
    package.json
    node_modules/
        react-native/
    web/
        index.js
        package.json
    mobile/
        index.js
        metro.config.js
        package.json

And when I run both run-android and run-ios from the mobile/ folder, it runs:

❯ /Users/me/Code/project/node_modules/react-native/scripts/launchPackager.command

and it starts the bundler, but it fails to bundle with the following message:

Error: Unable to resolve module ./index from /Users/me/project/.: 

None of these files exist:
  * index(.native|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)
  * index/index(.native|.ios.js|.native.js|.js|.ios.json|.native.json|.json|.ios.ts|.native.ts|.ts|.ios.tsx|.native.tsx|.tsx)

I don't see how the launchPackager.command uses cwd in any way.

@chrismcleod
Copy link

@duailibe did you ever figure this out?

@github-actions
Copy link

There hasn't been any activity on this issue in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

@jonrigerths2
Copy link

Was this ever resolved? still seems like it's not supported.
I'm using Turborepo/pnpm setup where I have:

my-project/
   apps/
       react-native/
             metro.config.js

I get No metro config found in my-project

@szymonrybczak
Copy link
Collaborator

Hey @jonrigerths2 this is broken in 0.72 version, but there's created fix for this, and we're waiting for release 👍

@jonrigerths2
Copy link

@szymonrybczak oh awesome, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants