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

feat(iOS): dynamically resolve react native path in Hermes podspec #46181

Closed

Conversation

okwasniewski
Copy link
Contributor

Summary:

This PR modifies hermes-engine.podspec to resolve the path to react-native dynamically.

In OOT platforms case we often have slightly different versioning, let's say react-native is at 0.75.1 and react-native-visionos is at 0.75.2. This causes an issue while resolving the prebuilt version of Hermes. We should always get the Hermes tied to the react-native package version, not the OOT platform.

Changelog:

[IOS] [FIXED] - Resolve Hermes prebuilt version based on react-native packge

Test Plan:

Install pods

@facebook-github-bot 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. p: Callstack Partner: Callstack Partner labels Aug 23, 2024
@okwasniewski okwasniewski changed the title feat(iOS): dynamically resolve hermes path feat(iOS): dynamically resolve react native path in Hermes podspec Aug 23, 2024
@facebook-github-bot 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 Aug 23, 2024
@cipolleschi
Copy link
Contributor

Thanks for fixing this for all the OOT platforms!

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 27, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in b10ed0e.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @okwasniewski in b10ed0e

When will my fix make it into a release? | How to file a pick request?

facebook-github-bot pushed a commit that referenced this pull request Sep 13, 2024
Summary:
This PR is a follow-up after #46181 this change makes sure that if the `require.resolve` fails we still fall back to the old behavior.

The `require.resolve` was failing for local builds of OOT platforms (because in OOT platforms mono repo `react-native` is renamed to `react-native-platform-name`)

## Changelog:

[IOS] [FIXED] - Fallback to old resolve mechanism when node require fails to resolve react native path

Pull Request resolved: #46432

Test Plan: CI Green

Reviewed By: christophpurrer

Differential Revision: D62577183

Pulled By: cipolleschi

fbshipit-source-id: d62d9c2a5eee3546a81d2aad52b7f73763315b18
@Saadnajmi
Copy link
Contributor

Slightly late thanks, I think I'm running into a similar issue for React Native macOS right now :)

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. Merged This PR has been merged. p: Callstack Partner: Callstack 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.

5 participants