fix: Android gradle config when bundling for release

[vdmtrv]

Summary

This fix aims to address the issue when bundling an Android app for release and getting the error exhibited in #28002 which I also encountered myself.

The config was changed sometime in November 2019 (as part of #26940, commit a3b0804) to be very opinionated when it comes to the use of npx which Gradle itself cannot find anyway (I have npx installed globally and it didn't pick it up).

Another issue that the use of npx creates is that Gradle should only ever use the currently installed react-native cli rather than a (possibly) higher version which may not always have backward compatibility.

The proposed change simply throws a more descriptive error rather than defaulting to a tool which may or may not exist on the machine, be it CI or a development environment. I've also modified the RNTester app to reflect the correct config implementation relative to the RNTester app itself.

In real projects, the config inside android/app/build.gradle should look similar to the following snippet:

project.ext.react = [
  cliPath: "$rootDir/../node_modules/react-native/cli.js",
  entryFile: "index.js"
];

Changelog

[Android] [Fixed] - Gradle release config

Test Plan

• Successfully bundled an Android release build with correct config
• Works with RNTester app

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,596,588	0
android	hermes	armeabi-v7a	6,273,741	0
android	hermes	x86	6,936,744	0
android	hermes	x86_64	6,866,547	0
android	jsc	arm64-v8a	8,904,972	0
android	jsc	armeabi-v7a	8,545,447	0
android	jsc	x86	8,729,149	0
android	jsc	x86_64	9,305,192	0

Base commit: d3658bc

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	966,896	-9,833,328

Base commit: d3658bc

[hramos]

Thank you for taking your time to improve this experience! Do you think we should mention android/app/build.gradle explicitly in the error message so it is clear where the config is located?

[vdmtrv]

You're correct. It's probably worth adding. I was simply following the convention on line 49:

throw new Exception("OS not recognized. Please set project.ext.react.hermesCommand " +
                        "to the path of a working Hermes compiler.");

If we include the path in this exception message then it needs to be added to the one on line 49.

[hramos]

I'm OK merging this as is, since it follows convention. I'm not as familiar with Gradle, so my main worry is whether these error messages are clear to the average Android developer. Would they know where to go to fix this config? Does the config location vary across workspaces or is it generally expected at the same path?

In other words, if the config is always at android/app/build.gradle then perhaps we can explicitly point to that file.

[facebook-github-bot]

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @vdmtrv in 0d1fb45.

^{When will my fix make it into a release? | Upcoming Releases}

[grabbou]

Thank you @vdmtrv and @hramos for working on this PR!

We have shipped a follow-up PR a few days after that which completely removed the usage of npx to fix this and a few other issues. We rely on pure node resolution mechanism, which works in all environments and is already used by the CLI to resolve additional modules from node_modules.

In this particular case, I'd consider this a downgrade in terms of the DX - we will now need to propagate cliPath to be added back to all React Native projects, including non-standard setups (such as mono-repositories) where this may be confusing. We never required this configuration, it was only meant to enable React Native to work in non-standard project configurations.

By default, the cliPath is defined here. I guess we should use that one instead, when custom config.cliPath is not present, rather than throwing an error.

That said, I am going to submit a follow-up PR which reverts the requirement of cliPath and aligns this with the rest of the CLI work. In other words, it will resolve the CLI path, but without using npx to do so.

[grabbou]

I am going to send a follow-up PR as well to update code owners to better detect changes around that.

[vdmtrv]

Thank you for providing more context, @grabbou!

I'll keep the points you raised in mind should I come across something similar in the future and CC you as requested. I was not aware that this was being worked on as it wasn't mentioned in any of the open issues related to the use of npx.

I did see config.cliPath, but the behaviour I was able to observe was that the default CLI path was not being respected. Because of this, it fell into the else, where it was using npx, which it couldn't find, hence the rationale behind the commit.

We have shipped a follow-up PR a few days after that which completely removed the usage of npx to fix this and a few other issues.

Can we have a link to the commit/PR for reference, please?

Lastly, I think it's worth linking to the PR which is aiming to improve the above for anyone's future reference - #28625

[grabbou]

No worries @vdmtrv, hard to stay on top of everything - we should update the code owners to better reflect that state. Appreciate time spent on debugging - we wouldn't find a fix if it wasn't for you.

Some additional context: react-native-community/cli#886