-
Couldn't load subscription status.
- Fork 4
Add a --react-native-package option to vendor-hermes command
#287
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
Changes from all commits
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,5 @@ | ||
| --- | ||
| "react-native-node-api": patch | ||
| --- | ||
|
|
||
| Add --react-native-package option to "vendor-hermes" command, allowing caller to choose the package to download hermes into |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,20 @@ | |
| raise "React Native Node-API cannot reliably patch JSI when React Native Core is prebuilt." | ||
| end | ||
|
|
||
| VENDORED_HERMES_DIR ||= `npx react-native-node-api vendor-hermes --silent '#{Pod::Config.instance.installation_root}'`.strip | ||
| if Dir.exist?(VENDORED_HERMES_DIR) | ||
| Pod::UI.info "Hermes vendored into #{VENDORED_HERMES_DIR.inspect}" | ||
| else | ||
| raise "Hermes patching failed. Please check the output above for errors." | ||
| if ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'].nil? | ||
| VENDORED_HERMES_DIR ||= `npx react-native-node-api vendor-hermes --silent '#{Pod::Config.instance.installation_root}'`.strip | ||
|
Comment on lines
+7
to
+8
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. Summary of the proposed behaviour for my benefit:
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. Exactly and I agree it would be ideal if we didn't vendor for |
||
| # Signal the patched Hermes to React Native | ||
| ENV['BUILD_FROM_SOURCE'] = 'true' | ||
| ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'] = VENDORED_HERMES_DIR | ||
| elsif Dir.exist?(ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR']) | ||
| # Setting an override path implies building from source | ||
| ENV['BUILD_FROM_SOURCE'] = 'true' | ||
| end | ||
|
|
||
| # Signal the patched Hermes to React Native | ||
| ENV['BUILD_FROM_SOURCE'] = 'true' | ||
| ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'] = VENDORED_HERMES_DIR | ||
| if !ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'].empty? | ||
| if Dir.exist?(ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR']) | ||
| Pod::UI.info "[Node-API] Using overridden Hermes in #{ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'].inspect}" | ||
| else | ||
| raise "Hermes patching failed: Expected override to exist in #{ENV['REACT_NATIVE_OVERRIDE_HERMES_DIR'].inspect}" | ||
| end | ||
| end | ||
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.
Is there any reason for this to be
||=rather than=? AsVENDORED_HERMES_DIRis a local variable, I think it can only be unset at this point?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.
It's my understanding that Ruby scripts aren't caching requires the same way Node.js would.
I had this originally because it prevents re-execution of the command if the .rb file is (transitively) "required" multiple times.