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: add conditionals for iOS only code in RCTDeviceInfo.mm #45176

Closed
wants to merge 1 commit into from

Conversation

okwasniewski
Copy link
Contributor

Summary:

Having React Native support every Apple platform is tough to achieve as it introduces many platform-specific ifdefs.

On the other side, maintaining an OOT platform fork is already a demanding job, so to make it easier I propose adding ifdefs for iOS-specific code. Thanks to this change, OOT platforms can focus on their OS-specific features while the core is also adding iOS-specific features behind ifdefs. Fortunately, most of the code on Apple platforms can be shared and this PR aims to introduce better support for this and to minimize OOT fork's surface.

In this example RCTDeviceInfo.mm has support for handling orientation changes and the availability of this feature across Apple OS looks as follows:

Platform Support
macOS
tvOS
visionOS
iOS/iPadOS

Here is a table from TargetConditionals.h header file which shows the coverage of TARGET_OS_IOS macro. (It supports both iOS and iPadOS)

CleanShot 2024-06-26 at 11 51 47@2x

Changelog:

[INTERNAL] [ADDED] - Conditionals for iOS only code in RCTDeviceInfo.mm

Test Plan:

CI Green

@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 Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jun 26, 2024
@okwasniewski okwasniewski changed the title afeat: add conditionals for iOS only code in RCTDeviceInfo.mm feat: add conditionals for iOS only code in RCTDeviceInfo.mm Jun 26, 2024
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 20,299,696 -14
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 23,496,902 -5
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: d3e0430
Branch: main

@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 Jun 28, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in c5653a0.

Copy link

This pull request was successfully merged by @okwasniewski in c5653a0.

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

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.

None yet

4 participants