Check for REACT_NATIVE_OVERRIDE_HERMES_DIR earlier#246
Conversation
2865a90 to
b0f6f61
Compare
| const { status, stdout, stderr } = cp.spawnSync( | ||
| "sh", | ||
| ["gradlew", "react-native-node-api:checkHermesOverride"], | ||
| ["gradlew", "react-native-node-api:linkNodeApiModules"], |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I need some task which wouldn't fail if the feature is broken, which is why I put it here.
matthargett
left a comment
There was a problem hiding this comment.
Minor wording improvement suggestion
| 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:", |
There was a problem hiding this comment.
| "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:", |
There was a problem hiding this comment.
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.
This is a follow-up to #234
Merging this PR will:
REACT_NATIVE_OVERRIDE_HERMES_DIRout of tasks to fail earlier.