feat: run npx from JS app dir #824

thymikee · 2019-10-24T15:18:29Z

Summary:

Gradle may be run from a driectory that's outside of JS project, with a passed project dir (through -p flag) like path/to/app/android/gradlew bundleRelease -p ../app/android/app.

project/
  app/ -> js, android, ios files
  scripts/ -> running gradlew from here, e.g. through Fastlane

Gradle CWD is usually (maybe always, but they don't give guarantees) set to the directory where it's ran from. This tricks npx, which doesn't know where to find react-native binary. To account for it, we can spawn npx with a CWD passed to it. We don't need an exact path to android project directory, path to the project which holds @react-native-community/cli in node_modules is just fine with current config search mechanism.

I'm not 100% sure about this fix, open for discussion.

Fixes #793
Fixes #804

Test Plan:

I don't have an automated test for this, you'll need to verify on repro provided in #793.

packages/platform-android/native_modules.gradle

thymikee · 2019-10-25T11:17:59Z

packages/platform-android/native_modules.gradle

    def npx = Os.isFamily(Os.FAMILY_WINDOWS) ? "npx.cmd" : "npx"
-    def command = "${npx} --quiet react-native config"
+    def command = "${npx} --quiet --no-install react-native config"


Adding --no-install so we don't accidentally install react-native into local npm cache. Verified that it's already present in npx shipped with Node 8.

Is it related to #823?

Not really sure, but it's preferable to run with --no-install anyway. Feel free to send a PR, as I don't want to mix iOS and Android in this one :)

packages/platform-android/native_modules.gradle

AndreiCalazans · 2019-10-26T11:03:32Z

I applied these changes inside node_modules and I"m still seeing an issue, it's no longer the same one pointed out in #793.

Here is a gist of the error outputted by Gradle
https://gist.github.com/AndreiCalazans/808217902369be04e0ab5c591afcccc7

thymikee · 2019-10-27T18:48:49Z

@AndreiCalazans it's a normal project structure, in terms of running from Android Studio. I'll need you to debug this a little deeper, as Gradle sync and build works on my machine. This is likely an environmental issue I cannot reproduce.

grabbou · 2019-10-29T20:20:45Z

How about simple heuristics here: if -p flag is present, spawn (cd $pValue && react-native config), otherwise just react-native config?

thymikee · 2019-10-29T21:10:39Z

I'm not sure how to access the -p value. Cc @Salakar

Salakar · 2019-10-29T22:27:56Z

I'm not sure how to access the -p value. Cc @Salakar

Afaik it's not possible to read this. Only properties passed via -P (e.g. -PcustomProp=foo) or --args can be read in the build script.

codestersunion · 2019-10-29T22:28:33Z

In my testing, a cd <path> && npx... does not solve the issue.

grabbou · 2019-11-04T20:05:28Z

@thymikee @Salakar that sounds to me like a serious issue not being able to read args. Being able to do that or at least have the equivalent of __dirname would seriously help with reducing the amount of code needed to resolve this.

Let's discuss this tomorrow and figure how to move forward!

packages/platform-android/native_modules.gradle

ifyapishore · 2019-11-05T16:09:39Z

As far as I know, module scope is better because Gradle can run task in separate thread with different env variables and current dir.

…

On Tue, Nov 5, 2019, 23:04 Mike Grabowski ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/platform-android/native_modules.gradle <#824 (comment)> : > this.logger = logger + this.jsAppDir = jsAppDir Is this.jsAppDir needed or can be a module variable? (Gradle newbie here) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#824?email_source=notifications&email_token=AEB3SYJH4V35E53Q3IUEGITQSGKSLA5CNFSM4JEVRO22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKK74WA#pullrequestreview-311819864>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEB3SYLDO34SK333R3XR4MLQSGKSLANCNFSM4JEVRO2Q> .

lucasbento · 2019-11-12T16:55:24Z

Bump
What's left to be done for this PR?

Looking forward to v3

thymikee · 2019-11-12T17:16:21Z

It's done, waiting for @grabbou and @Esemesek to review. And maybe @Salakar :)

grabbou · 2019-11-14T09:59:52Z

Do we know why the Windows CI is failing? It's blocking few important PRs that are good to be merged otherwise. I'd ship them all if that's not the case.

grabbou · 2019-11-14T10:01:53Z

packages/platform-android/native_modules.gradle

@@ -2,6 +2,7 @@ import groovy.json.JsonSlurper
 import org.gradle.initialization.DefaultSettings
 import org.apache.tools.ant.taskdefs.condition.Os

+def jsAppDir = buildscript.sourceFile.toString().split("node_modules/@react-native-community/cli-platform-android/native_modules.gradle")[0]


nit: I think we can just split by node_modules/@react-native-community/cli-platform-android, which is a path to the module (the gradle file at the end is not needed and may break if we refactor files).

I was thinking to go with node_modules, but that would break in case dependencies were not hoisted.

grabbou · 2019-11-14T10:03:24Z

packages/platform-android/native_modules.gradle

+    ArrayList<HashMap<String, String>> reactNativeModules = new ArrayList<HashMap<String, String>>()
+    def npx = Os.isFamily(Os.FAMILY_WINDOWS) ? "npx.cmd" : "npx"
+    def command = "${npx} --quiet --no-install react-native config"
+    // Running npx from the directory of the JS app which holds this script in its node_modules.


nit: blank space before comment and prefer /* */?
nit: I'd rephrase the comment a bit to explain when such a case happens, in other words, when it's run with a -p flag. We can also link to this PR for further ref.

grabbou

Looks good. Thanks for keeping this clean. Left few commits and let's ship it.

ghost · 2019-11-15T14:26:46Z

