Fix hermes executor not available when use_frameworks is enabled #34222

Kudo · 2022-07-19T14:23:02Z

Summary

When in use_frameworks! + hermes_enabled mode, the React podspecs have not dependency to the React-hermes podspec. the HermesExecutorFactory.h is not available here and here. The app will fallback to JSCRuntime and crash in release build because the JSCRuntime cannot evaluate Hermes bundles.

The pr tries to add React-hermes to Framework Search Paths and make the HermesExecutorFactory.h reachable when in use_frameworks mode.

This pr should fix the problem mentioned in 0.70 release discussion.

Changelog

[iOS] [Fixed] - Fix Hermes executor not available when use_frameworks is enabled

Test Plan

use patch-package to apply the change to a RN 0.69.1 project with the following test cases:

old architecture + jsc
new architecture + hermes
old architecture + use_frameworks! :linkage => :static + hermes
old architecture +use_frameworks! :linkage => :static + hermes + release build

analysis-bot · 2022-07-19T14:53:23Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,825,302	+0
android	hermes	armeabi-v7a	7,216,532	+0
android	hermes	x86	8,138,262	+0
android	hermes	x86_64	8,115,811	+0
android	jsc	arm64-v8a	9,703,479	+0
android	jsc	armeabi-v7a	8,457,929	+0
android	jsc	x86	9,654,618	+0
android	jsc	x86_64	10,252,016	+0

Base commit: ccdf9ac
Branch: main

analysis-bot · 2022-07-19T16:48:52Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: ccdf9ac
Branch: main

cipolleschi · 2022-07-20T15:18:52Z

Hi @Kudo, thanks for the PR. However, I don't understand why, with use_framework! we lose the dependency from hermes.

From this file we should install hermes when it is enabled.

I would rather try to understand why with use_framework! this Pod is not installed and fix that, instead of adding a workaround, falling back to something else.

Kudo · 2022-07-20T15:41:18Z

@cipolleschi in use_frameworks mode, each podspec would has a dedicated xcode target. the header search is mostly based on the Framework Search Paths settings. CocoaPods generates the values based on podspec dependencies.

this is an example of Framework Search Paths from React-Core target, where contain both RCTAppSetupUtils and RCTCxxBridge files. there's no React-hermes in the Framework Search Paths and no hermes headers available to RCTAppSetupUtils/RCTCxxBridge.

the reason hermes works in non_use_frameworks mode is that the React-Core adds an header search entry to hermes. also in non_use_frameworks mode, the compiler don't have to generate *.a or *.so files. if headers are reachable, the compile can link *.o all together for final executable. however for use_frameworks mode, we should add correct framework search paths for xcode to link correct React-hermes libs.

that's my understands. hopefully that would help you to have some insight for further investigation.

cipolleschi · 2022-07-20T15:54:12Z

Thanks for the clarification. I just have another question, following this:

the reason hermes works in non_use_frameworks mode is that the React-Core adds an header search entry to hermes. also in non_use_frameworks mode, the compiler don't have to generate *.a or *.so files. if headers are reachable, the compile can link *.o all together for final executable. however for use_frameworks mode, we should add correct framework search paths for xcode to link correct React-hermes libs.

I don't see any check that says to skip that line when use_frameworks! is used. So Why that change to the HEADER_SEARCH_PATHS is not applied in that case? Should we replicate that line in another podspec setting, perhaps?

Kudo · 2022-07-20T16:14:35Z

I don't see any check that says to skip that line when use_frameworks! is used. So Why that change to the HEADER_SEARCH_PATHS is not applied in that case? Should we replicate that line in another podspec setting, perhaps?

because the path in use_frameworks mode is different. it's ${PODS_CONFIGURATION_BUILD_DIR}/React-hermes/reacthermes.framework/Headers. and we should also change #import <reacthermes/HermesExecutorFactory.h> to #import <HermesExecutorFactory.h>

