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

Add visionOS support #2019

Merged
merged 3 commits into from
Jan 27, 2024
Merged

Add visionOS support #2019

merged 3 commits into from
Jan 27, 2024

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Jan 8, 2024

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary:

This is the last PR (for now) in a series of PRs to add visionOS support to React Native macOS

Add the actual native support for visionOS. This is done mostly with nicely placed ifdefs. Some notes:

  • Amusingly, many places where we added macOS ifdefs are also places we need vision ifdefs :D
  • In places where we started having nested ifdefs (#if !TARGET_OS_OSX && !TARGET_OS_VISION), the code inside is usually an iOS only API, and therefore the most correct ifdef to use is #if TARGET_OS_IOS. As such, there are a couple of places where I updated the usage.
  • Much of this is cross-referenced with the same support added in https://github.com/callstack/react-native-visionos

Changelog:

[INTERNAL] [ADDED] - Add visionOS support

Test Plan:

We can't test the RNTester-visionOS in CI yet (sadly), as we need an Arm64 image of macOS with the visionOS SDK to build visionOS, and Azure Pipelines doesn't seem to have that currently. Instead, let's locally verify that the scheme builds and runs. I took this screenshot with #2056 also cherry-picked because as of the time of writing, that PR hasn't landed yet.

Screenshot 2024-01-25 at 10 23 47 PM

Summary:
There seems to be a lot of `TARGET_OS_UIKITFORMAC` macro in React Native that don't need to be there. Let's remove them.

First off, what is `TARGET_OS_UIKITFORMAC` targeting? You might think it's [Mac Catalyst](https://developer.apple.com/mac-catalyst/), if you look at the [commit](facebook@3724810) introducing the ifdefs.  However.. that doesn't seem right because `TARGET_OS_MACCATALYST` exists, and is used elsewhere in the codebase. In fact, if you look at this handy comment inside `TargetConditionals.h` (the file that defines all these conditionals), `TARGET_OS_UIKITFORMAC` is not even on there!

```
/*
 *  TARGET_OS_*
 *
 *  These conditionals specify in which Operating System the generated code will
 *  run.  Indention is used to show which conditionals are evolutionary subclasses.
 *
 *  The MAC/WIN32/UNIX conditionals are mutually exclusive.
 *  The IOS/TV/WATCH/VISION conditionals are mutually exclusive.
 *
 *    TARGET_OS_WIN32              - Generated code will run on WIN32 API
 *    TARGET_OS_WINDOWS            - Generated code will run on Windows
 *    TARGET_OS_UNIX               - Generated code will run on some Unix (not macOS)
 *    TARGET_OS_LINUX              - Generated code will run on Linux
 *    TARGET_OS_MAC                - Generated code will run on a variant of macOS
 *      TARGET_OS_OSX                - Generated code will run on macOS
 *      TARGET_OS_IPHONE             - Generated code will run on a variant of iOS (firmware, devices, simulator)
 *        TARGET_OS_IOS                - Generated code will run on iOS
 *          TARGET_OS_MACCATALYST        - Generated code will run on macOS
 *        TARGET_OS_TV                 - Generated code will run on tvOS
 *        TARGET_OS_WATCH              - Generated code will run on watchOS
 *        TARGET_OS_VISION             - Generated code will run on visionOS
 *        TARGET_OS_BRIDGE             - Generated code will run on bridge devices
 *      TARGET_OS_SIMULATOR          - Generated code will run on an iOS, tvOS, watchOS, or visionOS simulator
 *      TARGET_OS_DRIVERKIT          - Generated code will run on macOS, iOS, tvOS, watchOS, or visionOS
 *
 *    TARGET_OS_EMBEDDED           - DEPRECATED: Use TARGET_OS_IPHONE and/or TARGET_OS_SIMULATOR instead
 *    TARGET_IPHONE_SIMULATOR      - DEPRECATED: Same as TARGET_OS_SIMULATOR
 *    TARGET_OS_NANO               - DEPRECATED: Same as TARGET_OS_WATCH
 *
 *    +--------------------------------------------------------------------------------------+
 *    |                                    TARGET_OS_MAC                                     |
 *    | +-----+ +------------------------------------------------------------+ +-----------+ |
 *    | |     | |                  TARGET_OS_IPHONE                          | |           | |
 *    | |     | | +-----------------+ +----+ +-------+ +--------+ +--------+ | |           | |
 *    | |     | | |       IOS       | |    | |       | |        | |        | | |           | |
 *    | | OSX | | | +-------------+ | | TV | | WATCH | | BRIDGE | | VISION | | | DRIVERKIT | |
 *    | |     | | | | MACCATALYST | | |    | |       | |        | |        | | |           | |
 *    | |     | | | +-------------+ | |    | |       | |        | |        | | |           | |
 *    | |     | | +-----------------+ +----+ +-------+ +--------+ +--------+ | |           | |
 *    | +-----+ +------------------------------------------------------------+ +-----------+ |
 *    +--------------------------------------------------------------------------------------+
 */
```

Going even deeper into `TargetConditionals.h`, you will see `TARGET_OS_UIKITFORMAC` defined... and it's always 1 when `TARGET_OS_MACCATALYST` is 1, making it feel even more redundant. My current conclusion is it's either another variant of Mac Catalyst (the one where they just run unmodified UIKit maybe..), or it's an older macro back from when Catalyst was still experimental.

Either way, it's pretty obvious nobody is running or testing this codepath, and it adds bloat, especially to React Native macOS where we have extra ifdef blocks for macOS support (and eventually visionOS support). Let's remove it.

Another change I made while we're here:
I've seen this lingering TODO to replace setTargetRect:InView: / setMenuVisible:animated: (deprecated as of iOS 13, below our minimum OS requirement) with showMenuFromView (deprecated as of iOS 16, in line with the availability check). Let's just.... do that?

[IOS] [REMOVED] - Remove TARGET_OS_UIKITFORMAC macros

Pull Request resolved: facebook#42278

Test Plan:
RNTester with Mac Catalyst still compiles:
![Screenshot 2024-01-15 at 12 26 03 AM](https://github.com/facebook/react-native/assets/6722175/015bd37d-f536-43c7-9586-96187cdbd013)

Reviewed By: cipolleschi

Differential Revision: D52780690

Pulled By: sammy-SC

fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
…nuController` (facebook#42277)

Summary:
`UIMenuController` is deprecated as of iOS 16. facebook@e08a197 migrated a usage into an `available` check. However, it does not properly fall back to the deprecated API in the "else" block of the availability check, instead it uses an early return. It seems this means Xcode still sees the API as used, and spits out a deprecated warning. Let's just refactor the code so we don't have that anymore.

[IOS] [FIXED] -  Remove an early return to suppress a deprecated API warning for `UIMenuController`

Pull Request resolved: facebook#42277

Test Plan: CI should pass.

Reviewed By: cipolleschi

Differential Revision: D52785488

Pulled By: sammy-SC

fbshipit-source-id: 0b47e8aa8d7c94728e3d68332fbb8f97f8ded34e
@Saadnajmi Saadnajmi merged commit a064b85 into microsoft:main Jan 27, 2024
16 checks passed
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 28, 2024
* Remove TARGET_OS_UIKITFORMAC macros (facebook#42278)

Summary:
There seems to be a lot of `TARGET_OS_UIKITFORMAC` macro in React Native that don't need to be there. Let's remove them.

First off, what is `TARGET_OS_UIKITFORMAC` targeting? You might think it's [Mac Catalyst](https://developer.apple.com/mac-catalyst/), if you look at the [commit](facebook@3724810) introducing the ifdefs.  However.. that doesn't seem right because `TARGET_OS_MACCATALYST` exists, and is used elsewhere in the codebase. In fact, if you look at this handy comment inside `TargetConditionals.h` (the file that defines all these conditionals), `TARGET_OS_UIKITFORMAC` is not even on there!

```
/*
 *  TARGET_OS_*
 *
 *  These conditionals specify in which Operating System the generated code will
 *  run.  Indention is used to show which conditionals are evolutionary subclasses.
 *
 *  The MAC/WIN32/UNIX conditionals are mutually exclusive.
 *  The IOS/TV/WATCH/VISION conditionals are mutually exclusive.
 *
 *    TARGET_OS_WIN32              - Generated code will run on WIN32 API
 *    TARGET_OS_WINDOWS            - Generated code will run on Windows
 *    TARGET_OS_UNIX               - Generated code will run on some Unix (not macOS)
 *    TARGET_OS_LINUX              - Generated code will run on Linux
 *    TARGET_OS_MAC                - Generated code will run on a variant of macOS
 *      TARGET_OS_OSX                - Generated code will run on macOS
 *      TARGET_OS_IPHONE             - Generated code will run on a variant of iOS (firmware, devices, simulator)
 *        TARGET_OS_IOS                - Generated code will run on iOS
 *          TARGET_OS_MACCATALYST        - Generated code will run on macOS
 *        TARGET_OS_TV                 - Generated code will run on tvOS
 *        TARGET_OS_WATCH              - Generated code will run on watchOS
 *        TARGET_OS_VISION             - Generated code will run on visionOS
 *        TARGET_OS_BRIDGE             - Generated code will run on bridge devices
 *      TARGET_OS_SIMULATOR          - Generated code will run on an iOS, tvOS, watchOS, or visionOS simulator
 *      TARGET_OS_DRIVERKIT          - Generated code will run on macOS, iOS, tvOS, watchOS, or visionOS
 *
 *    TARGET_OS_EMBEDDED           - DEPRECATED: Use TARGET_OS_IPHONE and/or TARGET_OS_SIMULATOR instead
 *    TARGET_IPHONE_SIMULATOR      - DEPRECATED: Same as TARGET_OS_SIMULATOR
 *    TARGET_OS_NANO               - DEPRECATED: Same as TARGET_OS_WATCH
 *
 *    +--------------------------------------------------------------------------------------+
 *    |                                    TARGET_OS_MAC                                     |
 *    | +-----+ +------------------------------------------------------------+ +-----------+ |
 *    | |     | |                  TARGET_OS_IPHONE                          | |           | |
 *    | |     | | +-----------------+ +----+ +-------+ +--------+ +--------+ | |           | |
 *    | |     | | |       IOS       | |    | |       | |        | |        | | |           | |
 *    | | OSX | | | +-------------+ | | TV | | WATCH | | BRIDGE | | VISION | | | DRIVERKIT | |
 *    | |     | | | | MACCATALYST | | |    | |       | |        | |        | | |           | |
 *    | |     | | | +-------------+ | |    | |       | |        | |        | | |           | |
 *    | |     | | +-----------------+ +----+ +-------+ +--------+ +--------+ | |           | |
 *    | +-----+ +------------------------------------------------------------+ +-----------+ |
 *    +--------------------------------------------------------------------------------------+
 */
```

Going even deeper into `TargetConditionals.h`, you will see `TARGET_OS_UIKITFORMAC` defined... and it's always 1 when `TARGET_OS_MACCATALYST` is 1, making it feel even more redundant. My current conclusion is it's either another variant of Mac Catalyst (the one where they just run unmodified UIKit maybe..), or it's an older macro back from when Catalyst was still experimental.

Either way, it's pretty obvious nobody is running or testing this codepath, and it adds bloat, especially to React Native macOS where we have extra ifdef blocks for macOS support (and eventually visionOS support). Let's remove it.

Another change I made while we're here:
I've seen this lingering TODO to replace setTargetRect:InView: / setMenuVisible:animated: (deprecated as of iOS 13, below our minimum OS requirement) with showMenuFromView (deprecated as of iOS 16, in line with the availability check). Let's just.... do that?

[IOS] [REMOVED] - Remove TARGET_OS_UIKITFORMAC macros

Pull Request resolved: facebook#42278

Test Plan:
RNTester with Mac Catalyst still compiles:
![Screenshot 2024-01-15 at 12 26 03 AM](https://github.com/facebook/react-native/assets/6722175/015bd37d-f536-43c7-9586-96187cdbd013)

Reviewed By: cipolleschi

Differential Revision: D52780690

Pulled By: sammy-SC

fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6

* Remove an early return to suppress a deprecated API warning for `UIMenuController` (facebook#42277)

Summary:
`UIMenuController` is deprecated as of iOS 16. facebook@e08a197 migrated a usage into an `available` check. However, it does not properly fall back to the deprecated API in the "else" block of the availability check, instead it uses an early return. It seems this means Xcode still sees the API as used, and spits out a deprecated warning. Let's just refactor the code so we don't have that anymore.

[IOS] [FIXED] -  Remove an early return to suppress a deprecated API warning for `UIMenuController`

Pull Request resolved: facebook#42277

Test Plan: CI should pass.

Reviewed By: cipolleschi

Differential Revision: D52785488

Pulled By: sammy-SC

fbshipit-source-id: 0b47e8aa8d7c94728e3d68332fbb8f97f8ded34e

* Native changes for visionOS
@Saadnajmi Saadnajmi mentioned this pull request Jan 29, 2024
4 tasks
@Saadnajmi Saadnajmi deleted the visionos branch February 18, 2024 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants