Summary:
A crash encountered in react-native-macOS is very similar to one fixed by https://github.com/microsoft/react-native-macos/pull/489#discussion_r451789471 (see discussion), and it's possible this `replacement` string also suffers from sharing the same backing store as the attributed string (maybe only when the range encompasses the entire string?) and therefore should be copied as well.
Changelog:
[iOS][Fixed] - Possible fix for convertIdToFollyDynamic crash in RCTBaseTextInputView and RCTEventDispatcher
Reviewed By: sammy-SC
Differential Revision: D38064551
fbshipit-source-id: 9c15f2a980155ab3cbb3fde79fcb93b24ee2091a
Summary:
# Context
ScrollView sticky headers rely on this bit of code to work:
```
AnimatedImplementation.attachNativeEvent(
this._scrollViewRef,
'onScroll',
[{nativeEvent: {contentOffset: {y: this._scrollAnimatedValue}}}],
);
```
What this code means:
When the ScrollView host component receives the "onScroll" event, assign event.nativeEvent.contentOffSet.y to the this._scrollAnimatedValue AnimatedValue.
How this subscription mechanism is set up:
NativeAnimatedTurboModule subscribes to events dispatched by RCTEventDispatcher sendEvent. Then, whenever RCTEventEmitter sendEvent executes, NativeAnimatedTurboModule also updates the AnimatedValue for that event.
# Problem
Previously, in bridgeless, we started dispatching RCTScrollView via the RCTEventDispatcher sendEvent to update the AnimatedValue for ScrollView. This meant that we started dispatching the onScroll event to JavaScript via RCTEventEmitter.receiveEvent JSModule, which isn't supported in the Fabric renderer.
With this diff, we dialed back that solution. Instead of (1) notifying NativeAnimatedTurboModule and (2) sending the onScroll event to JavaScript, we're only doing (1) (i.e: notifying NativeAnimatedTurboModule).
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D37257719
fbshipit-source-id: 7dea3cf8b9ae78f6b0fd40414b8f224d43367a5a
Summary:
In short, if an RCTEventDispatcher observer sends an event on the same thread that the observer was initially on, there will be a deadlock due to `sendEvent` already having the lock active on the `_observers` NSHashTable. An example where this occurred was when we had react-native-gesture-handler trigger an animated event, which then triggered an event on the underlying component being animated as a result of it being an observer on the animation event. Since this all occurred on the main thread, we ended up with a deadlock and the app froze.
To prevent this scenario, I used a `NSRecursiveLock` for _observersLock to be able to dispatch events on the same thread from observers.
joebernard
## 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] [Fix] - Prevent deadlock when dispatching events from observers on the same thread.
Pull Request resolved: https://github.com/facebook/react-native/pull/33490
Test Plan:
Not sure if there are any tests present for sending events with RCTEventDispatcher already in place, but In regular app usage this solution has proved to be a viable and stable option so far and has prevented the deadlock from occurring.
This would still be thread-safe since we are now allowing the event to be sent through observers on the same thread the initial event was dispatched on. The only issue I could see here is the behavior of sending an event could be changed.
Reviewed By: RSNara
Differential Revision: D35118752
Pulled By: charlesbdudley
fbshipit-source-id: 7e93a8d49841e001b235a437ccca1e072dcc7ab1
Summary:
RCTBaseViewController uses RCTRootView.bridge to call RCTViewEventEmitter.emit.
## Changes
- RCTEventDispatcher now exposes a method to dispatch RCTViewEventEmitter events.
- RCTRootView (for paper), and RCTSurfaceHostingProxyRootView (for fabric and bridgeless mode) now exports the RCTEventDispatcher by grabbing it from the bridge
- RCTBaseViewController now uses the event dispatcher exported from RCTRootView to dispatch the RCTViewEventEmitter event.
## Benefits
- RCTBaseViewController no longer uses the bridge to dispatch RCTViewEventEmitter events
- In the future, we'll make RCTSurfaceHostingProxyRootView work with bridgeless mode, which'll allow us to remove the bridge/bridgeless fork in RCTBaseViewController.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D30434884
fbshipit-source-id: d961a56ac2abc08a661f8fe7c37926f219b731d0
Summary:
This migrates all ObjC NativeModule setBridge: calls to the bridge compatible initialize: call. I filtered out ViewManagers.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D28595520
fbshipit-source-id: b0e5dd681fb3563894f1aa3da1728f0c694f04bb
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:
RCTJSInvokerModule invokeJS can do the work done by setInvokeJSWithModuleDotMethod. Therefore, this diff reduces the surface area of Venice, and gets rid of setInvokeJSWithModuleDotMethod.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D28232947
fbshipit-source-id: aa0d8497a1e0fba29109ca86a39de5d9e5b10c9c
Summary:
We're making the getTurboModule: method required for all classes that conform to RCTTurboModule.
Many of our ObjC-only and Cxx NativeModules don't implement this method. This diff implements a getTurboModule: method on all those modules that returns nullptr.
**Question:** Why is it fine to make ObjC-only NativeModules return nullptr from their getTurboModule: method?
- Because they're only accessed from ObjC, and should appear as null on the JavaScript side. Longer term, these NativeModules will also go away.
**Question:** Why is it fine to make Cxx NativeModules return nullptr from getTurboModule: method?
- Because after D27316872, the TurboModuleManager checks if the module is a CxxModule first. If it is, we do an early return, and never call the module's getTurboModule: method.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D27316871
fbshipit-source-id: bc693f2927ab3b0de24e6e9e7699390ec0f7d729
Summary:
According to the crash, RCTEventEmitter is being asked to emit events before RN is set up {emoji:1f928}. Neither the bridge, nor bridgeless mode is ready to send events.
In this scenario, drop the events on the floor instead of crashing.
Changelog: [Internal]
Reviewed By: naftaly
Differential Revision: D24634701
fbshipit-source-id: 933e5dfd15e5ee7c2215489305c71de46e78a9e5
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
Christoph Purrer
Ramanpreet Nara
Imran Shitta-Bey
Andres Suarez
Peter Argany
Lulu Wu