Skip to content

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

Closed
douglowder wants to merge 1 commit into
react:mainfrom
douglowder:react-core-fabric-ios
Closed

[iOS] Fix Fabric issue with React-Core pod when Xcode project has multiple targets#37581
douglowder wants to merge 1 commit into
react:mainfrom
douglowder:react-core-fabric-ios

Conversation

@douglowder

@douglowderdouglowder commented May 26, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,739,887-1
androidhermesarmeabi-v7a8,051,052-5
androidhermesx869,230,198-1
androidhermesx86_649,081,575-1
androidjscarm64-v8a9,302,497-2
androidjscarmeabi-v7a8,491,262-1
androidjscx869,363,881+0
androidjscx86_649,619,368-1

Base commit: dcb4eb0
Branch: main

@facebook-github-botfacebook-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-yadavPranav-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
Copy Markdown
Contributor

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

@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label May 30, 2023
@facebook-github-bot

Copy link
Copy Markdown
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 (react#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. Fixesreact#37102 .
## Changelog:
[iOS] [Fixed] - Fix Fabric issue with React-Core pod when Xcode project has multiple targets
Pull Request resolved: react#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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Type: New ArchitectureIssues 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

@douglowder@analysis-bot@facebook-github-bot@Pranav-yadav