-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Skip flushing layout animation requests when handling events on Android #8459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
RodolfoSilva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Thank you. After modifying the library according to this PR, my crash issue has been resolved. react-native-reanimated+4.1.3.patch patch
diff --git a/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpp b/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpp
index 05acd4f..fa0832c 100644
--- a/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpp
+++ b/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpp
@@ -598,7 +598,7 @@ bool ReanimatedModuleProxy::handleRawEvent(
// (res == true), but for now handleEvent always returns false. Thankfully,
// performOperations does not trigger a lot of code if there is nothing to
// be done so this is fine for now.
- performOperations();
+ performOperations(true);
return res;
}
@@ -649,15 +649,15 @@ double ReanimatedModuleProxy::getCssTimestamp() {
return currentCssTimestamp_;
}
-void ReanimatedModuleProxy::performOperations() {
+void ReanimatedModuleProxy::performOperations(const bool isTriggeredByEvent) {
ReanimatedSystraceSection s("ReanimatedModuleProxy::performOperations");
- auto flushRequestsCopy = std::move(layoutAnimationFlushRequests_);
- for (const auto surfaceId : flushRequestsCopy) {
- uiManager_->getShadowTreeRegistry().visit(
- surfaceId, [](const ShadowTree &shadowTree) {
- shadowTree.notifyDelegatesOfUpdates();
- });
+ if (!isTriggeredByEvent) {
+ auto flushRequestsCopy = std::move(layoutAnimationFlushRequests_);
+ for (const auto surfaceId : flushRequestsCopy) {
+ uiManager_->getShadowTreeRegistry().visit(
+ surfaceId, [](const ShadowTree &shadowTree) { shadowTree.notifyDelegatesOfUpdates(); });
+ }
}
jsi::Runtime &rt =
diff --git a/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h b/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h
index 9e0599a..ac089c0 100644
--- a/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h
+++ b/node_modules/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h
@@ -117,7 +117,7 @@ class ReanimatedModuleProxy
void maybeRunCSSLoop();
double getCssTimestamp();
- void performOperations();
+ void performOperations(const bool isTriggeredByEvent);
void setViewStyle(
jsi::Runtime &rt,
diff --git a/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp b/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp
index 01247a9..0bb5c80 100644
--- a/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp
+++ b/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpp
@@ -127,8 +127,8 @@ bool NativeProxy::isAnyHandlerWaitingForEvent(
eventName, emitterReactTag);
}
-void NativeProxy::performOperations() {
- reanimatedModuleProxy_->performOperations();
+void NativeProxy::performOperations(const bool isTriggeredByEvent) {
+ reanimatedModuleProxy_->performOperations(isTriggeredByEvent);
}
bool NativeProxy::getIsReducedMotion() {
diff --git a/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.h b/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.h
index 8a811e5..2945299 100644
--- a/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.h
+++ b/node_modules/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.h
@@ -62,7 +62,7 @@ class NativeProxy : public jni::HybridClass<NativeProxy>,
bool isAnyHandlerWaitingForEvent(
const std::string &eventName,
const int emitterReactTag);
- void performOperations();
+ void performOperations(const bool isTriggeredByEvent);
bool getIsReducedMotion();
void requestRender(std::function<void(double)> onRender);
void registerEventHandler();
diff --git a/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.java b/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.java
index a02506c..b82043d 100644
--- a/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.java
+++ b/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.java
@@ -110,7 +110,7 @@ public class NativeProxy {
public native boolean isAnyHandlerWaitingForEvent(String eventName, int emitterReactTag);
- public native void performOperations();
+ public native void performOperations(boolean isTriggeredByEvent);
protected native void installJSIBindings();
@@ -488,7 +488,7 @@ public class NativeProxy {
void maybeFlushUIUpdatesQueue() {
UiThreadUtil.assertOnUiThread();
if (!mNodesManager.isAnimationRunning()) {
- mNodesManager.performOperations();
+ mNodesManager.performOperations(false);
}
}
}
diff --git a/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java b/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java
index a322746..52def06 100644
--- a/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java
+++ b/node_modules/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java
@@ -115,10 +115,10 @@ public class NodesManager implements EventDispatcherListener {
}
}
- public void performOperations() {
+ public void performOperations(boolean isTriggeredByEvent) {
UiThreadUtil.assertOnUiThread();
if (mNativeProxy != null) {
- mNativeProxy.performOperations();
+ mNativeProxy.performOperations(isTriggeredByEvent);
}
}
@@ -155,7 +155,7 @@ public class NodesManager implements EventDispatcherListener {
}
}
- performOperations();
+ performOperations(false);
}
mCallbackPosted.set(false);
@@ -189,7 +189,7 @@ public class NodesManager implements EventDispatcherListener {
// the UI thread.
if (UiThreadUtil.isOnUiThread()) {
handleEvent(event);
- performOperations();
+ performOperations(true);
} else {
String eventName = mCustomEventNamesResolver.resolveCustomEventName(event.getEventName());
int viewTag = event.getViewTag();
diff --git a/node_modules/react-native-reanimated/apple/reanimated/apple/native/NativeProxy.mm b/node_modules/react-native-reanimated/apple/reanimated/apple/native/NativeProxy.mm
index 793d28d..19a4e2d 100644
--- a/node_modules/react-native-reanimated/apple/reanimated/apple/native/NativeProxy.mm
+++ b/node_modules/react-native-reanimated/apple/reanimated/apple/native/NativeProxy.mm
@@ -46,7 +46,7 @@ - (void *)runtime;
std::weak_ptr<ReanimatedModuleProxy> weakReanimatedModuleProxy = reanimatedModuleProxy; // to avoid retain cycle
[nodesManager registerPerformOperations:^() {
if (auto reanimatedModuleProxy = weakReanimatedModuleProxy.lock()) {
- reanimatedModuleProxy->performOperations();
+ reanimatedModuleProxy->performOperations(false);
}
}];
|
|
@tomekzaw Thanks for the fix, when will this fix be released? |
|
It's gonna be released in 4.2.0. |
|
Do you think its safe to use the 4.2.0 nightly build? Or should we wait for the official release? |
|
It should be safe to use |
When is version 4.2.0 coming out? |
|
@tomekzaw we still got these crashes with the nightly build 4.2.0-nightly-20251109-b10bd9cb3 |
|
Here is the sentry crash https://chrono24-gmbh.sentry.io/share/issue/ee6a62892aac4dec989d5bb17e9d29c0/ |
|
We are also using a patch with the changeset of this MR and seeing the exact same error in prod as @Brma1048 Can't really give any other details unfortunately |
|
In my case, some issues were resolved. This PR fixed the crash that occurred when closing a modal implemented with reanimated while simultaneously popping a screen. I haven't yet found the minimal conditions to reproduce the remaining issue, but it seemed to crash when the navigation state changed rapidly in a short moment. Edit: |

Summary
Fixes #8422.
Co-authored-with: Bartłomiej Błoniarz [email protected]
Test plan