Add a --react-native-package option to vendor-hermes command

[kraenhansen]

A first attempt at solving #270.

Merging this PR will:

• Add a --react-native-package option to the vendor-hermes command, controlling what package name to use when determining the base version of Hermes to use for cloning.
• Clone Hermes into sdks/node-api-hermes of the React Native package (choosen via --react-native-package ☝️)
• Refactor the scripts/patch-hermes.rb to allow supplying a REACT_NATIVE_OVERRIDE_HERMES_DIR from the outside to skip calling vendor-hermes internally.

With this, you should be able to clone and override Hermes in the MacOS out-of-tree package of React Native running (in <your-app>/ios):

REACT_NATIVE_OVERRIDE_HERMES_DIR=$(npx react-native-node-api vendor-hermes --silent --react-native-package react-native-macos) pod install

[kraenhansen]

@shirakaba I'd love your feedback on this approach.

[shirakaba]

So to my understanding, this would mean that RNM users can obtain a suitable value for REACT_NATIVE_OVERRIDE_HERMES_DIR (which they could then set in their CLI, or just write into their Podfile) with:

npx react-native-node-api vendor-hermes --silent --react-native-package react-native-macos

(your example said --react-native-package react-native – correct me if I'm wrong to use --react-native-package react-native-macos!)

I think it's certainly a step forward and makes it possible to proceed, so as it stands currently, it's worth merging already.

Though in terms of an ideal implementation, my impression of this approach is that it treats react-native-macos as a second-class citizen. RNM already has a thousand extra steps to get set up correctly and it would be nice to not have another gotcha along the way for Node-API modules, especially given that this concerns CocoaPods installs, which are very time-consuming.

I wonder if there's any way we can figure out that the script is being called from a react-native-macos app's Podfile. Like, inside packages/host/scripts/patch-hermes.rb, maybe if we're lucky there's some environment variable that will already have been set by the Podfile that we could use to automatically determine that we're running in a react-native-macos app, and if so, pass --react-native-package react-native-macos along to the vendor-hermes script.

[shirakaba]

Is there any reason for this to be ||= rather than =? As VENDORED_HERMES_DIR is a local variable, I think it can only be unset at this point?

Suggested change

	VENDORED_HERMES_DIR ||= `npx react-native-node-api vendor-hermes --silent '#{Pod::Config.instance.installation_root}'`.strip
	VENDORED_HERMES_DIR = `npx react-native-node-api vendor-hermes --silent '#{Pod::Config.instance.installation_root}'`.strip

[kraenhansen]

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.

[shirakaba]

Summary of the proposed behaviour for my benefit:

• If ENV.REACT_NATIVE_OVERRIDE_HERMES_DIR is defined:
  • The env var is used as-is.
  • The script checks whether the directory is exists.
    • If it exists, then good to go.
    • If not, an exception is raised.
• If ENV.REACT_NATIVE_OVERRIDE_HERMES_DIR is not defined
  • npx react-native-node-api vendor-hermes --silent is called, ultimately vendoring Hermes for react-native (regardless of whether the user is using react-native or react-native-macos.
  • That vendored directory gets set as the value for ENV.REACT_NATIVE_OVERRIDE_HERMES_DIR.

[kraenhansen]

Exactly and I agree it would be ideal if we didn't vendor for react-native if this was driven by a MacOS app. Let's make that happen in a follow-up PR, ideally when we actually have a MacOS app in the repo, testing that flow.

[kraenhansen]

@shirakaba

correct me if I'm wrong to use --react-native-package react-native-macos

You're right! I updated the description 👍 My bad.

my impression of this approach is that it treats react-native-macos as a second-class citizen

You're right - once I have a better testing setup, I'd love for us to add calling this conditionally "the right" --react-native-package value, based on what app Podfile is driving the "pod install".

I think we'd want a MacOS test app in the repo to ensure thats a tested path?