Add `--config` option to CLI to allow passing a path to an `rn-cli.config.js` #7883

skevy · 2016-06-01T19:51:34Z

Currently we just try to resolve a rn-cli.config.js file by walking up the tree from node_modules/react-native. In non-standard uses of RN, when your copy of RN may not live within node_modules, it's impossible to use rn-cli.config.js. This PR adds a "config" flag to the cli that let's you pass in a path to rn-cli.config.js.

cc @ide

ghost · 2016-06-01T19:52:18Z

By analyzing the blame information on this pull request, we identified @janicduplessis and @martinbigio to be potential reviewers.

ghost · 2016-06-01T20:05:33Z

@skevy updated the pull request.

ghost · 2016-06-01T22:09:37Z

@skevy updated the pull request.

ghost · 2016-06-01T23:43:28Z

@skevy updated the pull request.

ghost · 2016-06-02T00:20:20Z

@skevy updated the pull request.

ghost · 2016-06-02T22:20:18Z

@skevy updated the pull request.

skevy · 2016-06-02T22:25:04Z

I'm sorry I suck so much at programming @ide.

ide · 2016-06-02T23:41:20Z

@facebook-github-bot shipit

ghost · 2016-06-02T23:43:27Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

ghost · 2016-06-03T11:48:11Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

ide · 2016-06-03T17:20:51Z

cc @martinbigio could you tell us what the error for this commit is? The OSS unit tests pass so it's hard to see what's wrong.

janicduplessis · 2016-06-03T19:14:54Z

It's probably because there is a merge conflit at the moment.

skevy · 2016-06-03T20:58:42Z

Yep you're right @janicduplessis. Thanks.

ghost · 2016-06-03T20:59:20Z

@skevy updated the pull request.

ide · 2016-06-03T21:49:41Z

@facebook-github-bot shipit

ghost · 2016-06-03T21:58:22Z

Thanks for importing. If you are an FB employee go to Phabricator to review.

ghost · 2016-06-04T11:48:10Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

ide · 2016-07-01T01:07:01Z

@frantic could you help us find the stack trace that made the import fail?

frantic · 2016-07-15T09:44:14Z

Seems like it failed for unknown reason. Will rebase and try re-landing

Yours truly,
🐢

frantic · 2016-07-15T09:50:16Z

