Skip to content

[iOS] Rename RCTRootViewFrameDidChangeNotification as it's not trac… - #39835

Closed
Saadnajmi wants to merge 1 commit into
react:mainfrom
Saadnajmi:names-root
Closed

[iOS] Rename RCTRootViewFrameDidChangeNotification as it's not trac…#39835
Saadnajmi wants to merge 1 commit into
react:mainfrom
Saadnajmi:names-root

Conversation

@Saadnajmi

Copy link
Copy Markdown
Contributor

…king root view frame changes

Summary:

Looking through where this was introduced (#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

Test Plan:

CI should pass

@facebook-github-botfacebook-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: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 5, 2023
@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a16,901,417-416,185
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a20,380,170-305,544
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: 1b9b13a
Branch: main

@github-actions

Copy link
Copy Markdown
Warnings
⚠️

Can't find the E2E test log for ci/circleci: test_e2e_ios. Job link

⚠️

Can't find the E2E test log for ci/circleci: test_e2e_ios. Job link

Generated by 🚫 dangerJS against 5df8132

@jpdriver

Copy link
Copy Markdown
Contributor

naming things is hard 🤦🏻‍♂️

i think you’re right — the current name doesn’t really make sense anymore. sorry it wasn’t changed during review of the initial PR.

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

naming things is hard 🤦🏻‍♂️

i think you’re right — the current name doesn’t really make sense anymore. sorry it wasn’t changed during review of the initial PR.

No worries! I'm still planning a follow up change/proposal to listen/fire the notification in viewDidMoveToWindow, but I need to do some testing on it first. If that works, the notification can be contained to the hosting view so that users don't need to rely on RCTAppDelegate in cases like brownfield apps :)

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@cipolleschicipolleschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Saadnajmi, thanks for the change, but as you can already imagine, from me writing here... This is a breaking change!

We don't know who is using the RCTRootViewFrameDidChangeNotification constant exported in the header. :(

As a rule of thumb, every time we touch a header file it will be a breaking change. The only case when it is not is when we add something to the header.

Can we change the pr to deprecate the old notification? 🙏

Comment threadpackages/react-native/React/Base/RCTConstants.h
@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

Hi @Saadnajmi, thanks for the change, but as you can already imagine, from me writing here... This is a breaking change!

We don't know who is using the RCTRootViewFrameDidChangeNotification constant exported in the header. :(

As a rule of thumb, every time we touch a header file it will be a breaking change. The only case when it is not is when we add something to the header.

Can we change the pr to deprecate the old notification? 🙏

@cipolleschi I should have expected it was breaking, but I had somewhat hope it wouldn't count 😅. I can tell you who used it, I did :P!

Before I go change the PR to a deprecation notice, I did want to talk about the implementation a bit. Even after this change, there could be a place for RCTRootViewFrameDidChangeNotification to signal when the root view frame changes, but not necessarily the window frame. In practical terms, I don't know when this would ever happen, across Greenfield and brownfield experiences alike. I think we'd be fine ignoring that case unless it becomes a bug later.

Furthermore, the original change now requires that your window be set up by RCTAppDelegate, or else useWindowDimensions won't work. That's fine for most Greenfield apps, but won't work in brownfield apps where you're just hosting an RCTRootView / RCTSurface and you may not have control of the window like that. Because of that I had a proposal for a set of changes

  1. Rename the notification
  2. Override your [RCTRootView didMoveToWindow:] to get a pointer to your views' window
  3. Listen for frame changes on the window that way.

However, afaik there isn't actually a way to do (3) safely without using KVO on UIKit 😞. Ironically, I did find a way with Appkit.. the one time that framework is more convenient 😄. I'll keep on exploring options but if there isn't a way to keep all this logic inside the root view / brownfield friendly, then this change feels less impactful to me. I can still do it with the deprecation notice for correctness sake.

That's the background, I'd be curious if other people have thoughts on how to do (3) better. Meanwhile, I'll work on making this a deprecation PR.

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

Actually, @cipolleschi I think the original change only made it to 0.73 RC0, not 0.72 and co: 61861d2

So if it changes during the RC phase, would it no longer be considered a breaking change?

@javache

Copy link
Copy Markdown
Contributor

I think given this landed recently it's worth fixing the name before we ship 0.73.

@cipolleschi

Copy link
Copy Markdown
Contributor

Ok, yes, so... we will need to land this and then to cherry pick it into 0.73! 👍

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

Copy link
Copy Markdown
Contributor

@javache merged this pull request in 625d0ec.

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

Ok, yes, so... we will need to land this and then to cherry pick it into 0.73! 👍

I made a pick request.

