feat: bring 0.74 support #2047
feat: bring 0.74 support #2047
Conversation
| @@ -416,7 +416,7 @@ - (void)notifyHeaderHeightChange:(double)headerHeight | |||
| [[RNSHeaderHeightChangeEvent alloc] initWithEventName:@"onHeaderHeightChange" | |||
| reactTag:[NSNumber numberWithInt:self.tag] | |||
| headerHeight:headerHeight]; | |||
| [[RCTBridge currentBridge].eventDispatcher sendEvent:event]; | |||
| [[RCTBridge currentBridge].eventDispatcher notifyObserversOfEvent:event]; | |||
There was a problem hiding this comment.
@RSNara do you maybe know if it is enough for listeners from Animated to be informed that the values changed since for some reason calling sendEvent here on RN 0.74-rc.0 crashed with:
(NOBRIDGE) ERROR Error: Failed to call into JavaScript module method RCTEventEmitter.receiveEvent(). Module has not been registered as callable. Registered callable JavaScript modules (n = 8): HMRClient, GlobalPerformanceLogger, RCTNativeAppEventEmitter, SamplingProfiler, RCTDeviceEventEmitter, RCTLog, HeapCapture, Systrace. Did you forget to call `RN$registerCallableModule`?
If I use ScrollView in the example, than it starts to work, so maybe ScrollView registers the module somehow? I can see that registering is done here: https://github.com/facebook/react-native/blob/767330f21885e668bc0d9b5b3063113d0446bcbc/packages/react-native/Libraries/EventEmitter/RCTEventEmitter.js but I cannot see anything connected with ScrollView calling this code.
There was a problem hiding this comment.
yep, I fixed it in another library: react-native-pager-view
That's a terrible approach, we have to fix though.
ScrollView works because it does this. Which basically import the whole paper renderer to register the RCTEventEmitter.
We will work on a better solution for 0.75, as the fix is more convoluted than just registering the module, unfortunately, and it is unlikely that we would be able to have the proper fix in time for 0.74 stable.
As a work around, you can apply the same fix of above.
There was a problem hiding this comment.
I just checked and the code I trigger here makes it work with Animated so it is all I need I think.
There was a problem hiding this comment.
@WoLewicki Sadly, I don't know if it's all you need. I see that this PR has been raised some time ago, that may be related to your error: #2027
Can you try if applying the fix from pager-view fixes that issue as well?
I might also take a look on that fix, if you don't have any time to spare!
WoLewicki
changed the title
|
Hi @WoLewicki, I see that some tasks on CI are failing here. Do you know what may be the cause of errors? |
| @@ -2,8 +2,6 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| xmlns:tools="http://schemas.android.com/tools"> | |||
|
|
|||
| <uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW"/> | |||
There was a problem hiding this comment.
Do we need to remove that permission?
FabricExample/patches/@react-native-community+cli-platform-android+13.6.0.patch
Outdated
Show resolved
Hide resolved
FabricTestExample/android/app/src/main/java/com/fabrictestexample/MainActivity.kt
Show resolved
Hide resolved
| @@ -416,7 +416,7 @@ - (void)notifyHeaderHeightChange:(double)headerHeight | |||
| [[RNSHeaderHeightChangeEvent alloc] initWithEventName:@"onHeaderHeightChange" | |||
| reactTag:[NSNumber numberWithInt:self.tag] | |||
| headerHeight:headerHeight]; | |||
| [[RCTBridge currentBridge].eventDispatcher sendEvent:event]; | |||
| [[RCTBridge currentBridge].eventDispatcher notifyObserversOfEvent:event]; | |||
There was a problem hiding this comment.
@WoLewicki Sadly, I don't know if it's all you need. I see that this PR has been raised some time ago, that may be related to your error: #2027
Can you try if applying the fix from pager-view fixes that issue as well?
I might also take a look on that fix, if you don't have any time to spare!
PR with fixes for new arch in RN 0.74
Description
PR with fixes for new arch in RN 0.74
Changes
Test code and steps to reproduce
Checklist