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

[iOS] Fix Fabric issue with React-Core pod when Xcode project has multiple targets #37581

Closed
wants to merge 1 commit into from

Conversation

douglowder
Copy link
Contributor

@douglowder douglowder commented May 26, 2023

Summary:

In Xcode projects with multiple targets, and in particular when targets are for different platforms (e.g. iOS and macOS), Cocoapods may add a suffix to a Pod name like React-Core.

When this happens, the code in new_architecture.rb (which was looking for a pod with exact name React-Core) would not add the preprocessor definitions for Fabric as expected.

This change fixes this issue. Fixes #37102 .

Changelog:

[iOS] [Fixed] - Fix Fabric issue with React-Core pod when Xcode project has multiple targets

Test Plan:

Tested that this change fixes this issue which occurs 100% of the time in React Native TV projects.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,739,887 -1
android hermes armeabi-v7a 8,051,052 -5
android hermes x86 9,230,198 -1
android hermes x86_64 9,081,575 -1
android jsc arm64-v8a 9,302,497 -2
android jsc armeabi-v7a 8,491,262 -1
android jsc x86 9,363,881 +0
android jsc x86_64 9,619,368 -1

Base commit: dcb4eb0
Branch: main

@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 May 26, 2023
@Pranav-yadav Pranav-yadav added the Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) label May 26, 2023
@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 May 30, 2023
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in d3e3599.

Szymon20000 pushed a commit to Szymon20000/react-native that referenced this pull request Jun 14, 2023
…targets (facebook#37581)

Summary:
In Xcode projects with multiple targets, and in particular when targets are for different platforms (e.g. iOS and macOS), Cocoapods may add a suffix to a Pod name like `React-Core`.

When this happens, the code in `new_architecture.rb` (which was looking for a pod with exact name `React-Core`) would not add the preprocessor definitions for Fabric as expected.

This change fixes this issue. Fixes facebook#37102 .

## Changelog:

[iOS] [Fixed] - Fix Fabric issue with React-Core pod when Xcode project has multiple targets

Pull Request resolved: facebook#37581

Test Plan: Tested that this change fixes this issue which occurs 100% of the time in React Native TV projects.

Reviewed By: dmytrorykun

Differential Revision: D46264704

Pulled By: cipolleschi

fbshipit-source-id: 8dfc8e342b5a110ef1f028636e01e5c5f2b6e2f0
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. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Build for IOS after enabling new architecture,
4 participants