sorry the following description was wrong. i just tested that. once i reached the header in ``${PODS_CONFIGURATION_BUILD_DIR}/React-hermes/reacthermes.framework/Headers`, xcode can build it successfully.

also in non_use_frameworks mode, the compiler don't have to generate *.a or *.so files. if headers are reachable, the compile can link *.o all together for final executable. however for use_frameworks mode, we should add correct framework search paths for xcode to link correct React-hermes libs.

cipolleschi · 2022-07-20T16:28:56Z

because the path in use_frameworks mode is different. it's ${PODS_CONFIGURATION_BUILD_DIR}/React-hermes/reacthermes.framework/Headers. and we should also change #import <reacthermes/HermesExecutorFactory.h> to #import <HermesExecutorFactory.h>

I understand. Sorry if I'm throwing all these questions to you but:

why is cocoapods adding the header into the search path for React-cxxreact and not for React-hermes?
what happens if we add the here an entry "FRAMEWORK_SEARCH_PATH" with the proper configurations?
Are we sure we need to change this #import <reacthermes/HermesExecutorFactory.h>? Can't we use the modular_headers settings to avoid it, for example?

I would rather try to reduce as much as we can the changes to the code to avoid the introduction of potential bugs.

Kudo · 2022-07-20T16:50:02Z

why is cocoapods adding the header into the search path for React-cxxreact and not for React-hermes?

React-Core.podspec doesn't have dependencies to React-hermes.

what happens if we add the here an entry "FRAMEWORK_SEARCH_PATH" with the proper configurations?

brilliant idea. that's work and it's an easy fix. i'll update the pr tomorrow based on this solution.

Are we sure we need to change this #import <reacthermes/HermesExecutorFactory.h>? Can't we use the modular_headers settings to avoid it, for example?

i didn't test this. i like approach 2.

I would rather try to reduce as much as we can the changes to the code to avoid the introduction of potential bugs.

np! i like these discussion and brainstorming for better solution. and there we are 👍

cipolleschi · 2022-07-22T15:37:35Z

Hi Kudo, thanks for updating this. Could you rebase on main? 🙏

Kudo · 2022-07-22T15:48:20Z

thanks for the update. let's see how it going for ci jobs

Kudo · 2022-07-22T17:42:51Z

remaining a test js error , other ci jobs are passed 🤔

cipolleschi · 2022-07-25T10:52:26Z

JS Error seems unrelated...

facebook-github-bot · 2022-07-25T10:53:02Z

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

react-native-bot · 2022-07-27T14:15:05Z

This pull request was successfully merged by @Kudo in 88b7b64.

^{When will my fix make it into a release? | Upcoming Releases}

) Summary: When in `use_frameworks!` + `hermes_enabled` mode, the React podspecs have not dependency to the `React-hermes` podspec. the `HermesExecutorFactory.h` is not available [here](https://github.com/facebook/react-native/blob/1d997ce6d67242b9f667b01365c5e719c3d3f8d7/React/AppSetup/RCTAppSetupUtils.h#L15) and [here](https://github.com/facebook/react-native/blob/1d997ce6d67242b9f667b01365c5e719c3d3f8d7/React/CxxBridge/RCTCxxBridge.mm#L44). The app will fallback to JSCRuntime and crash in release build because the JSCRuntime cannot evaluate Hermes bundles. The pr tries to add `React-hermes` to Framework Search Paths and make the `HermesExecutorFactory.h` reachable when in use_frameworks mode. This pr should fix the problem mentioned in [0.70 release discussion](reactwg/react-native-releases#26 (reply in thread)). ## Changelog [iOS] [Fixed] - Fix Hermes executor not available when `use_frameworks` is enabled Pull Request resolved: #34222 Test Plan: use patch-package to apply the change to a RN 0.69.1 project with the following test cases: - old architecture + jsc - new architecture + hermes - old architecture + `use_frameworks! :linkage => :static` + hermes - old architecture +`use_frameworks! :linkage => :static` + hermes + release build Reviewed By: sammy-SC Differential Revision: D38112717 Pulled By: cipolleschi fbshipit-source-id: ee78527f4686400856ab9a055cf30b20900afc17

…ebook#34222) Summary: When in `use_frameworks!` + `hermes_enabled` mode, the React podspecs have not dependency to the `React-hermes` podspec. the `HermesExecutorFactory.h` is not available [here](https://github.com/facebook/react-native/blob/1d997ce6d67242b9f667b01365c5e719c3d3f8d7/React/AppSetup/RCTAppSetupUtils.h#L15) and [here](https://github.com/facebook/react-native/blob/1d997ce6d67242b9f667b01365c5e719c3d3f8d7/React/CxxBridge/RCTCxxBridge.mm#L44). The app will fallback to JSCRuntime and crash in release build because the JSCRuntime cannot evaluate Hermes bundles. The pr tries to add `React-hermes` to Framework Search Paths and make the `HermesExecutorFactory.h` reachable when in use_frameworks mode. This pr should fix the problem mentioned in [0.70 release discussion](reactwg/react-native-releases#26 (reply in thread)). ## Changelog [iOS] [Fixed] - Fix Hermes executor not available when `use_frameworks` is enabled Pull Request resolved: facebook#34222 Test Plan: use patch-package to apply the change to a RN 0.69.1 project with the following test cases: - old architecture + jsc - new architecture + hermes - old architecture + `use_frameworks! :linkage => :static` + hermes - old architecture +`use_frameworks! :linkage => :static` + hermes + release build Reviewed By: sammy-SC Differential Revision: D38112717 Pulled By: cipolleschi fbshipit-source-id: ee78527f4686400856ab9a055cf30b20900afc17

@Kudo

## Description When there are frameworks used and Hermes enabled in the Podfile of your project, RN falls back to JSC in debug mode and crashes in production mode. This PR makes it possible to use Hermes when there is `use_frameworks!` enabled in the Podfile. Thanks @Kudo who originally fixed this issue in ReactNative repository: facebook/react-native#34222 ## Changes Added path to `Framework Search Paths` in Podspec of react-native-reanimated. ## Test code and steps to reproduce 1. Create empty ReactNative project 2. Add react-native-reanimated dependency. 3. For ReactNative 0.69.3 apply patch from: https://github.com/facebook/react-native/pull/34222/files 4. Enable frameworks in Podfile: `use_frameworks! :linkage => :static` 5. Build project in debug mode -> you will not see `engine: hermes` in top right corner. 6. Build project in release mode -> app will crash at startup ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [ ] Updated documentation - [ ] Ensured that CI passes

@Kudo

When there are frameworks used and Hermes enabled in the Podfile of your project, RN falls back to JSC in debug mode and crashes in production mode. This PR makes it possible to use Hermes when there is `use_frameworks!` enabled in the Podfile. Thanks @Kudo who originally fixed this issue in ReactNative repository: facebook/react-native#34222 Added path to `Framework Search Paths` in Podspec of react-native-reanimated. 1. Create empty ReactNative project 2. Add react-native-reanimated dependency. 3. For ReactNative 0.69.3 apply patch from: https://github.com/facebook/react-native/pull/34222/files 4. Enable frameworks in Podfile: `use_frameworks! :linkage => :static` 5. Build project in debug mode -> you will not see `engine: hermes` in top right corner. 6. Build project in release mode -> app will crash at startup - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [ ] Updated documentation - [ ] Ensured that CI passes

@Kudo

…-mansion#3491) ## Description When there are frameworks used and Hermes enabled in the Podfile of your project, RN falls back to JSC in debug mode and crashes in production mode. This PR makes it possible to use Hermes when there is `use_frameworks!` enabled in the Podfile. Thanks @Kudo who originally fixed this issue in ReactNative repository: facebook/react-native#34222 ## Changes Added path to `Framework Search Paths` in Podspec of react-native-reanimated. ## Test code and steps to reproduce 1. Create empty ReactNative project 2. Add react-native-reanimated dependency. 3. For ReactNative 0.69.3 apply patch from: https://github.com/facebook/react-native/pull/34222/files 4. Enable frameworks in Podfile: `use_frameworks! :linkage => :static` 5. Build project in debug mode -> you will not see `engine: hermes` in top right corner. 6. Build project in release mode -> app will crash at startup ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Added TS types tests - [ ] Added unit / integration tests - [ ] Updated documentation - [ ] Ensured that CI passes

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner labels Jul 19, 2022

react-native-bot added Bug Platform: iOS iOS applications. labels Jul 19, 2022

Kudo marked this pull request as ready for review July 19, 2022 16:51

Kudo mentioned this pull request Jul 19, 2022

Building iOS to a real device fails mikehardy/rnfbdemo#10

Closed

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 19, 2022

Kudo force-pushed the fix-use-frameworks-hermes branch from 78ab43b to 6b6f78a Compare July 21, 2022 14:50

cipolleschi approved these changes Jul 22, 2022

View reviewed changes

Fix hermes executor not available when use_frameworks is enabled

5ad512f

Kudo force-pushed the fix-use-frameworks-hermes branch from 6b6f78a to 5ad512f Compare July 22, 2022 15:41

facebook-github-bot closed this in 88b7b64 Jul 27, 2022

react-native-bot added the Merged This PR has been merged. label Jul 27, 2022

skaldo mentioned this pull request Aug 18, 2022

Fix Hermes not available on iOS when use_frameworks enabled software-mansion/react-native-reanimated#3491

Merged

6 tasks

cipolleschi mentioned this pull request Nov 1, 2023

[RN][CI]Update node installation on debian (0.70) #41278

Closed

cipolleschi mentioned this pull request Jan 8, 2024

[0.70][RN][General] Migrate boost download url away from JFrog #42178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix hermes executor not available when use_frameworks is enabled #34222

Fix hermes executor not available when use_frameworks is enabled #34222

Kudo commented Jul 19, 2022 •

edited

Loading

analysis-bot commented Jul 19, 2022 •

edited

Loading

analysis-bot commented Jul 19, 2022 •

edited

Loading

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 22, 2022

Kudo commented Jul 22, 2022

Kudo commented Jul 22, 2022

cipolleschi commented Jul 25, 2022

facebook-github-bot commented Jul 25, 2022

react-native-bot commented Jul 27, 2022

Fix hermes executor not available when use_frameworks is enabled #34222

Fix hermes executor not available when use_frameworks is enabled #34222

Conversation

Kudo commented Jul 19, 2022 • edited Loading

Summary

Changelog

Test Plan

analysis-bot commented Jul 19, 2022 • edited Loading

analysis-bot commented Jul 19, 2022 • edited Loading

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 20, 2022

Kudo commented Jul 20, 2022

cipolleschi commented Jul 22, 2022

Kudo commented Jul 22, 2022

Kudo commented Jul 22, 2022

cipolleschi commented Jul 25, 2022

facebook-github-bot commented Jul 25, 2022

react-native-bot commented Jul 27, 2022

Kudo commented Jul 19, 2022 •

edited

Loading

analysis-bot commented Jul 19, 2022 •

edited

Loading

analysis-bot commented Jul 19, 2022 •

edited

Loading