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

fix(iOS): Use REACT_NATIVE_PATH env to resolve react native path in Hermes podspec #48574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gabrieldonadel
Copy link
Collaborator

Summary:

On iOS, the REACT_NATIVE_PATH environment variable is set during both the CocoaPods installation process and the app build. Most React Native scripts already rely on this variable or provide a way to specify a custom React Native path (rather than relying on the default node_modules resolution).

However, the Hermes podspec currently does not support using REACT_NATIVE_PATH to determine the React Native version. This limitation creates issues in scenarios like Expo Go, where React Native is built from a submodule, making Hermes resolve an incorrect React Native version

Changelog:

[IOS] [CHANGED] - Use REACT_NATIVE_PATH env to resolve react native path in Hermes podspec

Test Plan:

Run pod install and build

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 9, 2025
@facebook-github-bot facebook-github-bot added p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 9, 2025
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Hi @gabrieldonadel thanks for the PR.
I don't really like the idea of using an Env variable for this approach and I believe we can clean up the podspec in a better way.

the use_react_native! function receives the react_antive_path as a parameter.
Is this value set properly for Expo GO?

What we can do, instead of recomputing the react_native_path, is to set that value in a variable in the react_native_pods.rb file and then expose the variable so that it can be consumed by the hermes-engine.podspec file.

How does that sound like? Can you take care of updating the PR?

@gabrieldonadel
Copy link
Collaborator Author

Hi @gabrieldonadel thanks for the PR. I don't really like the idea of using an Env variable for this approach and I believe we can clean up the podspec in a better way.

the use_react_native! function receives the react_antive_path as a parameter. Is this value set properly for Expo GO?
Yeah, we use the react_native_path param, and it works for everything but the hermes podspec

What we can do, instead of recomputing the react_native_path, is to set that value in a variable in the react_native_pods.rb file and then expose the variable so that it can be consumed by the hermes-engine.podspec file.

How does that sound like? Can you take care of updating the PR?

This sounds great, let me update this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants