Check Hermes override pre-condition from Host package when building for Android #234
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kraenhansen
added
Android 🤖
kraenhansen
force-pushed
the
kh/android/check-hermes-override
branch
from
b55f856 to
46934c5
Compare
|
Drafting this again, since it seems Gradle is having a hard time executing the CLI on Windows. |
kraenhansen
force-pushed
the
kh/android/check-hermes-override
branch
2 times, most recently
from
3a11ac5 to
d8b9a83
Compare
kraenhansen
force-pushed
the
kh/android/check-hermes-override
branch
from
c1a69c0 to
78f34dc
Compare
kraenhansen
force-pushed
the
kh/android/check-hermes-override
branch
from
78f34dc to
c6a7d68
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation and fixes for React Native Node-API's Android build process, specifically ensuring proper Hermes configuration and improving cross-platform compatibility.
- Adds a
checkHermesOverrideGradle task to verifyREACT_NATIVE_OVERRIDE_HERMES_DIRenvironment variable is set - Fixes Windows compatibility for CLI auto-linking in Gradle builds
- Adds comprehensive test coverage for Gradle tasks and CLI functionality
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/host/android/build.gradle | Adds Hermes override validation task and fixes Windows CLI execution |
| packages/host/src/node/gradle.test.ts | New test file for Gradle task validation |
| packages/host/src/node/cli/bin.test.ts | New test file for CLI command validation |
| packages/host/package.json | Adds test script for Gradle-specific tests |
| .github/workflows/check.yml | Enables Gradle tests in CI environment |
| .changeset/silver-suits-double.md | Documents the Hermes override assertion feature |
| .changeset/many-candies-retire.md | Documents the Windows auto-linking fix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Pushed a fix for the failure to call into the CLI on Windows. |
Co-authored-by: Copilot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ideally, we would have liked to inject the overriding of Hermes source directory from within, but haven't found a way to do that. Until we find a way we can assert that it's done by the caller.
Merging this PR will:
checkHermesOverrideto the host package Gradle build script to ensureREACT_NATIVE_OVERRIDE_HERMES_DIRis set (fixing Sanity check of Hermes source directory via environment variable, when building Android projects #163).linkNodeApiModulestask is calling into the CLI to execute auto-linking.