I believe this has broken projects running in a monorepo as the react-native config needs to be run in the package directory, however is now running in the root directory as the node_module dependencies are hoisted.

thymikee · 2019-11-15T14:29:12Z

@mbios could you provide a repro we could investigate?

ghost · 2019-11-15T14:59:15Z

git clone git@github.com:brunolemos/react-native-web-monorepo.git
cd react-native-web-monorepo
rm yarn.lock
yarn
yarn android

yields this:

FAILURE: Build failed with an exception.

* Where:
Script '~/react-native-web-monorepo/node_modules/@react-native-community/cli-platform-android/native_modules.gradle' line: 206

* What went wrong:
A problem occurred evaluating script.
> React Native CLI failed to determine Android project configuration. This is likely due to misconfiguration. Config output:

but works without removing the yarn.lock as that points to 3.0.0 alpha

thymikee · 2019-11-15T15:37:43Z

Yep, it's a regression, thanks for noticing!

thymikee requested review from Esemesek and grabbou as code owners October 24, 2019 15:18

thymikee commented Oct 25, 2019

View reviewed changes

packages/platform-android/native_modules.gradle Outdated Show resolved Hide resolved

thymikee commented Oct 25, 2019

View reviewed changes

thymikee mentioned this pull request Oct 25, 2019

Gradle sync fail - ERROR: Text must not be null or empty #793

Closed

ifyapishore reviewed Oct 25, 2019

View reviewed changes

packages/platform-android/native_modules.gradle Outdated Show resolved Hide resolved

thymikee mentioned this pull request Oct 30, 2019

Android: latest release broke support custom react-native Root #804

Closed

lucasbento mentioned this pull request Nov 4, 2019

Add React Native doctor blog post facebook/react-native-website#1420

Merged

3 tasks

grabbou reviewed Nov 5, 2019

View reviewed changes

packages/platform-android/native_modules.gradle Show resolved Hide resolved

subroh0508 mentioned this pull request Nov 6, 2019

ReactNativeを0.61.xにアプデ subroh0508/ID44#1

Merged

thymikee force-pushed the feat/gradle-from-js-root branch from 4ab5666 to 65e2275 Compare November 10, 2019 17:17

grabbou reviewed Nov 14, 2019

View reviewed changes

grabbou approved these changes Nov 14, 2019

View reviewed changes

thymikee added 5 commits November 14, 2019 16:42

feat: run npx from JS app dir

2b4f7f4

improve comment

27b5843

use .sourceFile

a94f79d

extract getCommandOutput

95df79b

better errors

a978a53

thymikee force-pushed the feat/gradle-from-js-root branch from 829a685 to a978a53 Compare November 14, 2019 15:42

apply feedback

dee944a

grabbou merged commit a3a70a1 into master Nov 14, 2019

thymikee deleted the feat/gradle-from-js-root branch November 15, 2019 14:25

thymikee mentioned this pull request Nov 15, 2019

fix: temporary fix for a regression in monorepos #852

Merged

jkoutavas mentioned this pull request Dec 19, 2019

AppCenter error when building Android ReactNative jkoutavas/ReactNative-AppCenter#1

Open

grabbou mentioned this pull request Mar 18, 2020

fix: custom root or calling Gradle outside of project directory #1057

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: run npx from JS app dir #824

feat: run npx from JS app dir #824

thymikee commented Oct 24, 2019 •

edited

Loading

thymikee Oct 25, 2019

ifyapishore Oct 25, 2019

thymikee Oct 27, 2019

AndreiCalazans commented Oct 26, 2019

thymikee commented Oct 27, 2019

grabbou commented Oct 29, 2019

thymikee commented Oct 29, 2019

Salakar commented Oct 29, 2019 •

edited

Loading

codestersunion commented Oct 29, 2019

grabbou commented Nov 4, 2019

ifyapishore commented Nov 5, 2019 via email

lucasbento commented Nov 12, 2019

thymikee commented Nov 12, 2019

grabbou commented Nov 14, 2019

grabbou Nov 14, 2019

grabbou Nov 14, 2019

grabbou left a comment

ghost commented Nov 15, 2019

thymikee commented Nov 15, 2019

ghost commented Nov 15, 2019 •

edited by ghost

Loading

thymikee commented Nov 15, 2019

feat: run npx from JS app dir #824

feat: run npx from JS app dir #824

Conversation

thymikee commented Oct 24, 2019 • edited Loading

Summary:

Test Plan:

thymikee Oct 25, 2019

Choose a reason for hiding this comment

ifyapishore Oct 25, 2019

Choose a reason for hiding this comment

thymikee Oct 27, 2019

Choose a reason for hiding this comment

AndreiCalazans commented Oct 26, 2019

thymikee commented Oct 27, 2019

grabbou commented Oct 29, 2019

thymikee commented Oct 29, 2019

Salakar commented Oct 29, 2019 • edited Loading

codestersunion commented Oct 29, 2019

grabbou commented Nov 4, 2019

ifyapishore commented Nov 5, 2019 via email

lucasbento commented Nov 12, 2019

thymikee commented Nov 12, 2019

grabbou commented Nov 14, 2019

grabbou Nov 14, 2019

Choose a reason for hiding this comment

grabbou Nov 14, 2019

Choose a reason for hiding this comment

grabbou left a comment

Choose a reason for hiding this comment

ghost commented Nov 15, 2019

thymikee commented Nov 15, 2019

ghost commented Nov 15, 2019 • edited by ghost Loading

thymikee commented Nov 15, 2019

thymikee commented Oct 24, 2019 •

edited

Loading

Salakar commented Oct 29, 2019 •

edited

Loading

ghost commented Nov 15, 2019 •

edited by ghost

Loading