Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/red-candles-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-native-node-api": patch
---

Move REACT_NATIVE_OVERRIDE_HERMES_DIR out of tasks to fail earlier
30 changes: 13 additions & 17 deletions packages/host/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@ import java.nio.file.Paths
import groovy.json.JsonSlurper
import org.gradle.internal.os.OperatingSystem

if (!System.getenv("REACT_NATIVE_OVERRIDE_HERMES_DIR")) {
throw new GradleException([
"React Native Node-API needs a custom version of Hermes with Node-API enabled.",
"Run the following in your terminal, to clone Hermes and instruct React Native to use it:",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Run the following in your terminal, to clone Hermes and instruct React Native to use it:",
"Run the following in your Bash- or zsh-compatible terminal, to clone Hermes and instruct React Native to use it:",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting in separate examples for powershell and cmd prompt probably isn't reasonable, but hopefully this will make it clear (to a human and LLM) not to try to paste this command.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed this as d8872b6 to main 👍

"",
"export REACT_NATIVE_OVERRIDE_HERMES_DIR=\$(npx react-native-node-api vendor-hermes --silent --force)",
"",
"And follow this guide to build React Native from source:",
"https://reactnative.dev/contributing/how-to-build-from-source#update-your-project-to-build-from-source"
].join('\n'))
}

buildscript {
ext.getExtOrDefault = {name ->
return rootProject.ext.has(name) ? rootProject.ext.get(name) : project.properties['NodeApiModules_' + name]
Expand Down Expand Up @@ -135,22 +147,6 @@ dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
}

task checkHermesOverride {
doFirst {
if (!System.getenv("REACT_NATIVE_OVERRIDE_HERMES_DIR")) {
throw new GradleException([
"React Native Node-API needs a custom version of Hermes with Node-API enabled.",
"Run the following in your terminal, to clone Hermes and instruct React Native to use it:",
"",
"export REACT_NATIVE_OVERRIDE_HERMES_DIR=\$(npx react-native-node-api vendor-hermes --silent --force)",
"",
"And follow this guide to build React Native from source:",
"https://reactnative.dev/contributing/how-to-build-from-source#update-your-project-to-build-from-source"
].join('\n'))
}
}
}

def commandLinePrefix = OperatingSystem.current().isWindows() ? ["cmd", "/c", "node"] : []
def cliPath = file("../bin/react-native-node-api.mjs")

Expand All @@ -169,5 +165,5 @@ task linkNodeApiModules {
}
}

preBuild.dependsOn checkHermesOverride, linkNodeApiModules
preBuild.dependsOn linkNodeApiModules

11 changes: 7 additions & 4 deletions packages/host/src/node/gradle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ describe(
// Skipping these tests by default, as they download a lot and takes a long time
{ skip: process.env.ENABLE_GRADLE_TESTS !== "true" },
() => {
describe("checkHermesOverride task", () => {
describe("linkNodeApiModules task", () => {
it("should fail if REACT_NATIVE_OVERRIDE_HERMES_DIR is not set", () => {
const { status, stdout, stderr } = cp.spawnSync(
"sh",
["gradlew", "react-native-node-api:checkHermesOverride"],
["gradlew", "react-native-node-api:linkNodeApiModules"],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test Misplaced: Hermes Directory Check

The test "should fail if REACT_NATIVE_OVERRIDE_HERMES_DIR is not set" is nested under the linkNodeApiModules task describe block, but the REACT_NATIVE_OVERRIDE_HERMES_DIR check now occurs during Gradle build script evaluation. This means the failure happens before the task executes, making the test's context misleading as it implies the check is part of the task itself.

Fix in Cursor Fix in Web

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need some task which wouldn't fail if the feature is broken, which is why I put it here.

{
cwd: TEST_APP_ANDROID_PATH,
env: {
Expand Down Expand Up @@ -45,15 +45,18 @@ describe(
/And follow this guide to build React Native from source/,
);
});
});

describe("linkNodeApiModules task", () => {
it("should call the CLI to autolink", () => {
const { status, stdout, stderr } = cp.spawnSync(
"sh",
["gradlew", "react-native-node-api:linkNodeApiModules"],
{
cwd: TEST_APP_ANDROID_PATH,
env: {
...process.env,
// We're passing some directory which exists
REACT_NATIVE_OVERRIDE_HERMES_DIR: __dirname,
},
encoding: "utf-8",
},
);
Expand Down