[iOS] Remove TARGET_OS_UIKITFORMAC macros #42278
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
Base commit: f30acc6 |
|
@sammy-SC has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cipolleschi
approved these changes
Jan 15, 2024
Trust me, it makes my life just as much easier 🤣 |
Saadnajmi
added a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 26, 2024
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:  Reviewed By: cipolleschi Differential Revision: D52780690 Pulled By: sammy-SC fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
Saadnajmi
added a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 26, 2024
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:  Reviewed By: cipolleschi Differential Revision: D52780690 Pulled By: sammy-SC fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
Saadnajmi
added a commit
to microsoft/react-native-macos
that referenced
this pull request
Jan 27, 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:  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
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:  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
added a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 29, 2024
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:  Reviewed By: cipolleschi Differential Revision: D52780690 Pulled By: sammy-SC fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
Saadnajmi
added a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 29, 2024
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:  Reviewed By: cipolleschi Differential Revision: D52780690 Pulled By: sammy-SC fbshipit-source-id: df6a333e8e15f79de0ce6f538ebd73b92698dcb6
Saadnajmi
added a commit
to microsoft/react-native-macos
that referenced
this pull request
Jan 29, 2024
* Remove deprecated uses of UIActivityIndicatorViewStyle (facebook#38208) Summary: Super simple change to replace some deprecated ENUM values with the correct one, now that we're on iOS 13.0+. From `UIActivityIndicator.h`: ``` typedef NS_ENUM(NSInteger, UIActivityIndicatorViewStyle) { UIActivityIndicatorViewStyleMedium API_AVAILABLE(ios(13.0), tvos(13.0)) = 100, UIActivityIndicatorViewStyleLarge API_AVAILABLE(ios(13.0), tvos(13.0)) = 101, UIActivityIndicatorViewStyleWhiteLarge API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleLarge", ios(2.0, 13.0), tvos(9.0, 13.0)) = 0, UIActivityIndicatorViewStyleWhite API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0), tvos(9.0, 13.0)) = 1, UIActivityIndicatorViewStyleGray API_DEPRECATED_WITH_REPLACEMENT("UIActivityIndicatorViewStyleMedium", ios(2.0, 13.0)) API_UNAVAILABLE(tvos) = 2, }; ``` ## Changelog: [IOS] [CHANGED] - Remove deprecated uses of UIActivityIndicatorViewStyle Pull Request resolved: facebook#38208 Test Plan: CI should pass Reviewed By: cipolleschi Differential Revision: D47254035 Pulled By: rshest fbshipit-source-id: 6d3270899e8d52611d91c777f324c3c6f0c520be * Refactor RCTActivityIndicator * 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:  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 * DeviceInfo: Simplify RCTExportedDimensions's API Summary: RCTExportedDimensions doesn't need access to the ModuleRegistry, or the bridge. It just uses those two things to get the fontScale. We could make RCTExportedDimensions easier to understand, by making it do fewer things (i.e: computing the fontScale up front, and passing it into RCTExportedDimensions). Let's just do that. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D48237715 fbshipit-source-id: b3af648d88276846742d0e1192d33d180ee49dbb * iOS: trigger didUpdateDimensions event when resizing without changing traits (facebook#37649) Summary: - when using the `useWindowDimensions()` hook in a component, it's possible for the hook to report incorrect sizes and not update as frequently as it should - this is most applicable to apps built for iPad and macOS - closes facebook#36118 - either when resizing a React Native app to a different [Size Class](https://developer.apple.com/design/human-interface-guidelines/layout#Device-size-classes) or changing the Appearance, we dispatch an `RCTUserInterfaceStyleDidChangeNotification` notification - these are then handled in the `interfaceFrameDidChange` method of `RCTDeviceInfo` - this results in a `didUpdateDimensions` Device Event, which in turn updates the results of `useWindowDimensions()` - see [RCTDeviceInfo.mm#L217-L232](https://github.com/facebook/react-native/blob/v0.72.0-rc.4/packages/react-native/React/CoreModules/RCTDeviceInfo.mm#L217-L232) - and [Dimensions.js#L119-L124](https://github.com/facebook/react-native/blob/v0.72.0-rc.4/packages/react-native/Libraries/Utilities/Dimensions.js#L119-L124) 🐛 **However** 🐛 - if you are resizing without triggering a `UITraitCollection` change, the Dimensions reported by `useWindowDimensions()` can become stale, until you either:- - hit a certain width that is considered a different Size Class - change the Appearance - background then resume the app - make the app full-screen - added a new `RCTRootViewFrameDidChangeNotification` notification - the thinking here is to avoid additional overhead by re-using the same `RCTUserInterfaceStyleDidChangeNotification` - maybe it's overkill? - the new notifications are sent from an override of `setFrame` on `RCTRootView` - the new notifications call the same `interfaceFrameDidChange` method of `RCTDeviceInfo` - Dimensions are now reported correctly when resizing; even within the same Size Class <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [FIXED] - Dimensions could be reported incorrectly when resizing iPad or macOS apps Pull Request resolved: facebook#37649 Test Plan: **Reproduction: https://github.com/jpdriver/Dimensions** or to recreate it yourself:- - Generate a new project - Change App.tsx ``` import * as React from 'react'; import {View, Text, useWindowDimensions} from 'react-native'; export default function App() { const {width, height} = useWindowDimensions(); return ( <View style={{flex: 1, justifyContent: 'center', alignItems: 'center'}}> <Text>Width: {width}</Text> <Text>Height: {height}</Text> </View> ); } ``` - Open the iOS project in Xcode and enable iPad support - Enable the iPad app to be used in any orientation - Run the app - Enable Stage Manager - Drag one of the corners to resize the app Reviewed By: javache Differential Revision: D46335537 Pulled By: NickGerleman fbshipit-source-id: 1533f511cf3805fdc9629a2ee115cc00e204d82c * Refactor RCTDeviceInfo * Rename `RCTRootViewFrameDidChangeNotification` as it's not trac… (facebook#39835) Summary: …king root view frame changes Looking through where this was introduced (facebook#37649), it seems the notification went from tracking root view size changes to window size changes. However, it was not renamed. I was using it for root view changes in RN-macOS, which.. I guess I'll refactor. Meanwhile, let's update the name? ## Changelog: [IOS] [CHANGED] - Rename `RCTRootViewFrameDidChangeNotification` as it's not tracking root view frame changes Pull Request resolved: facebook#39835 Test Plan: CI should pass Reviewed By: cipolleschi Differential Revision: D50173742 Pulled By: javache fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c * Default dimensions `scale` value to 1.0 if window is nil (#1970) * [0.73] Default scale to 1.0 on if window is nil * Unify RCTExportedDimensions between iOS & macOS * Use typedef instead of #define for NSWindow * Fix issues from cherry-picks * Add visionOS support * Extra changes for visionOS on 0.72-stable * Use Xcode 15.2 --------- Co-authored-by: Ramanpreet Nara <ramanpreet@meta.com> Co-authored-by: JP <jpdriver@users.noreply.github.com> Co-authored-by: Shawn Dempsey <96719+shwanton@users.noreply.github.com>
shirakaba
added a commit
to scoville/react-native-app-auth
that referenced
this pull request
Jul 27, 2024
Based on these: facebook/react-native#42278
shirakaba
added a commit
to scoville/react-native-app-auth
that referenced
this pull request
Aug 18, 2024
Based on these: facebook/react-native#42278
shirakaba
added a commit
to scoville/react-native-app-auth
that referenced
this pull request
Aug 18, 2024
Based on these: facebook/react-native#42278
shirakaba
added a commit
to scoville/react-native-app-auth
that referenced
this pull request
Oct 30, 2024
Based on these: facebook/react-native#42278
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
There seems to be a lot of
TARGET_OS_UIKITFORMACmacro in React Native that don't need to be there. Let's remove them.First off, what is
TARGET_OS_UIKITFORMACtargeting? You might think it's Mac Catalyst, if you look at the commit introducing the ifdefs. However.. that doesn't seem right becauseTARGET_OS_MACCATALYSTexists, and is used elsewhere in the codebase. In fact, if you look at this handy comment insideTargetConditionals.h(the file that defines all these conditionals),TARGET_OS_UIKITFORMACis not even on there!Going even deeper into
TargetConditionals.h, you will seeTARGET_OS_UIKITFORMACdefined... and it's always 1 whenTARGET_OS_MACCATALYSTis 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?
Changelog:
[IOS] [REMOVED] - Remove TARGET_OS_UIKITFORMAC macros
Test Plan:
RNTester with Mac Catalyst still compiles:
