Summary:
Changelog: [Internal][iOS] Minor: Rename RCTNotAllowedInAppWideFabric to RCTNotAllowedInFabricWithoutLegacy
`RCTNewArchitectureValidationPlaceholder(RCTNotAllowedInBridgeless` is to track Bridge APIs that are okay in Fabric but not in Bridgeless.
`RCTNewArchitectureValidationPlaceholder(RCTNotAllowedInFabricWithoutLegacy` is to track legacy APIs that should not exist if the app was using Fabric **without any legacy architecture**. e.g. RCTBridgeModule, legacy interop view components.
Reviewed By: fkgozali
Differential Revision: D37659105
fbshipit-source-id: aee4e083820e83a8dac19eb3b5efc49b37d90039
Summary:
Changelog: [Internal][iOS] Fix AppState in Bridgeless mode by removing guard for bridge, since it doesn't use bridge
AppState doesn't use bridge because RCTAppState subclasses RCTEventEmitter, which calls `_callableJSModules invokeModule` in both Bridge and Bridgeless mode to send events to JS.
Reviewed By: fkgozali
Differential Revision: D35988515
fbshipit-source-id: fb19f0f2df5b270f0ef57637930f94686e39a9a1
Summary:
Changelog: [Internal][iOS] Add validation log to the only static method in RCTUIManager, JSResponder
When all surfaces of an app has been fully migrated to Fabric, we don't expect `[RCTUIManager JSResponder]` to be called, so I'm expecting this API to be broken on Fabric and Bridgeless.
Reviewed By: RSNara
Differential Revision: D35723794
fbshipit-source-id: ec786946a33fca98c89e8cad0e0467bf45dc1735
Summary:
Changelog: [iOS][Internal] Refactor: Migrate Logbox surface initialization to Fabric when available, in Bridge and Bridgeless modes
# Why
This diff main purpose is to add `RCTErrorNewArchitectureValidation(RCTNotAllowedInAppWideFabric)` in `RCTSurface`, to ensure Paper surfaces are never created in FBiOS.
# The Situation
Before this diff, in Bridged Fabric, `[RCTLogbox show]` initializes a Paper `RCTSurface`, [using `[RCTLogBoxView initWithWindow]`](https://github.com/facebook/react-native/blob/main/React/CoreModules/RCTLogBoxView.mm#L46))
In this diff, in Bridged and Bridgeless Fabric, `[RCTLogbox show]` initializes a Fabric `RCTFabricSurface`.
Before this diff, in Bridgeless Fabric, RCTLogBox posts a "CreateLogBoxSurface" notification to RCTInstance.
In this diff, the notification hack is replaced by the same `RCTFabricSurface` initialization above.
Behavior is the same.
Reviewed By: RSNara
Differential Revision: D35177311
fbshipit-source-id: 6de418af8a01f914c9a806bb8d74915015f9087a
Summary:
Changelog: [Internal][iOS] Add validation reporting APIs for unexpected uses of Paper when Fabric is enabled
## RCTNotAllowedInBridgeless
Previously, we only had violation reporting APIs for when **Bridge APIs** are used in **Bridgeless mode**, which was only enabled in Bridgeless mode.
## RCTNotAllowedInFabric
This diff adds violation reporting APIs to use when **pre-Fabric Bridge APIs** are used in **Bridge or Bridgeless mode**. This allows us to add RCTAssert/RCTError/RCTLog to more APIs in Bridge mode. The main purpose is to distinguish between Bridge APIs that still work in Fabric, versus Bridge APIs that are no longer used in Fabric, so that the latter can be removed.
Reviewed By: philIip
Differential Revision: D35015758
fbshipit-source-id: 35366bc5143a59ee9a16d75da4de546ebfe250e6
Summary:
This reverts commit fcead14b0e.
This should close https://github.com/facebook/react-native/issues/32509 . There was a bug where il8nManager.forceRTL() wouldn't work on app launch, and required an app restart. That was caused by an earlier change (https://github.com/facebook/react-native/pull/31032) which should not be necessary (the deadlock it was attempting to fix was actually caused by separate code).
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fixed bug where forceRTL did not work on app launch
Pull Request resolved: https://github.com/facebook/react-native/pull/32574
Test Plan: Simple revert back to previously working code.
Reviewed By: RSNara
Differential Revision: D32315034
Pulled By: GijsWeterings
fbshipit-source-id: dae6c1f0a2481e53f2f1e80f1ac083947681ef99
Summary:
Note: PR to react-native-macos here https://github.com/microsoft/react-native-macos/pull/733
Internally in Microsoft code, we ran into a deadlock where the main queue and the UIManager queue were both trying to access `[RCTI18nUtil sharedInstance]`, and were blocked on each other. This is similar to an earlier issue with RCTScreenScale decsribed [here](https://github.com/facebook/react-native/issues/18096).
To summarize:
1- RCTShadowView (on the UIManager queue) and RCTView (on the main queue) both try to access `[RCTI18nUtil sharedInstance]`
2- The UIManager thread gets there first, and lazily initializes the sharedInstance. Meanwhile, the main thread is waiting on a lock possessed by the UIManager thread
3- As part of the initialization, we set an NSUserDefault, which seems to require the (blocked) main thread.
4- Deadlock.
For whatever reason, this only happens on debug. I did not figure out why, but I do know based on [this comment](https://github.com/facebook/react-native/issues/18096#issuecomment-368718081), that the UIManagerQueue should never block the main queue.
The fix is to not use NSUserDefaults, and simpy use atomic properties instead. We get the thread safety for free, and it also simplifies the code somewhat without changing the public API. The downside is values aren't persisted anymore, but I do not think that was necessary / intended.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fix deadlock on RCTi18nUtil
Pull Request resolved: https://github.com/facebook/react-native/pull/31032
Test Plan:
Ran the RTLExample in RNTester, and ensured switching to RTL still worked, and that setting forceRTL would still work after reloading the bundle.
https://user-images.githubusercontent.com/6722175/108775429-aefdae80-7526-11eb-9a89-3114f7ddc2af.mov
Reviewed By: javache
Differential Revision: D29522152
Pulled By: RSNara
fbshipit-source-id: 160840f63a7b1d6721b0fd8294fb11990a4509fa
Summary:
This diff removes all synthesize invokeJS = _invokeJS calls, and instead funnels them through synthesize callableJSModules = _callableJSModules. Now, all these NativeModules shouldn't have different branching in bridgeless mode vs bridge mode.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D28395445
fbshipit-source-id: 41a58d54c60be55e6bf5031e5417728f5eb6285c
Summary:
In the full bridgeless, the following aren't allowed:
* using legacy view manager interop layer (won't support long term, but still needed today, so just warn)
* initializing any subclass of RCTViewManager (won't support long term, but still used by legacy interop layer)
* initializing RCTUIManager (fabric UIManager should be the only one used)
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D28111530
fbshipit-source-id: 4f5eab600c6c7896d51861545b7f878c25248e44
Summary:
This will help us migrate our iOS EventEmitters off the bridge.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D25656902
fbshipit-source-id: 2ca434e073383a1bf5b1d7366f1e1858c0c49aa6
Summary:
This is an extension of D25449795. I searched for all usages of .eventDispatcher within NativeModules, and migrated them all to the Venice-compatible RCTModuleRegistry API.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D25473844
fbshipit-source-id: 2b8deec236e019f3adfb59fadd745c249ff822f4
Summary:
All NativeModules that use the bridge to require the eventDispatcher are now instead using the RCTModuleRegistry I introduced in D25412847 (0ed81b28d3).
## What does this codemod do?
For all ObjC files that contain `synthesize bridge = _bridge`, migrate calls that access the React Native bridge from `self`, and use it to load the event dispatcher.
**Thoughts on Codemod Safety:** If we can access the bridge from self, then that means that if we synthesize the module registry, we can access the module registry from self. Therefore, this codemod is safe.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D25449795
fbshipit-source-id: 2f7235d14659e73d673ae08763dc2cccdde55a19
Summary:
## Rationale
For every 1 call to RCTNetworking.sendRequest, we execute 6 calls to RCTNetworking.addListener. This is followed by at least one call to RCTNetworking.removeListeners. Aside from incrementing and decrementing the `_listeners` integer, these two methods accomplish nothing else: RCTNetworking doesn't implement the `startObserving` and `stopObserving` methods.
This diff makes RCTEventEmitter dispatch events without looking at the listeners integer. In the future, this will allow us to stop making these ~8 unnecessary NativeModule calls for every Network request we send.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24272560
fbshipit-source-id: 7996eba5abfa4669a89c43a3ffa536c0faa214a8
Summary:
This diff ended up being a bit more complicated than I anticipated, since the source files in `ReactInternal` were depending on `RCTEventDispatcher`. I made the following changes:
1. Make `RCTEventDispatcher` a `protocol`, keep it in `ReactInternal`.
2. Rename the `RCTEventDispatcher` NativeModule to `RCTEventDispatcherModule`, make it conform to the `RCTEventEmitter` `protocol`, and move it to `CoreModules`.
3. Where necessary, replace categories of `RCTEventDispatcher` with functions.
Changelog:
[iOS][Added] - Make RCTEventDispatcher TurboModule-comaptible
Reviewed By: fkgozali
Differential Revision: D18439488
fbshipit-source-id: b3da15c29459fddf884519f33b0c3b8c036b5539
Summary:
Refs: [0.62 release](https://reactnative.dev/blog/#moving-apple-tv-to-react-native-tvos), https://github.com/facebook/react-native/issues/28706, https://github.com/facebook/react-native/issues/28743, https://github.com/facebook/react-native/issues/29018
This PR removes most of the tvOS remnants in the code. Most of the changes are related to the tvOS platform removal from `.podspec` files, tvOS specific conditionals removal (Obj-C + JS) or tvOS CI/testing pipeline related code.
In addition to the changes listed above I have removed the deprecated `Platform.isTVOS` method. I'm not sure how `Platform.isTV` method is correlated with Android TV devices support which is technically not deprecated in the core so I left this method untouched for now.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
* **[Internal] [Removed]** - remove most of tvOS remnants from the code:
* `TVEventHandler`, `TVTouchable`, `RCTTVView`, `RCTTVRemoteHandler` and `RCTTVNavigationEventEmitter`
* **[Internal] [Removed]** - remove `TARGET_TV_OS` flag and all the usages
* **[iOS] [Removed]** - remove deprecated `Platform.isTVOS` method
* **[iOS] [Removed]** - remove deprecated and TV related props from View:
* `isTVSelectable`, `hasTVPreferredFocus` and `tvParallaxProperties`
* **[iOS] [Removed]** - remove `BackHandler` utility implementation
Pull Request resolved: https://github.com/facebook/react-native/pull/29407
Test Plan: Local tests (and iOS CI run) do not yield any errors, but I'm not sure how the CI pipeline would react to those changes. That is the reason why this PR is being posted as Draft. Some tweaks and code adjustment could be required.
Reviewed By: PeteTheHeat
Differential Revision: D22619441
Pulled By: shergin
fbshipit-source-id: 9aaf3840c5e8bd469c2cfcfa7c5b441ef71b30b6
Summary:
Since `dealloc` can be called from any thread, this would result `stopObserving` being called on a different thread/queue than the specified `methodQueue`. We specifically encountered this issue with a module needing the main queue having its `stopObserving` called on a background queue.
Changelog:
[iOS][Fixed] - Call [RCTEventEmitter stopObserving] on specified method queue
Reviewed By: RSNara
Differential Revision: D23821741
fbshipit-source-id: 693c3be6876f863da6dd214a829af2cc13a09c3f
Summary:
With the upgrade to React Native 0.63, we started running into nullability warnings that were breaking our build. This PR fixes those nullability warnings as well as a few other warnings in React-Core.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fix xcodebuild warnings in React-Core
Pull Request resolved: https://github.com/facebook/react-native/pull/29622
Test Plan:
- Nullability annotations should only affect compilation, but even though RNTester compiles, I'm not fully convinced that this won't break projects downstream. It would be good to get another opinion on this.
- The change in `RCTAllocateRootViewTag` is the only real logic change in this PR. We throw an exception if the root view tag is not in the correct format, so this change seems safe after some basic manual testing in RNTester.
Reviewed By: shergin
Differential Revision: D23386678
Pulled By: appden
fbshipit-source-id: a74875195a4614c3248e8f968aa98602e3ee2de0
Summary:
To get `RCTNativeAnimatedModule` working bridgeless, I need to get `RCTEventDispatcher` working bridgeless.
To get `RCTEventDispatcher` working bridgeless, I need to support 2 new bridge methods:
- `- (void)enqueueJSCall:(NSString *)moduleDotMethod args:(NSArray *)args`
- `- (void)dispatchBlock:(dispatch_block_t)block queue:(dispatch_queue_t)queue;`
For (1) I copied the bridge impl exactly. For (2), the bridge only dispatches to JS thread, else uses `dispatch_async`. I only added support for dispatching to JS thread, callers can `dispatch_async` themselves if they want to.
Changelog: [Internal]
Differential Revision: D22962292
fbshipit-source-id: e34d15aee72f80dffcaa945bfda05ea415f66df7
Summary:
Changelog: [Internal]
Do not log warning for "Invalid view set to be the JS responder".
{F242140509}
Fabric doesn't store views in view registry, therefore this warning would be shown to the developer everytime `[RCTUIManager setJSResponder]` is called.
Reviewed By: shergin
Differential Revision: D22309447
fbshipit-source-id: cac8985cdc79ab2d03a246cc2d9377472a64a683
Summary:
Changelog: [Internal]
In D8552360 (48b9a6f887) an experimental integration between old and new UIManager has been introduced.
It isn't needed anymore.
I did some measurements and it is the slowest part of `[RCTComponentViewDescriptor dequeueComponentViewWithComponentHandle]`.
{F241943384}
Reviewed By: shergin
Differential Revision: D22274782
fbshipit-source-id: 799ba047f1c57f68f00b0b6fa7c58782874991bc
Summary:
This refactors some logic which sets up HMRClient in JS. The logic should live in RCTDevSettings, so it is shared in bridge/bridgeless mode.
This also means the logic will be compiled out when `RCT_DEV_MENU` is false.
Reviewed By: RSNara
Differential Revision: D19563629
fbshipit-source-id: 5c2553be9fd686a2a178f03bb5eed7a82cbadb1b
Summary:
In working to remove dead code from the code base, we identified that a number of classes that subclassed `RCTEventEmitter` were being reported as unused when they are in active use. That was due to a lack of a `[super initialize]`. This diff adds the call to super.
Changelog:
[General] [Added] - Call to `super` in `+initialize` of `RCTEventEmitter`
Reviewed By: javache
Differential Revision: D19727783
fbshipit-source-id: 19c6c7f08e7a8b2f764988fdfcd70f92dbf1b74b
Summary:
This fixing a crash in RCTNativeAnimatedModule caused by accessing an `RCTSurfacePreseter` instance as "Objective-C runtime associated object" which was retained with `OBJC_ASSOCIATION_ASSIGN` policy. The documentation for `OBJC_ASSOCIATION_ASSIGN` says "Specifies a weak reference to the associated object." but it's a lie ( https://stackoverflow.com/questions/16569840/using-objc-setassociatedobject-with-weak-references) ). The policy is actually `ASSIGN` (aka `unsafe-unretained`).
That's why it's crashing.
We change that to `OBJC_ASSOCIATION_RETAIN` to retain the object (which meets the expectation of the interface of the category).
We also should not have over-retaining issues because:
* SurfacePresenter does not retain a Bridge or any object that can retain a Bridge;
* SurfacePresenter is a long-living object, we don't recreate it during bridge reloading or stuff like that.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19333869
fbshipit-source-id: 1ff03659a880f2742b909c5668c47144719eeee2
Summary:
Diff 1/N explained how calls to `[_bridge enqueueJSCall:]` can be replaced with `_invokeJS` in a bridgeless world.
This diff replaces RCTEventEmitter's usage of `enqueueJSCall` if the bridge isn't available.
Changelog: [Internal][iOS] Make RCTEventEmitter a RCTJSInvokerModule
Reviewed By: RSNara
Differential Revision: D18941955
fbshipit-source-id: 1b81e46585432e005cff5aa0ab4d151f50ea051b
Summary:
Changelog: [Internal]
The main use-case here is to get the rootTag off RCTImageView, for image loading instrumentation. The fact is, each RCTView subclass already has `reactTag` attached today. We already have the `rootTag` when the view is created by the UIManager, so why not just attach it like reactTag? If we don't, looking up the rootTag from the native component is non-trivial and extremely inefficient (have to jump to shadow queue, back to main queue, etc).
Reviewed By: sammy-SC
Differential Revision: D18497002
fbshipit-source-id: 8409e3a1c95e09accedd959592cbf178fab0b2c3
Summary:
Changelog: [internal] Fixes fabric specific crash
`_viewRegistry` is not thread safe and accessing it on another thread can cause a crash.
Calling it inside `RCTUnsafeExecuteOnMainQueueSync` shouldn't degrade performance because it is only used as a fallback.
Changelog: [Internal]
Reviewed By: rickhanlonii
Differential Revision: D18421789
fbshipit-source-id: 2adff434afe0a3c19318c95b051ee91ccf6293e5
Summary:
A very common pattern I've seen in RN codebase:
- (instancetype) init {
[[NSNotificationCenter defaultCenter] addObserver:self ...]
}
- (void) dealloc {
[[NSNotificationCenter defaultCenter] removeObserver:self ...]
}
From Apple:
https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc
> If your app targets iOS 9.0 and later or macOS 10.11 and later, you don't need to unregister an observer in its dealloc method.
RN targets iOS9+
Changelog: [Internal][Cleanup] Remove unneeded NSNotification center removeObserver
Reviewed By: shergin
Differential Revision: D18264235
fbshipit-source-id: 684e5f5555cec96b055b13cd83daaeb393f4fac9
Summary:
This diff adds back RN reload reason, which was removed earlier in the stack. cc/Rick
For callsites which already had a reason, I kept the string exactly the same. For callsites which didn't have a reason, I made up something reasonable.
Changelog: [Internal][Fixed] Re-implemented bridge reload reason text.
Reviewed By: RSNara
Differential Revision: D18074478
fbshipit-source-id: 64a3cd7718674a7ba7228a80e34791ce9f153f9f
Summary:
Motivation described in diff 1/N.
This diff replaces calls to `[bridge reload]` with calls to `RCTReloadCommand`. This shouldn't have any change in behaviour since RCTBridge listens to RCTReloadCommand and calls `[bridge reload]` [here](https://fburl.com/diffusion/kemzkrei).
It will allow us to customize who listens and reacts to RN lifecycle.
Changelog: [Internal][Changed] - Migrated [bridge reload] calls to RCTReloadCommand
Reviewed By: shergin
Differential Revision: D17880909
fbshipit-source-id: 80b26c6badd4b216656fed6dd04554e9877f4bb7
Summary:
Renders frames in RedBox in a greyed-out style when their `collapse` field is set to `true`. This avoids outright hiding information in the stack trace while still drawing attention to frames that are likely to be more meaningful.
Changelog: [General] [Changed] - Render collapsed JavaScript frames in RedBox
Reviewed By: rickhanlonii
Differential Revision: D18039438
fbshipit-source-id: 527588f11c0bff495842be7036cd1293bab65eb9
Summary:
Adds the ability to add extra buttons and renders them along with the other buttons.
Changelog: [iOS] [Added] - RCTRedBox ability to add extra buttons
Reviewed By: PeteTheHeat
Differential Revision: D17935352
fbshipit-source-id: f8fb28653e535cd2c098566afbc639eb5c196228
Summary:
@public
A lot of this code was duplicated, so move it all into one method.
Later in this stack, I will make custom red box buttons which will need to reuse this method.
Changelog: [iOS] [Changed] - Refactor RCTRedBox button creation
Reviewed By: PeteTheHeat
Differential Revision: D17915485
fbshipit-source-id: ede649862556b057779e0267118799c63f0215b8
Summary:
**Note:** This was landed in D17724498 but reverted in D17855088. The revert had nothing to do with this NativeModule.
Changelog: [iOS][Added] Make RCTAsyncLocalStorage TurboModule-compatible
Reviewed By: PeteTheHeat
Differential Revision: D17917841
fbshipit-source-id: 0f9dd5f592180d6512ca560007daa531a4da5b59
Summary:
**Note:** This was landed in D17722913, but reverted in D17855088. The revert had nothing to do with this NativeModule.
Changelog: [iOS][Added] Make RCTAlertManager TurboModule-compatible
Reviewed By: PeteTheHeat
Differential Revision: D17917827
fbshipit-source-id: d86ea2cddddd9535d656709296c74aebd6f45793
Summary: To enable onScroll animations with Fabric's scrollView on iOS, we dispatch onScroll event to Paper's eventDispatcher as well as to Fabric's one. One we have a proper NativeAnimationDriver in place, we will get rid of this.
Reviewed By: shergin
Differential Revision: D17814260
fbshipit-source-id: f04faf59cdfd4ea5cede513388e30500b4cb2ad5
Paige Sun
Vincent Riemer
Andres Suarez
Saad Najmi
Saad Najmi
Ramanpreet Nara
Kevin Gozali
Peter Argany
Lulu Wu
simek
Scott Kyle
Jayme Deffenbaugh
Samuel Susla
Valentin Shergin
Marcus Zarra
Nicolas Charpentier
Moti Zilberman
Mehdi Mulani