local-cli/util/Config.js

 */
 const Config = {
-  get(cwd, defaultConfig) {
-    if (cachedConfig) {


so we are no longer doing caching? What's the possible side effects of this change?

In OSS at least, there's only one caller:

$ git grep 'Config.get(' local-cli/cliEntry.js: command[0](args, Config.get(__dirname, defaultConfig)).done();

And since cliEntry.js runs just once, I don't think caching mattered. Also the cache doesn't create any cache key based on cwd + defaultConfig, so if you called Cache.get with different arguments you might get the wrong cached value.

facebook-github-bot · 2016-08-09T08:07:45Z

@skevy updated the pull request.

skevy · 2016-08-09T21:58:36Z

So it seems like this passes now. w00t

@facebook-github-bot shipit

ghost · 2016-08-09T21:59:03Z

I will not rubber stamp and land your change for you @skevy! I can import it for you and you can get your change reviewed by someone though :)

ide · 2016-08-10T05:55:39Z

I added a package.json change (added minimist), someone at FB will have to merge this diff.

satya164 · 2016-08-10T12:13:24Z

cc @bestander

bestander · 2016-08-10T15:28:37Z

@ide, I don't see the change in package.json.
We switched to using commander in RN now, could you switch to that?
I'll merge it afterwards.
Sorry for the delay

ide · 2016-08-10T16:29:31Z

I had to use minimist on purpose because rn-cli.config.js is used to set up the other CLI options when configuring Commander. I'll double check package.json and update this diff if needed.

…nfig.js` file.

facebook-github-bot · 2016-08-10T16:35:16Z

@skevy updated the pull request.

ide · 2016-08-10T16:36:55Z

Added minimist to package.json. It actually is not a new dependency because another old dependency (optimist probably) already depends on minimist, but better to explicitly add minimist to package.json.

bestander · 2016-08-10T16:43:53Z

@facebook-github-bot import

ghost · 2016-08-10T16:44:40Z

Thanks for importing.If you are an FB employee go to Phabricator to review internal test results.

ide · 2016-08-12T20:15:02Z

Thanks @bestander!

…nfig.js` Summary: Currently we just try to resolve a rn-cli.config.js file by walking up the tree from node_modules/react-native. In non-standard uses of RN, when your copy of RN may not live within node_modules, it's impossible to use rn-cli.config.js. This PR adds a "config" flag to the cli that let's you pass in a path to rn-cli.config.js. cc ide Closes facebook#7883 Differential Revision: D3382823 Pulled By: bestander fbshipit-source-id: b946f3bb355050fc2fe99273d0e99e441dbed111

…nfig.js` Summary: Currently we just try to resolve a rn-cli.config.js file by walking up the tree from node_modules/react-native. In non-standard uses of RN, when your copy of RN may not live within node_modules, it's impossible to use rn-cli.config.js. This PR adds a "config" flag to the cli that let's you pass in a path to rn-cli.config.js. cc ide Closes facebook/react-native#7883 Differential Revision: D3382823 Pulled By: bestander fbshipit-source-id: b946f3bb355050fc2fe99273d0e99e441dbed111

ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 1, 2016

skevy force-pushed the config-flag-for-cli branch from 62ac4ae to 86d952c Compare June 1, 2016 23:42

skevy force-pushed the config-flag-for-cli branch from 86d952c to fa4ff07 Compare June 2, 2016 00:20

skevy force-pushed the config-flag-for-cli branch from fa4ff07 to 6adc8c1 Compare June 2, 2016 22:18

ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jun 2, 2016

ghost added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Jun 3, 2016

skevy force-pushed the config-flag-for-cli branch from 6adc8c1 to f97fcf9 Compare June 3, 2016 20:58

ghost added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jun 3, 2016

ghost removed the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jun 4, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 12, 2016

frantic reviewed Jul 15, 2016
View reviewed changes

ide force-pushed the config-flag-for-cli branch from f97fcf9 to fb57cc7 Compare July 16, 2016 05:56

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 16, 2016

ide force-pushed the config-flag-for-cli branch from fb57cc7 to 5f3aa3c Compare August 9, 2016 08:07

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 9, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 10, 2016

Add --config option to CLI to allow passing a path to an `rn-cli.co…

8096ea6

…nfig.js` file.

ide force-pushed the config-flag-for-cli branch from 5f3aa3c to 8096ea6 Compare August 10, 2016 16:33

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 10, 2016

ghost closed this in 757ab0b Aug 12, 2016

ide deleted the config-flag-for-cli branch April 25, 2020 08:31

This pull request was closed.

Add --config option to CLI to allow passing a path to an rn-cli.config.js #7883

Add --config option to CLI to allow passing a path to an rn-cli.config.js #7883

Uh oh!

Conversation

skevy commented Jun 1, 2016

Uh oh!

ghost commented Jun 1, 2016

Uh oh!

ghost commented Jun 1, 2016

Uh oh!

ghost commented Jun 1, 2016

Uh oh!

ghost commented Jun 1, 2016

Uh oh!

ghost commented Jun 2, 2016

Uh oh!

ghost commented Jun 2, 2016

Uh oh!

skevy commented Jun 2, 2016

Uh oh!

ide commented Jun 2, 2016

Uh oh!

ghost commented Jun 2, 2016

Uh oh!

ghost commented Jun 3, 2016

Uh oh!

ide commented Jun 3, 2016

Uh oh!

janicduplessis commented Jun 3, 2016

Uh oh!

skevy commented Jun 3, 2016

Uh oh!

ghost commented Jun 3, 2016

Uh oh!

ide commented Jun 3, 2016

Uh oh!

ghost commented Jun 3, 2016

Uh oh!

ghost commented Jun 4, 2016

Uh oh!

ide commented Jul 1, 2016

Uh oh!

frantic commented Jul 15, 2016

Uh oh!

frantic Jul 15, 2016

Choose a reason for hiding this comment

Uh oh!

ide Jul 15, 2016

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 9, 2016

Uh oh!

skevy commented Aug 9, 2016

Uh oh!

ghost commented Aug 9, 2016

Uh oh!

ide commented Aug 10, 2016

Uh oh!

satya164 commented Aug 10, 2016

Uh oh!

bestander commented Aug 10, 2016

Uh oh!

ide commented Aug 10, 2016

Uh oh!

facebook-github-bot commented Aug 10, 2016

Uh oh!

ide commented Aug 10, 2016

Uh oh!

bestander commented Aug 10, 2016

Uh oh!

ghost commented Aug 10, 2016

Uh oh!

ide commented Aug 12, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Add `--config` option to CLI to allow passing a path to an `rn-cli.config.js` #7883

Add `--config` option to CLI to allow passing a path to an `rn-cli.config.js` #7883