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

feat: Allow projectRoot and reactNativePath to be read from metro config #191

Closed
wants to merge 1 commit into from
Closed

Conversation

rajivshah3
Copy link

Summary:

Currently, there's no way to specify projectRoot or reactNativePath in a config file (as shown in the example in #167). This means that these parameters have to be specified as arguments in every react-native command, which poses a problem for repos with a nonstandard reactNativePath. As a result, these repos (namely react-native itself) cannot upgrade to later versions of @react-native-community/cli without updating all of their test scripts (see facebook/react-native#23627, where the tests are failing because the scripts haven't been updated). An easier way to do this would be to simply specify projectRoot and reactNativePath in the CLI/Metro config (see example)

Test Plan:

Running node ../react-native-cli/packages/cli/build/index.js in facebook/react-native@6f731fe no longer throws an error

@thymikee
Copy link
Member

These are not valid Metro configs and we shouldn't mix them.
@grabbou I think it's the right time to think about config for the the CLI itself, separate from Metro.

Something like:

// rncli.config.js
module.exports = {
  projectRoot: '/foo',
  bundlerOptions: {} // likely not necessary, metro.config.js / webpack.config.js should handle bundler options
}

We could use https://github.com/davidtheclark/cosmiconfig for resolving/loading config easily.

@rajivshah3
Copy link
Author

These are not valid Metro configs and we shouldn't mix them.

I agree. I was basically trying to avoid making any significant changes, but if you and @grabbou would prefer to add a more "official" config file (like the example you posted) then I think that would be a good idea too.

@grabbou
Copy link
Member

grabbou commented Feb 27, 2019

Yeah, part of the reason we moved files around was to decouple CLI from Metro. Historically, Metro configuration was shared with CLI one (in fact, there was nothing CLI was reading from that config, so it was just misleading).

I think cosmiconfig is a great and standard solution.

@grabbou
Copy link
Member

grabbou commented Feb 27, 2019

Thank you for doing investigation on this issue @rajivshah3. I have already answered in the React Native PR with some hints how to get this fixed for time being.

Do you feel like having some time in the nearest future to work on the configuration? I would be more than happy to assist when needed.

@grabbou
Copy link
Member

grabbou commented Mar 1, 2019

Going to close this PR in the meantime.

@grabbou grabbou closed this Mar 1, 2019
@rajivshah3
Copy link
Author

Do you feel like having some time in the nearest future to work on the configuration? I would be more than happy to assist when needed.

Sure!

@rajivshah3 rajivshah3 deleted the feature/metro-config branch March 3, 2019 21:30
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

Successfully merging this pull request may close these issues.

3 participants