huntie pushed a commit that referenced this pull request Oct 18, 2023
)
Summary:
…king root view frame changes
Looking through where this was introduced (#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: #39835
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D50173742
Pulled By: javache
fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c
Saadnajmi added a commit to microsoft/react-native-macos that referenced this pull request Nov 7, 2023
* Set Hermes release version
* bumped packages versions
#publish-packages-to-npm
* Bump react-native-babel-preset to 0.73.18 #publish-packages-to-npm
* Enable native view configs in bridgeless mode in OSS (react#39476)
Summary:
Pull Request resolved: react#39476
This diff enables native view config interop layer in bridgeless mode by default for OSS.
It also removes redundant `enableNativeViewConfigsInBridgelessMode` JS feature flag.
Changelog: [General][Added] - Native view config interop layer enabled in bridgeless mode.
Reviewed By: luluwu2032
Differential Revision: D49318325
fbshipit-source-id: ea2e38b2ea10637b578d98e38d97eed923498fb3
* Enable Template with Bridgeless (react#39661)
Summary:
Pull Request resolved: react#39661
Changelog:
[Android][Changed] - Enable Template with Bridgeless
Reviewed By: cortinico
Differential Revision: D49464580
fbshipit-source-id: 26dd8e2a0b9a66421d58f147eed4d2fd28817dc7
* [Hotfix] Remove node engines constraint for normalize-color (react#39698)
* Bump CLI to 12.0.0-alpha.16
* bumped packages versions
#publish-packages-to-npm
* Reduce visibility of functions in ReactHostImpl (react#39640)
Summary:
Pull Request resolved: react#39640
changelog: [internal] internal
Reducing visibility from public to package only for `ReactHostImpl.getDefaultHardwareBackBtnHandler()` since it's only used within package
Reviewed By: mdvacca
Differential Revision: D49612859
fbshipit-source-id: 3c40888da732f33dc046d9363b08119e707f4ea4
* Enable TurboModule interop in Bridgeless Mode (react#39687)
Summary:
Pull Request resolved: react#39687
Bridgeless mode is releasing in 0.73.
Bridgeless mode isn't stable inside RNTester (in 0.73) without the TurboModule interop layer: T164853040.
**Changes:** Enable the TurboModule interop layer in RNTester (Bridgeless Mode).
**Urgency:** This will unblock the 0.73.
Changelog: [General][Added] - Enable TurboModule interop in Bridgeless mode
Reviewed By: fkgozali, dmytrorykun, luluwu2032
Differential Revision: D49653919
fbshipit-source-id: a4a09a99544c2ef8713484dbeab5cb04f29256cc
* Export Commands and Constants only if native view config interop is enabled (react#39696)
Summary:
Pull Request resolved: react#39696
`Commands` and `Constants` should be set in native only if component data is instantiated via native view config interop layer.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D49684166
fbshipit-source-id: ceaa29c2ed3336aa6e21a116a3f5f94e03c225c1
* Bump CLI to 12.0.0-alpha.17
* Bump package versions
#publish-packages-to-npm
* Update Xcode 15 patches to be more robust (react#39710)
Summary:
Pull Request resolved: react#39710
Last week Apple released Xcode 15, which required us to ship a workaround for the new linker.
Unfortunately, the previous fix was not good enough and there were some edge cases that were not covered.
For example, in some occasions the flags are read as an array and the `-Wl` and the `-ld_classic` flags were separated and not properly removed when moving from Xcode 15 to Xcpde 14.3.1.
This change fixes those edge cases, with a more robust solution where:
- We convert the flags to a string.
- We trim the string and the values properly.
- We add the flags when running `pod install` with Xcode 15 as the default iOS toolchain.
- We remove the flags when running `pod install` with Xcode <15 as the default iOS toolchain.
## Changelog:
[Internal] - Make the Xcode 15 workaround more robust.
Reviewed By: dmytrorykun
Differential Revision: D49748844
fbshipit-source-id: 34976d148f123c5aacba6487a500874bb938fe99
* Sync debugger-frontend
Summary:
Sync latest `debugger-frontend` artifacts, based on motiz88/rn-chrome-devtools-frontend#12.
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D49502838
fbshipit-source-id: 25805066670ac35ec976dc820a73a722ee182dd2
* Mark initHybrid as @JvmStatic (react#39755)
Summary:
Pull Request resolved: react#39755
The goal of this diff is to fix:
```
JNI DETECTED ERROR IN APPLICATION: JNI NewGlobalRef called with pending exception java.lang.NoSuchMethodError: no static or non-static method
"Lcom/facebook/react/jscexecutor/JSCExecutor;.initHybrid(Lcom/facebook/react/bridge/ReadableNativeMap;)Lcom/facebook/jni/HybridData;"
```
changelog: [internal] internal
Reviewed By: luluwu2032
Differential Revision: D49831595
fbshipit-source-id: 9ce22cdccdd02af74edb27be2df72a469d3166c9
* Bump package versions
#publish-packages-to-npm
* Bump package versions (dependant packages)
#publish-packages-to-npm
* [0.73.0-rc.0] Bump version numbers
* Fix typings for ReactNativeVersion.js
This is an eager cherry pick of react#39784 for `0.73-stable`.
* Revert "[0.73.0-rc.0] Bump version numbers"
This reverts commit 6b9cf50.
* [RN][CI] Make the Choose CI Job run also on tags (react#39774)
* [0.73.0-rc.0] Bump version numbers
* Revert "Fix typings for ReactNativeVersion.js"
This reverts commit b6c0703.
* Fix typings for ReactNativeVersionCheck
Applies correct fix for `_formatVersion` misalignment — based on react#36657.
* Fix start command exit behaviour
This is an eager cherry pick of react#39788 for `0.73-stable`.
* Bump package versions
#publish-packages-to-npm
* [0.73.0-rc.1] Bump version numbers
* Revert "[0.73.0-rc.1] Bump version numbers"
This reverts commit 725b1c9.
* Make jobs required for hermes to run on tagged builds (react#39800)
* [0.73.0-rc.1] Bump version numbers
* Fix broken Loading/Refreshing indicator on Android
Summary:
The Loading.../Refreshing... indicator is currently broken on Android.
The reason is related to D42599220
We used to have a Toast shown to users on Android as a fallback, but as the
DevLoadingView is not always loaded as a module in the core package, this ends up in the banner never beign shown to the user (on RN Tester or template apps).
Changelog:
[Android] [Fixed] - Fix broken Loading/Refreshing indicator on Android
Reviewed By: cipolleschi
Differential Revision: D49876757
fbshipit-source-id: 400e002327ebca908e3e7a7f81c5066888ac4e9b
* Remove unneeded write to .version file (react#39807)
Summary:
Pull Request resolved: react#39807
We used to need this file so that we could read the react native version when [creating Hermes artifacts](react@e4b5d3e#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47R1507). Originally, that change was introduced [here](react@e4b5d3e#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47)
Despite the fact that that approach was wrong, as we already have the right version in the package.json which is guaranteed to be present, a lot has changed since then and we don't need that file anymore.
## Changelog:
[Internal] - Remove lines that write a .version file while releasing on npm
Reviewed By: lunaleaps
Differential Revision: D49909718
fbshipit-source-id: bd23d6d73001d0b58bf6b0321ed6d4ceb3523e7a
* Default to native view configs in bridged mode and to static view configs in bridgeless mode (react#39704)
Summary:
Pull Request resolved: react#39704
Default to native view configs in bridged mode and to static view configs in bridgeless mode.
Remove `setRuntimeConfigProvider` calls from RNTester and from the Template.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D49687252
fbshipit-source-id: 140e1c510ba3fbc153978b59c8bb4b4e35bc7571
* Fix Gemfile, setting Active support to < 7.1.0 (react#39828)
Summary:
Pull Request resolved: react#39828
Active Suppert released a new Gem which is incompatible with Cocoapods 1.13.0, the latest release, as they removed a method used by cocoapods.
This fix ensures that we install compatible versions of the Gem.
## Changelog:
[iOS][Fixed] - Set the max version of Active support to 7.0.8
Reviewed By: hoxyq
Differential Revision: D49949782
fbshipit-source-id: 278097502d3a416567cc8c0b90090fee4fb21503
* fix: view flips on RTL with new transform origin changes in Paper arch. (react#39803)
Summary:
Fixes - react#38626 (comment). Explained the issue [here](react#38626 (comment)).
## Changelog:
[IOS] [FIXED] - View flips horizontally in paper arch on RTL
Pull Request resolved: react#39803
Test Plan: Run RNTester in Paper mode on iOS
Reviewed By: NickGerleman
Differential Revision: D49952227
Pulled By: lunaleaps
fbshipit-source-id: 7240552e499765859dceea0a0406561cc3a3148f
* [iOS] Deprecate RCTGetMultiplierForContentSizeCategory (react#39785)
* Set hermes for Android template project in testing
Summary:
Changelog: [Internal] - Set the hermes value as specified by the test-e2e-local script flag. Right now, the script incorrectly ignores the flag
By default, the template project has `hermesEnabled=true`
Reviewed By: cipolleschi
Differential Revision: D49831355
fbshipit-source-id: 7fb8613fa86f2c6140b7d25b16aeb583e6e26c12
* fix missing sed import
* Update pod for CI
* Update .gitignore for sdks/downloads
* [0.73.0-rc.2] Bump version numbers
* Add --experimental-debugger-frontend flag, restore 0.72 flow as base (react#40766)
Summary:
Pull Request resolved: react#40766
This changeset allows users to opt into the new debugger frontend experience by passing `--experimental-debugger` to `react-native start`. **We are defaulting this option to `true`** for now, but will continue to evaluate this feature before 0.73 ships. It restores Flipper (via `flipper://`) as the default handling for `/open-debugger` (matching 0.72 behaviour) when this flag is not enabled.
Detailed changes:
- Replaces `enableCustomDebuggerFrontend` experiment in `dev-middleware` with `enableNewDebugger`. The latter now hard-swaps between the Flipper and new launch flows.
- Removes now-unused switching of `devtoolsFrontendUrl`.
- Implements `deprecated_openFlipperMiddleware` (matching previous RN CLI implementation).
- Disables "`j` to debug" key handler by default.
- Marks "`j` to debug" and `/open-debugger` console logs as experimental.
Changelog:
[Changed][General] Gate new debugger frontend behind `--experimental-debugger` flag, restore Flipper as base launch flow
Reviewed By: motiz88
Differential Revision: D50084590
fbshipit-source-id: 5234634f20110cb7933b1787bd2c86f645411fff
* [0.73] Make sure template is consuming the right buildToolsVersion (react#39957)
* Store hermes stable artifacts inside Pods directory (react#40733)
Summary:
Pull Request resolved: react#40733
Node package managers may purge or recreate `node_modules/react-native` when adding/removenf project dependencies. Storing hermes iOS artifacts inside `node_modules/react-native/sdks` is not reliable.
This diff moves those artifacts to `Pods/hermes-engine-artifacts`.
Should fixreact#39903
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D50081559
fbshipit-source-id: a130898e12fb6275cadaef7617bf4b6a09e6487e
* Top-down onLayout events (react#39644)
Summary:
Pull Request resolved: react#39644
This makes Android Paper/Classic renderer fire `onLayout` events top down, like in Fabric/new Architecture. This gives a much more sane model for using layout events to calculate bottom/right-edge insets.
I was under the impression that Paper in general was bottom-up, but it turns out that is only true for Android and Windows (iOS seems totally deterministic).
This is a behavior change, but to my knowledge was never hit during the Fabric migration, and any JS code already written for both Android and iOS cannot make assumptions here anyways.
Changelog:
[General][Changed] - Make layout events top-down on Android classic renderer
Reviewed By: mdvacca
Differential Revision: D49627996
fbshipit-source-id: 29964b421dd420681d45348c7db16f211a6c087f
* Deterministic onLayout event ordering for iOS Paper (react#40748)
Summary:
Pull Request resolved: react#40748
The ordering of `onLayout` events is non-deterministic on iOS Paper, due to nodes being added to an `NSHashTable` before iteration, instead of an ordered collection.
We don't do any lookups on the collection, so I think this was chosen over `NSMutableArray` for the sake of `[NSHashTable weakObjectsHashTable]`, to avoid retain/release. Using a collection which does retain/release seems to cause a crash due to double release or similar, so those semantics seem intentional (though I'm not super familiar with the model here).
We can replicate the memory semantics with ordering by using `NSPointerArray` (which is unfortunately not parameterized). This change does that, so we get consistently top-down layout events (matching Fabric, and Android Paper as of D49627996). This lets us use multiple layout events to calculate right/bottom edge insets deterministically.
Changelog:
[iOS][Changed] - Deterministic onLayout event ordering for iOS Paper
Reviewed By: luluwu2032
Differential Revision: D50093411
fbshipit-source-id: f6a9d5c973b97aede879baa8b952cc1be2447f28
* Remove code to support bottom-up layout events in horizontal RTL (react#39646)
Summary:
Pull Request resolved: react#39646
We can dramatically simplify this code and remove quirks/hacks, now that we can assume layout events are always fired top down.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D49628669
fbshipit-source-id: 7de5bbc4597eba1c59aaa7672c70e76d2786c7ef
* Fix iOS Paper Scroll Event RTL check (react#40751)
Summary:
Pull Request resolved: react#40751
In D48379915 I fixed inverted `contentOffset` in `onScroll` events on iOS. I thought I tested on Paper, but I think this was during a period where the Paper route in Catalyst was actually launching Fabric (oops).
In Paper, at least under `forceRTL` and English, `[UIApplication sharedApplication].userInterfaceLayoutDirection` is not set to RTL. We instead have a per-view `reactLayoutDirection` we should be reading.
This sort of thing isn't currently set on Fabric, which checks application-level RTL. This seems... not right with being able to set `direction` in a subtree context, but Android does the same thing, and that would take some greater changes.
Changelog:
[iOS][Fixed] - Fix iOS Paper Scroll Event RTL check
Reviewed By: luluwu2032
Differential Revision: D50098310
fbshipit-source-id: e321fca7b2f7983e903e23237bc2d604c72f98a3
* Disable --experimental-debugger by default (react#40795)
Summary:
Pull Request resolved: react#40795
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D50178494
fbshipit-source-id: b833b6accadd2902d97a45e030cd48d1b62ac438
* make runtime reference thread safe on tickleJs call
Summary:
The reference to runtime assumes the queue will ensure references to runtime are valid when invoked. This
isn't the case if you create a breakpoint, Hermes hit that breakpoint and your refresh the app. This consistently
will crash the app.
The fix is to not assument this, similar to ReactCommon/react/runtime/hermes/HermesInstance.cpp
Reviewed By: javache
Differential Revision: D50225678
fbshipit-source-id: b45cae1f5f687bc8c699fd74b187376a547012c5
* Run all the tests when not on a PR
Summary:
## Changelog:
[Internal] - Run all the tests in CI when not on a PR
Reviewed By: cortinico
Differential Revision: D50220596
fbshipit-source-id: be1a30d713e9d427858cf22bd3ca9549ad513057
* Add "Hermes only" to open debugger key prompt
Summary: Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D50327180
fbshipit-source-id: 6ee08fadd313828ddbc7702d5b3b1174a91b1ac1
* Refactors InspectorProxy to compare pathname portion of url passed in… (react#41005)
Summary:
… request to local pathname comparator variables to fix issue with other rightward elements of url such as query or fragment entering the comparison and causing 404 errors for key debugging routes.
A change in Chromium appended the query "?for_tabs" to the /json/list request made by Chrome DevTools to find remote debugger targets.
The current comparison in InspectorProxy.js compares the entire node IncomingMessage url field to the local pathname constants. The issue arises as url can also contain the query and fragment portions so the original comparison of "/json/list" === "/json/list" which resolved as true would become "/json/list?for_tabs" === "/json/list" and evaluate to false ultimately resulting in a 404 for the request.
In summary, all these changes/issues caused remote debugging of Hermes code in React Native apps to become unavailable, greatly impacting developer experience.
## Changelog:
[GENERAL] [FIXED] JS Debugging: Fix inspector-proxy to allow for DevTools requests with query strings
Pull Request resolved: react#41005
Reviewed By: NickGerleman
Differential Revision: D50342265
Pulled By: robhogan
fbshipit-source-id: a65f2908f0bea9fc15e1e3e4e6d31a3b9598e81f
* Make sure buildConfig is turned on for all the 3rd party libraries (react#40939)
Summary:
Pull Request resolved: react#40939
Currently some libs on RN 0.73 are broken as the default for Build Config generation changed
from true to false since AGP 8.x. This reverts the behavior to the old flag.
Closesreact#40791Closesreact#40559
Changelog:
[Internal] [Changed] - Make sure buildConfig is turned on for all the 3rd party libraries
Reviewed By: mdvacca
Differential Revision: D50270382
fbshipit-source-id: 02dcb031c577f65be2f41d9da0334c1b3d89e4c5
* Configure Java toolchains only when executing App configuration (react#40757)
Summary:
Fixreact#40560
## Changelog:
[ANDROID] [FIXED] - Ensure that `configureJavaToolChains` is only executed once during configuration
Pull Request resolved: react#40757
Test Plan:
- Create a fresh `react-native@0.73.0-rc.1` project
- Install `react-native-webview`
- Apply [this patch](https://github.com/react-native-webview/react-native-webview/pull/3175/files) for `react-native-webview` (caused by another issue react#40559)
- Edit `android/gradle.properties` and set `newArchEnabled` to true
- Build application
- (Expected) Application fail to build
- Apply this PR
- (Expected) Application build successfully
**Additional explanation:**
According to the implementation of `configureJavaToolChains`, all the subprojects (both the app and the libraries) will have their toolchains setup in one execution of the method. Therefore, it is okay for the method to be invoked only when configuring the plugin for the app.
On the other hand, invoking the method for more than one time will cause the issue stated in react#40560.
Reviewed By: cipolleschi
Differential Revision: D50361871
Pulled By: cortinico
fbshipit-source-id: bd5e18df97988122788d0482dba954e517a0cb5c
* Fix instacrash on bridgeless due to calling showMessage on null instance (react#39834)
Summary:
Pull Request resolved: react#39834
Bridgeless is instacrashing on fast-refresh. This fixes it.
Changelog:
[Android] [Fixed] - Fix instacrash on bridgeless due to calling showMessage on null instance
Reviewed By: cipolleschi
Differential Revision: D49929822
fbshipit-source-id: a2ce65797abd34d6a3e2b7f2c50d38a62ea8bdea
* Re-introduce FabricViewStateManager interface (react#40998)
Summary:
Pull Request resolved: react#40998
changelog: [Android] Add back interface FabricViewStateManager to unblock 0.73
I incorrectly deleted FabricViewStateManager in D47993140. This is a breaking change even for old architecture. Let's add it back and mark it as deprected so we can remove it later on.
This interface is not used in react-native anymore.
We are removing FabricViewStateManager because it simply wraps StateWrapper and provides no additional anymore.
Reviewed By: cortinico
Differential Revision: D50318633
fbshipit-source-id: aeb1c66c35018e336339616b564dee6f3156b54b
* Add ExceptionsManagerModule for RNTester (react#40767)
Summary:
Pull Request resolved: react#40767
Fix the following issue:
```Invariant Violation: TurboModuleRegistry.getEnforcing(...):
'ExceptionsManager' could not be found. Verify that a module by this name is registered in the native
binary.Bridgeless mode: true. TurboModule interop: true. Modules loaded: {"NativeModules":[],"TurboModules":
["PlatformConstants","AppState","SourceCode","BlobModule","WebSocketModule","DevSettings","DevToolsSettingsManager","LogBox","Networking","Appearance","DevLoadingView","DeviceInfo","DeviceEventManager",
"SoundManager","ImageLoader","DialogManagerAndroid","NativeAnimatedModule","I18nManager","AccessibilityInfo","StatusBarManager","StatusBarManager","IntentAndroid","ToastAndroid","ShareModule","Vibration"],
"NotFound":["NativePerformanceCxx","NativePerformanceObserverCxx","RedBox","BugReporting","HeadlessJsTaskSupport","FrameRateLogger","KeyboardObserver",
"AccessibilityManager","ModalManager","LinkingManager","ActionSheetManager","ExceptionsManager"]}
```
Changelog:
[Android][Changed] - Add Add ExceptionsManagerModule to CoreReactPackage
Reviewed By: cortinico
Differential Revision: D50017783
fbshipit-source-id: 8642bb23bdae50a1e702f5e0586b0ede80007bb1
# Conflicts:
#	packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.kt
* Fix compilation warnings introduced by Gradle 8.4 (react#39959)
Summary:
Pull Request resolved: react#39959
We're still accessing project.buildDir which will be removed in Gradle 9.0
I'm cleaning it up here.
Changelog:
[Internal] [Changed] - Fix compilation warnings introduced by Gradle 8.4
Reviewed By: yungsters
Differential Revision: D50016573
fbshipit-source-id: de7a725f61b503f08991ebf85b9a002cefab221a
* update: fresco 3.0.0 -> 3.1.0 (react#41024)
Summary:
- ref: reactwg/react-native-releases#64 (comment)
With react-native@0.73.0-rc.2 and fresco@3.0.0, `dlopen failed: library "libnative-imagetranscoder.so"` error made crash in android. so, I patched fresco version to 3.1.0, so that resolves the crash error according to facebook/fresco#2722 (comment) this comment.
## Changelog:
<!-- 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
-->
[ANDROID] [FIXED] - b1fccb0 bumped fresco version to 3.1.0.
Pull Request resolved: react#41024
Test Plan: after bumping fresco version, error not occurs.
Reviewed By: cipolleschi
Differential Revision: D50359564
Pulled By: cortinico
fbshipit-source-id: d24caecc057bad4d9c94e378422ab18c106a7ee6
# Conflicts:
#	packages/react-native/gradle/libs.versions.toml
* Rename `RCTRootViewFrameDidChangeNotification` as it's not trac… (react#39835)
Summary:
…king root view frame changes
Looking through where this was introduced (react#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: react#39835
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D50173742
Pulled By: javache
fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c
* Fix unstable RCTAppDelegate podspec (react#41009)
Summary:
Pull Request resolved: react#41009
This change should fix [react#39971](react#39971), computing the relative path from the App path to the pod installation root and using that instead of the absolute path to the `react-native.config.js` file
## Changelog
[Internal] - Stabilize RCTAppDelegate podspec
Reviewed By: cortinico
Differential Revision: D50323710
fbshipit-source-id: e29e62228d08c752e822d7a9ab5b1a2b5dcd6eb4
* Symbolicate unhandled promise rejections (react#40914)
Summary:
For a very long time when a promise rejects without an attached catch we get this warning screen without a correct stack trace, only some internal calls to the RN internals.
<img src="https://github.com/facebook/react-native/assets/1634213/75aa7615-ee3e-4229-80d6-1744130de6e5" width="200" />
I created [an issue for discussion](react-native-community/discussions-and-proposals#718) in the react-native-community repo and we figured out it was only a matter of symbolication. While it cannot be done on release without external packages and source maps, at least while developing we can provide a symbolicated stack-trace so developers can better debug the source of rejected promise.
I got the stack trace symbolicated and the correct code frame. I'm missing some help trying to display it in the warning view but at the very least I can now correctly show the line of the error and log the codeframe to the console.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[GENERAL] [FIXED] - Show correct stack frame on unhandled promise rejections on development mode.
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: react#40914
Test Plan:
I simply created a throwing function on a dummy app, and checked the output of the console and the warning view:
```ts
import React from 'react';
import {SafeAreaView, Text} from 'react-native';
async function throwme() {
throw new Error('UNHANDLED');
}
function App(): JSX.Element {
throwme();
return (
<SafeAreaView>
<Text>Throw test</Text>
</SafeAreaView>
);
}
export default App;
```
Here is the output
<img src="https://github.com/facebook/react-native/assets/1634213/2c100e4d-618e-4143-8d64-4095e8370f4f" width="200" />
Edit: I got the warning window working properly:
<img src="https://github.com/facebook/react-native/assets/1634213/f02a2568-da3e-4daa-8132-e05cbe591737" width="200" />
Reviewed By: yungsters
Differential Revision: D50324344
Pulled By: javache
fbshipit-source-id: 66850312d444cf1ae5333b493222ae0868d47056
* Align all the pods to 13.4 (react#41050)
Summary:
Pull Request resolved: react#41050
This change aligns all the podspecs to 13.4, including 3rd party libraries. This should fixreact#39826
## Changelog
[iOS][Changed] - Align all pods to 13.4
Reviewed By: huntie
Differential Revision: D50405114
fbshipit-source-id: 2d1b50ed8433e8cdc2214eb17e15be6ea81107a1
# Conflicts:
#	packages/react-native/scripts/cocoapods/utils.rb
* Bump SocketRocket to 0.6.1 (react#40774)
Summary:
The SocketRocket version was upgraded to 0.6.1 on the 0.72-stable branch but for some reason it was not updated in main, causing a downgrade when running `pod install` with 0.73.0 RC1
Original commit bumping SocketRocket -> react@8ce471e
## Changelog:
[IOS] [CHANGED] - Bump SocketRocket to 0.6.1
Pull Request resolved: react#40774
Test Plan: Run rntester locally
Reviewed By: cipolleschi
Differential Revision: D50137261
Pulled By: arushikesarwani94
fbshipit-source-id: dfc2760f5d5611881126ad114d8f6ada23630a29
# Conflicts:
#	packages/react-native/React-Core.podspec
#	packages/react-native/React/CoreModules/React-CoreModules.podspec
#	packages/rn-tester/Podfile.lock
* Bump RNTester Cocoapods cache keys after SocketRocket 6.1.0 bump (react#40789)
Summary:
Commit 8b88883 broke the Cache for RNTester because the cached version of the pods does not know about the exitence of SocketRocket 6.1.0
Bumping the keys should force a redownload of the cocoapods specs repo
## Changelog:
[Internal] - Bump RNTester cache keys
Pull Request resolved: react#40789
Test Plan: CircleCI is green
Reviewed By: GijsWeterings
Differential Revision: D50169281
Pulled By: cipolleschi
fbshipit-source-id: 83e251495bfa43d62384470efe97c5505d76684f
# Conflicts:
#	packages/rn-tester/Podfile.lock
* Revert "Add ExceptionsManagerModule for RNTester (react#40767)"
This reverts commit 034d815.
* rewrite RNTesterApplication to kotlin (react#39557)
Summary:
Rewrite `RNTesterApplication` to Kotlin as per [Help us Kotlin-ify React Native tests - Round 2](react#38825)
## Changelog:
[ANDROID] [CHANGED] - Rewrite RNTesterApplication to Kotlin, add AnnotationTarget property.
Pull Request resolved: react#39557
Test Plan:
`yarn && yarn android` ✅
The only thing I'm kinda unsure of is whether `AnnotationTarget.PROPERTY` should be added, but it didn't let me annotate `reactHostInterface` without that and didn't compile.
<img width="637" alt="image" src="https://github.com/facebook/react-native/assets/33528752/8bc84870-f3f2-4a46-b076-6ee7e38bd735">
cortinico mdvacca
Reviewed By: cortinico
Differential Revision: D49598401
Pulled By: mdvacca
fbshipit-source-id: 105ae0c13c93dae0eeb2b6fa9040f03f42d2736a
* Do not guard initializeFlipper for bridgeless for RN Tester
Summary:
We don't need this if-than-else because the initializeFlipper already checks if we're on bridgeless or not
Changelog:
[Internal] [Changed] - Do not guard initializeFlipper for bridgeless for RN Tester
Reviewed By: NickGerleman
Differential Revision: D49881903
fbshipit-source-id: e6bfc941b43382580bd418a5f27ad9426d300c69
* Fix new arch example not render in RNTester (react#39810)
Summary:
Pull Request resolved: react#39810
Two issues will be fixed:
- Bridgeless has lazy view manager loading by default so the React Package that provides view managers must implement ViewManagerOnDemandReactPackage, we might could refactor the design of package classes later
- ThemedReactContext should **NOT** be used directly to call function ```getJSModule```, since it doesn't overrides ```getJSModule``` for Bridgeless, we can use it's internal variable ```meactApplicationContext``` which should be an instance of BridgelessReactContext
Reviewed By: cortinico
Differential Revision: D49912656
fbshipit-source-id: a0bdd717612398e8d7a6f36d36dba241a3b06bd7
* Move ReactHost to com.facebook.react (react#39769)
Summary:
Pull Request resolved: react#39769
Move ReactHost to com.facebook.react
bypass-github-export-checks
changelog: [internal] internal
Reviewed By: javache
Differential Revision: D49834870
fbshipit-source-id: 2cfdd34297399115457424f0ba1279fd477da4a1
* Add rn-tester Android BUCK configuration (react#39987)
Summary:
Pull Request resolved: react#39987
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D49983211
fbshipit-source-id: c362d5b93e159bfb5ca17bea0ec0484adcdb4dca
* Add ExceptionsManagerModule for RNTester (react#40767)
Summary:
Pull Request resolved: react#40767
Fix the following issue:
```Invariant Violation: TurboModuleRegistry.getEnforcing(...):
'ExceptionsManager' could not be found. Verify that a module by this name is registered in the native
binary.Bridgeless mode: true. TurboModule interop: true. Modules loaded: {"NativeModules":[],"TurboModules":
["PlatformConstants","AppState","SourceCode","BlobModule","WebSocketModule","DevSettings","DevToolsSettingsManager","LogBox","Networking","Appearance","DevLoadingView","DeviceInfo","DeviceEventManager",
"SoundManager","ImageLoader","DialogManagerAndroid","NativeAnimatedModule","I18nManager","AccessibilityInfo","StatusBarManager","StatusBarManager","IntentAndroid","ToastAndroid","ShareModule","Vibration"],
"NotFound":["NativePerformanceCxx","NativePerformanceObserverCxx","RedBox","BugReporting","HeadlessJsTaskSupport","FrameRateLogger","KeyboardObserver",
"AccessibilityManager","ModalManager","LinkingManager","ActionSheetManager","ExceptionsManager"]}
```
Changelog:
[Android][Changed] - Add Add ExceptionsManagerModule to CoreReactPackage
Reviewed By: cortinico
Differential Revision: D50017783
fbshipit-source-id: 8642bb23bdae50a1e702f5e0586b0ede80007bb1
* Make sure onIntArrayChanged is invoked on RN Tester (react#40940)
Summary:
Pull Request resolved: react#40940
Currently the `onIntArrayChanged` event is invoked
only once inside RN Tester. I'm changing the logic to make sure it fires whenever we click "Set Opacity"
Changelog:
[Internal] [Changed] - Make sure onIntArrayChanged is invoked on RN Tester
Reviewed By: mdvacca, dmytrorykun
Differential Revision: D50264765
fbshipit-source-id: 93a60fd1b657c3d8b8182cab6bb7cd4368ac9a42
* Fix broken Fabric Interop example (react#40942)
Summary:
Pull Request resolved: react#40942
The Fabric Interop example for Android is broken. This is due to the Kotlin conversion which moved the `Color.colorToHSV(color, hsv)` statement *after* the HSV array is read so the array is always [0,0,0].
I'm fixing it here.
Changelog:
[Internal] [Changed] - Fix broken Fabric Interop example
Reviewed By: cipolleschi
Differential Revision: D50264766
fbshipit-source-id: 27ae5289408c7c23c667d6d7112437fa7ebe36d5
* Simplify new app template for bridgeless (react#40929)
Summary:
Pull Request resolved: react#40929
This diff reduces the footprint that bridgeless is imposing on the new app template. Specifically:
- I've created a `.toReactHost` method that converts a DefaultReactNativeHost to a DefaultReactHost
- I've updated RN Tester to use the same setup as the New App template which reduces code duplication.
I also had to remove a couple of `UnstableReactNativeAPI` as those were bleeding in the new app template.
I don't think we should ask users to opt-in in `UnstableReactNativeAPI` in the New App template itself as
this means that all the apps will get this opt-in.
Instead we should keep it only for specific APIs that we want the users to opt into.
Changelog:
[Internal] [Changed] - Simplify new app template for bridgeless
Reviewed By: cipolleschi, luluwu2032
Differential Revision: D50227693
fbshipit-source-id: e86c54d5156cc27f1f898b43ca89c57d5cf148b8
* Make events work for Fabric Interop on Bridgeless (react#40941)
Summary:
Pull Request resolved: react#40941
Events are currently not working for Fabric Interop on Bridgeless. That's because the `BridgelessReactContext` is not checking for interop modules on `getJsModule` calls, so the `InteropEventEmitter` is never returned.
This extends `BridgelessReactContext` so that `InteropEventEmitter` is returned if the Interop Layer is turned on.
Changelog:
[Internal] [Changed] - Make events work for Fabric Interop on Bridgeless
Reviewed By: cipolleschi
Differential Revision: D50266484
fbshipit-source-id: 0188d71bdc7acc8c188d886d45f0258914ad7af7
* Make IntArray events work on Bridgeless for RN-Tester (react#41047)
Summary:
Pull Request resolved: react#41047
Array events are currently broken in the sample for RN Tester. This is because the event name is not registered correctly.
I'm updating the event registration to be correct.
Changelog:
[Internal] [Changed] - Make IntArray events work on Bridgeless for RN-Tester
Reviewed By: cipolleschi
Differential Revision: D50266485
fbshipit-source-id: 13bbce91a41281383d4857048e573b6d9cc5387b
* Bump package versions
#publish-packages-to-npm
* Make hermes build fetching more flavour resilient (react#41055)
Summary:
Pull Request resolved: react#41055
This should fix the issue highlighted in [this comment](reactwg/react-native-releases#64 (comment)).
Basically, before the fix, we were not supporting flavours correctly, as we assumed that only Debug and Release were available.
With this change, we infer whether we have to fetch Hermes for Debug or Release based on the actual flags that are passed. In this way, the users can customize their app's flavors more freely.
## Changelog:
[Internal] - Support multiple flavors when downloading Hermes
Reviewed By: huntie
Differential Revision: D50408381
fbshipit-source-id: 6990218b286b4dd823323bc63de90279efc9e74e
* Fix Glog for earlier versions of Xcode (react#41058)
Summary:
Pull Request resolved: react#41058
Glog has a piece of code which looks like this:
```
namespace google {
// They need the definitions of integer types.
#include "glog/log_severity.h"
#include "glog/vlog_is_on.h"
```
This fragment is:
- Always valid when the pod does not define a module
- Valid for Xcode >= 14.3, when the pod do define a module
- Invalid for Xcode < 14.3, when the pod do define a module
Modules are required to support Swift, so, in the long run, we want to have `DEFINES_MODULE` set to `YES` for `Glog`.
This is a temporary workaround to keep supporting older versions of Xcode while Apple keeps allowing to use Xcode 14.1 to submit apps to the store.
Historically, Apple pushes the minimum version of Xcode every April, so we expect to be able to remove this workaround in April 2024.
## Changelog:
[Internal] - Make Glog work with older versions of Xcode
Reviewed By: cortinico
Differential Revision: D50410487
fbshipit-source-id: 96145cdf9ba1bc75622403d3c06454d6d4bfd967
* Make the Fabric Interop layer work in Bridgeless (react#40732)
Summary:
Pull Request resolved: react#40732
This change allows the Fabric Interop Layer to work in bridgeless mode.
Given that the legacy components requires a Bridge to send events and use commands, this change simulates a bridge when it doesn't exists (i.e.: we are in bridgeless mode).
In order to make it work, we had to simulate a few elements from the Bridge and the UIManager.
## Changelog:
[iOS][Fixed] - Make the Fabric interop layer works in bridgeless mode.
## Facebook:
As an alternative approach, we could have created a `protocol RCTBridging`, have the `RCTBridge` conform to that protocol, and create a new type for it.
Practically this would have been much more cumbersome:
1. The [RCTBridge](https://www.internalfb.com/code/fbsource/[916531b9bf7a9943036807f7563c925b4c3e0101]/xplat/js/react-native-github/packages/react-native/React/Base/RCTBridge.h?lines=87-238) interface is quite big. All the props and method should be part of the protocol.
2. Extensions declared on RCTBridge would not have worked. For example, [`RCTBridge (RCTUIManager)`](https://www.internalfb.com/code/fbsource/[916531b9bf7a9943036807f7563c925b4c3e0101]/xplat/js/react-native-github/packages/react-native/React/Modules/RCTUIManager.h?lines=170-174).
3. It would require a major overhaul of the APIs, returning `id<RCTBridging>` in place of **every** function that takes/return an instance of `RCTBridge *`.
Clearly, not a feasible way to go.
Reviewed By: RSNara
Differential Revision: D50079929
fbshipit-source-id: 77d4370e9043c8e06b676bff3987511c042b0f9c
* [0.73.0-rc.3] Bump version numbers
* Remove deprecated jsinspector files (react#41159)
Summary:
Pull Request resolved: react#41159
Changelog: [Internal]
Cleanup from D48966244. The deprecated `jsinspector` files are no longer used by any code in either fbsource or `react-native`.
Reviewed By: hoxyq
Differential Revision: D50530796
fbshipit-source-id: b539b097cb6caf6c50a482fa93bf5d7886e76e52
* [LOCAL] Fix e2e script for 0.73
* [LOCAL] Run prettier
* [LOCAL] Update Podfile.lock
* Update Metro to ^0.80.0, don't pin to exact version
cherry-pick 7853b06
* cipolleschi/fix test script (react#41267)
* [0.73] Collection of Android picks for RC4 (react#41221)
Co-authored-by: Lulu Wu <luluwu@meta.com>
Co-authored-by: Gabriel Donadel <donadeldev@gmail.com>
fix `java.lang.NoSuchMethodError` for Bridgeless (react#41081)
resolved: react#41081resolved: react#40999resolved: react#41165
Fix RNTester not showing Redbox when Metro is not connected (react#41191)
resolved: react#41191resolved: react#41190resolved: react#41085resolved: react#41206
* Build fixes
---------
Co-authored-by: Alex Hunt <hello@alexhunt.io>
Co-authored-by: Luna Wei <luwe@fb.com>
Co-authored-by: Dmitry Rykun <dmitryrykun@meta.com>
Co-authored-by: Lulu Wu <luluwu@meta.com>
Co-authored-by: Alex Hunt <alexeh@meta.com>
Co-authored-by: Arushi Kesarwani <arushikesarwani@meta.com>
Co-authored-by: Ramanpreet Nara <ramanpreet@meta.com>
Co-authored-by: Riccardo Cipolleschi <cipolleschi@meta.com>
Co-authored-by: Alex Hunt <huntie@meta.com>
Co-authored-by: David Vacca <dvacca@meta.com>
Co-authored-by: Distiller <distiller@static.198.206.135.134.macminivault.com>
Co-authored-by: Distiller <distiller@static.38.23.39.57.cyberlynk.net>
Co-authored-by: Distiller <distiller@static.38.23.38.177.cyberlynk.net>
Co-authored-by: Thibault Malbranche <malbranche.thibault@gmail.com>
Co-authored-by: Distiller <distiller@static.38.23.40.157.cyberlynk.net>
Co-authored-by: Nicola Corti <ncor@meta.com>
Co-authored-by: Nishan <nishanbende@gmail.com>
Co-authored-by: Luna Wei <luwe@meta.com>
Co-authored-by: Distiller <distiller@static.38.23.40.28.cyberlynk.net>
Co-authored-by: Nick Gerleman <ngerlem@meta.com>
Co-authored-by: Blake Friedman <blakef@meta.com>
Co-authored-by: 116-7 <1167@local>
Co-authored-by: UNIDY2002 <UNIDY2002@outlook.com>
Co-authored-by: Samuel Susla <samuelsusla@meta.com>
Co-authored-by: DeokHyeon Wi <dnl9807@gmail.com>
Co-authored-by: Oscar Franco <ospfranco@gmail.com>
Co-authored-by: Gabriel Donadel <donadeldev@gmail.com>
Co-authored-by: Riccardo Cipolleschi <riccardo.cipolleschi@gmail.com>
Co-authored-by: Oleksii <alexx271298@gmail.com>
Co-authored-by: Christoph Purrer <chpurrer@meta.com>
Co-authored-by: Distiller <distiller@static.38.39.184.83.cyberlynk.net>
Co-authored-by: Moti Zilberman <moti@meta.com>
Co-authored-by: Rob Hogan <rob@rhogan.net>
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 29, 2024
…ct#39835)
Summary:
…king root view frame changes
Looking through where this was introduced (react#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: react#39835
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D50173742
Pulled By: javache
fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c
Saadnajmi added a commit to microsoft/react-native-macos that referenced this pull request Jan 29, 2024
* Remove deprecated uses of UIActivityIndicatorViewStyle (react#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: react#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 (react#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](react@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: react#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` (react#42277)
Summary:
`UIMenuController` is deprecated as of iOS 16. react@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: react#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 (react#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
- closesreact#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: react#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… (react#39835)
Summary:
…king root view frame changes
Looking through where this was introduced (react#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: react#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>
@Saadnajmi
Saadnajmi deleted the names-root branch March 15, 2024 00:11
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request May 24, 2024
…ct#39835)
Summary:
…king root view frame changes
Looking through where this was introduced (react#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: react#39835
Test Plan: CI should pass
Reviewed By: cipolleschi
Differential Revision: D50173742
Pulled By: javache
fbshipit-source-id: 4651696174c439800984a5e6cf642200bb9c4f3c
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.p: MicrosoftPartner: MicrosoftPartnerShared with MetaApplied 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.

6 participants

@Saadnajmi@analysis-bot@jpdriver@facebook-github-bot@javache@cipolleschi