Skip to content
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

[RN][macos] Enable Hermes for RNTester #1308

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

shwanton
Copy link

@shwanton shwanton commented Jul 29, 2022

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

On RN Core, Hermes for iOS can be enabled by setting a flag in the Podfile & running pod install
https://reactnative.dev/docs/hermes#ios

Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:

   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )

In Core RNTester for iOS, Hermes is enabled by default
Not until 0.70 - https://github.com/facebook/react-native/blob/0.70-stable/scripts/react_native_pods.rb#L41

def use_react_native! (
  path: "../node_modules/react-native",
  fabric_enabled: false,
  new_arch_enabled: ENV['RCT_NEW_ARCH_ENABLED'] == '1',
  production: ENV['PRODUCTION'] == '1',
  hermes_enabled: true,
  flipper_configuration: FlipperConfiguration.disabled,
  app_path: '..',
  config_file_dir: '')

This will build & install the Hermes runtime & enable the RCT_USE_HERMES macro.
https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm#L10-L16

In 0.69 Hermes is disabled by default - https://github.com/facebook/react-native/blob/0.69-stable/scripts/react_native_pods.rb#L31


On RN Desktop, the Hermes flag was disabled #780 due to M1 build reasons which have since been resolved. #952 #781

Curiously, the RNTester-macOS target AppDelegate was never updated to use Hermes once the Pod was added.
Only the RNTester target for mobile had the correct Hermes usage.

RNTester-macOS: https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm
RNTester: https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

This change will not enable Hermes by default on RNTester. Pass USE_HERMES=1to pod install to enable.


The documentation for enabling Hermes on RN Desktop macOS can be simplified now.
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

  • Setting :hermes_enabled => true in Podfile should do all of the above.

Changelog

[macOS] [pod] Copy Hermes support to RNTester-macOS AppDelegate
[macOS] [pod] Remove pods(:hermes_enabled => true) in favor of using envvar USE_HERMES=1 during pod install.

Test Plan

RNTester with Hermes enabled:

cd packages/rn-tester

# Ensure fresh pod install. I noticed `RCT_USE_HERMES` would sometimes not work w/o full clean/rebuild
rm -rf Pods 

# Install pods w/ Hermes flag
USE_HERMES=1 bundle exec pod install --verbose

# Note the installing (or downloading) of hermes-engine

-> Installing hermes-engine (0.11.0)
  > Copying hermes-engine from `/Users/shawndempsey/Library/Caches/CocoaPods/Pods/Release/hermes-engine/0.11.0-84e3a` to `Pods/hermes-engine`

# Open in xcode
open RNTesterPods.xcworkspace
hermesworkingrntestermed.mp4

Summary:
**Context**

On Core RN, Hermes for iOS can be enabled by setting a flag in the Podfile
https://reactnative.dev/docs/hermes#ios

| Since React Native 0.64, Hermes also runs on iOS. To enable Hermes for iOS, edit your ios/Podfile file and make the change illustrated below:
```
   use_react_native!(
     :path => config[:reactNativePath],
     # to enable hermes on iOS, change `false` to `true` and then install pods
     # By default, Hermes is disabled on Old Architecture, and enabled on New Architecture.
     # You can enable/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`.
     :hermes_enabled => true
   )
```
In the RNTester Podfile, Hermes is enabled using envvar:
https://github.com/facebook/react-native/blob/main/packages/rn-tester/Podfile#L27
```
  # Hermes is now enabled by default.
  # The following line will only disable Hermes if the USE_HERMES envvar is SET to a value other than 1 (e.g. USE_HERMES=0).
  hermes_enabled = !ENV.has_key?('USE_HERMES') || ENV['USE_HERMES'] == '1'
```
Build command: `USE_HERMES=1 bundle exec pod install`

This will install the Hermes runtime Pod (not build it from scratch) & thus enable the `RCT_USE_HERMES` macro.

https://www.internalfb.com/code/fbsource/[9f57823a75a40d3f8559c8f1b7ae0add8e95d6dc]/xplat/js/react-native-github/packages/rn-tester/RNTester/AppDelegate.mm?lines=10-16

---

The documentation for enabling Hermes on RN Desktop macOS are outdated:
https://microsoft.github.io/react-native-windows/docs/hermes#hermes-on-macos

> Install the npm package yarn add 'hermes-engine-darwin@^0.4.3'

* `hermes-engine-darwin` is no longer required

> Add (or uncomment) the following pod dependencies to your macOS target in your Podfile:
pod 'React-Core/Hermes', :path => '../node_modules/react-native-macos/'
pod 'hermes', :path => '../node_modules/hermes-engine-darwin'
pod 'libevent', :podspec => '../node_modules/react-native-macos/third-party-podspecs/libevent.podspec'

* Setting `USE_HERMES=1` during `pod install= replaces all of this

> Copy
Run pod install
Be sure to set your target's deployment target to at least 10.14 before running pod install

* `USE_HERMES=1 bundle exec pod install --verbose`

---

On RN Desktop, the Hermes flag was [set to false](microsoft#780) due to M1 build reasons which have since been resolved.
- microsoft#952
- microsoft#781

Curiously, the `RNTester-macOS` target AppDelegate was never updated to import & use Hermes when  `RCT_USE_HERMES` was `true`. Only the `RNTester` for mobile had the correct Hermes usage.

**RNTester-macOS:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester-macOS/AppDelegate.mm

**RNTester:** https://github.com/microsoft/react-native-macos/blob/main/packages/rn-tester/RNTester/AppDelegate.mm

**Change**

* Remove `pods(:hermes_enabled => true)` in favor of passing `USE_HERMES=1` to `pod install` (This is how it's done on RNTester iOS)
* Copy Hermes support to `RNTester-macOS` AppDelegate

Test Plan: **Install from scratch**

Differential Revision: https://phabricator.intern.facebook.com/D38277077
@Saadnajmi
Copy link
Collaborator

Just to be clear, this change brings the RNTester-macOS target back into parity with RN Core and our own RNTester target where enabling and disabling Hermes is as simple as a flag in the pod file? And from the looks of it, keeps it off by default?

@shwanton
Copy link
Author

Yes, we don't have plans on enabling by default (which is what happens now on RN). However, we would like to enable it during development so we can connect & debug using flipper.

Copy link
Collaborator

@Saadnajmi Saadnajmi left a comment

Choose a reason for hiding this comment

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

It looks good from me. I'll have @amgleitman give it a second look if he likes since he was involved with merging the changes from RN Core that included Hermes stuff.

@shwanton shwanton marked this pull request as ready for review July 29, 2022 22:59
@shwanton shwanton requested a review from a team as a code owner July 29, 2022 22:59
@christophpurrer
Copy link

Awesome change!

@Saadnajmi
Copy link
Collaborator

I'll go ahead and merge this.

@Saadnajmi Saadnajmi merged commit 2bcaeb1 into microsoft:main Aug 2, 2022
@christophpurrer
Copy link

If you want to try this out on an M1/M2/... MacBook you need to run

USE_HERMES=1 bundle exec arch -x86_64 pod install --verbose

@Saadnajmi
Copy link
Collaborator

Saadnajmi commented Aug 29, 2022

If you want to try this out on an M1/M2/... MacBook you need to run

USE_HERMES=1 bundle exec arch -x86_64 pod install --verbose

The latest version of cocoapods from homebrew should have m1 support? I don't think you need the "arch -x86_64" bit anymore

@christophpurrer
Copy link

Ooh!
I don't know. I need to do that locally ;-)
I am using this version

chpurrer@chpurrer-mbp rn-tester % pod --version
1.11.3

@Saadnajmi
Copy link
Collaborator

Ooh! I don't know. I need to do that locally ;-) I am using this version

chpurrer@chpurrer-mbp rn-tester % pod --version 1.11.3

Latest I found.. looks like you have to use homebrew? https://stackoverflow.com/a/65334677
I'm also using 1.11.3

@shwanton shwanton deleted the shwanton/enable-hermes-rntester branch November 3, 2022 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants