-
Notifications
You must be signed in to change notification settings - Fork 3
chore: better gradle setup #188
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
base: main
Are you sure you want to change the base?
Changes from all commits
511e61a
9f3ed85
4c0cc1d
6b62848
c755b10
9077adf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@react-native-node-api/test-app': patch | ||
'react-native-node-api': patch | ||
--- | ||
|
||
Improvements to setup scripts, updated docs |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
22 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,11 +2,32 @@ | |||||
|
||||||
## Building Hermes from source | ||||||
|
||||||
Because we're using a version of Hermes patched with Node-API support, we need to build React Native from source. | ||||||
### Step 1: `settings.gradle` | ||||||
|
||||||
Gradle needs specific configuration to build React Native from source: dependency substitutions for React Native & Hermes modules, and modifications to default logic in RN build scripts, which is done by setting an environment variable, as described in this section. | ||||||
|
||||||
In your app's `settings.gradle` please include the below: | ||||||
|
||||||
```groovy | ||||||
// customize the path to match your node_modules location | ||||||
apply(from: "../../../node_modules/react-native-node-api/android/app-settings.gradle") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we introduce our own settings script it becomes a bit more magical IMO and I'm not 100% convinced this is an increase in DX 🤔 There's something "simple" in just instructing users to build React Native from source and defer to whatever upstream documentation is available for doing that, with a code-snippet to make it easier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT @shirakaba? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now we have asymmetry between the iOS and Android sides – iOS is set up automatically, while Gradle isn't. So I think we need to at least make things consistent. If this were an Expo project, I'd probably arrange it as a config plugin called I think it's okay to defer to the upstream docs for building React Native from source. Maybe that part needn't be handled by our plugin, then? Something I would like to improve the DX on, though, is this part: export REACT_NATIVE_OVERRIDE_HERMES_DIR=`npx react-native-node-api vendor-hermes --silent` It's a pain because it relies on shell stuff, so there's great potential for people to get it wrong (e.g. run it in the wrong directory or forget to export it again). If this were an Expo project, I'd probably say that Any ideas? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This path ( Can we use proper Node module resolution instead? As demonstrated here. |
||||||
applyNodeAPISettings(settings) | ||||||
``` | ||||||
export REACT_NATIVE_OVERRIDE_HERMES_DIR=`npx react-native-node-api vendor-hermes --silent` | ||||||
|
||||||
### Step 2: script for adjusting environment variables | ||||||
|
||||||
> [!IMPORTANT] | ||||||
> Each time you run Android Studio or build the Android app from a terminal, make sure the below is in place. | ||||||
|
||||||
Because we're using a version of Hermes patched with Node-API support, we need to build React Native from source. A special environment variable (`REACT_NATIVE_OVERRIDE_HERMES_DIR`) must be set to the path of a Hermes engine with Node-API support. Since Gradle does not support loading `.env` files directly, this must be automated by the consumer. We provide the `react-native-node-api vendor-hermes --silent` command, which will download Hermes and output the path to Hermes directory path as its only output. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
You can configure the environment variable using the following command: | ||||||
|
||||||
``` | ||||||
export REACT_NATIVE_OVERRIDE_HERMES_DIR="$(npx react-native-node-api vendor-hermes --silent)" | ||||||
artus9033 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
``` | ||||||
|
||||||
This either needs to be done each time before Gradle / Android Studio invocation, or permanently in a shell init script such as `~/.zshrc` on Zsh (MacOS) or `~/.bashrc` on Bash (Linux). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this is a good suggestion. It has the potential to mess up people's setup, quite significantly - in a way which is really hard to debug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I think it'd be best to avoid mentioning the idea of populating the user profiles at all. For Expo apps, we can recommend writing it into For non-Expo apps, I don't think we have any options. The React Native Community CLI template has no hits for |
||||||
|
||||||
## Cleaning your React Native build folders | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# iOS support | ||
|
||
Because we're using a version of Hermes patched with Node-API support, we need to build React Native from source. Special environment variables (`REACT_NATIVE_OVERRIDE_HERMES_DIR`, `BUILD_FROM_SOURCE`) must be set to the path of a Hermes engine with Node-API support. The podspec of the iOS Pod already includes instrumentation to configure React Native appropriately via [`patch-hermes.rb`](../packages/host/scripts/patch-hermes.rb). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import org.gradle.initialization.DefaultSettings | ||
|
||
|
||
ext.applyNodeAPISettings = { DefaultSettings settings -> | ||
def searchDirectory = rootDir.toPath() | ||
do { | ||
def p = searchDirectory.resolve("node_modules/react-native") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of walking the file system, could we use actual Node module resolution, like the Expo Android template demonstrates? Caution: I'm not clear whether the reactNativeDir variable is populated in projects made with the React Native Community CLI, so it'd be safest to just re-evaluate the path inside this Gradle script. |
||
if (p.toFile().exists()) { | ||
println "[Node-API] !!! PATCHING HERMES WITH NODE-API SUPPORT !!! Found React Native in ${p.toRealPath().toString()}" | ||
|
||
includeBuild(p.toRealPath().toString()) { | ||
dependencySubstitution { | ||
substitute(module("com.facebook.react:react-android")).using(project(":packages:react-native:ReactAndroid")) | ||
substitute(module("com.facebook.react:react-native")).using(project(":packages:react-native:ReactAndroid")) | ||
substitute(module("com.facebook.react:hermes-android")).using(project(":packages:react-native:ReactAndroid:hermes-engine")) | ||
substitute(module("com.facebook.react:hermes-engine")).using(project(":packages:react-native:ReactAndroid:hermes-engine")) | ||
} | ||
} | ||
break | ||
} | ||
} while (searchDirectory = searchDirectory.getParent()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I intended the "bootstrap" script to be the one you run to build both TypeScript and prebuilds, but it does take a while 🤔