Skip to content
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

2.13.2 breaking app #2644

Closed
bhandanyan-nomad opened this issue Oct 17, 2023 · 16 comments
Closed

2.13.2 breaking app #2644

bhandanyan-nomad opened this issue Oct 17, 2023 · 16 comments
Labels
Close when stale The issue will be closed automatically if it remains inactive Missing repro Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS

Comments

@bhandanyan-nomad
Copy link

Description

I'm currently using version 2.13.1 of this package and my app has no issues. After I tried upgrading to 2.13.2, my app is no longer working. A couple things I noticed right away were:

  1. The user I was previously authenticated with was logged out. I'm not sure why this would impact my persisted data, but it seems it did.
  2. Our log in button no longer works. Other pressables on the login screen still work, but the login button does not. The main difference with the log in button compared to the other pressables on the screen is it is connected to the login information inputs using react-hook-form

I don't have a reproduction outside of my project at the moment, if needed I could try to create one. Mainly I'm just looking for any insight into what changed and why a patch upgrade would seem to have messed up our setup so much. Thanks 🙏

Steps to reproduce

N/A at the moment issues are specific to my project

Snack or a link to a repository

N/A

Gesture Handler version

2.13.2

React Native version

0.70.13

Platforms

Android, iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS Missing repro labels Oct 17, 2023
@github-actions
Copy link

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@j-piasecki
Copy link
Member

Hi! It's hard to tell what could be happening here without a reproduction sadly, as only changes done in 2.13.2 were related to the Android build process, new swipeable props were added, and a queueMicrotask change that would affect RN < 0.66.

Our log in button no longer works. Other pressables on the login screen still work, but the login button does not.

What do you mean by pressables? Is that referring to RN's Pressable component, Touchable components, or a custom component?

The user I was previously authenticated with was logged out. I'm not sure why this would impact my persisted data, but it seems it did.

That's strange. Are you sure nothing besides the Gesture Handler version has changed?

@Saikedo
Copy link

Saikedo commented Oct 18, 2023

Having the same issue. All platform fail in some ways in the latest version (including react native web)

@m-bert
Copy link
Contributor

m-bert commented Oct 19, 2023

Hi @Saikedo! In that case could you provide repro? It's hard to say what's happening without it, since changes between those versions shouldn't break anything.

@m-bert m-bert added the Close when stale The issue will be closed automatically if it remains inactive label Oct 30, 2023
@bhandanyan-nomad
Copy link
Author

This is still an issue with the new minor version. I can add a couple more details from how it's breaking my app, but my app is private so I won't be able to share it. I can try to produce a snack at some point, but it's not something I can quickly put a lot of time into.

My log in button is part of a login form set up with react-hook-form and that button no longer fires correctly after the upgrade.

My authentication is persisted with redux-persist and when I upgrade this package my persistence logging no longer shows up my debugger. I tried re-ordering the redux provider and persist gate components that live in the root of my app along with the gesture handler root view but that didn't seem to fix the issue.

@m-bert
Copy link
Contributor

m-bert commented Nov 28, 2023

Unfortunately we can't do much unless you provide reproduction.

I've tried to look into react-hook-form problem and our Buttons seem to work in 2.14.0. There was a problem if you pass arrow function as parameter into onPress like this:

<BaseButton onPress={() => handleSubmit(onSubmit)} />

but the same problem can be seen with standard RN Button. You could do

<BaseButton onPress={handleSubmit(onSubmit)} />

however Typescript is not happy with that. At the end I did this:

const handleFormSubmit = () => {
    handleSubmit(onSubmit)();
};

<BaseButton onPress={handleFormSubmit} />

and this seems to work with both RN buttons and our buttons, though I'm still not sure if this was the exact problem.

@bhandanyan-nomad
Copy link
Author

bhandanyan-nomad commented Jan 23, 2024

I noticed that with 2.13.2 my app starts "working" (not completely, but major issues are resolved) again if I do two things:

  1. Remove import 'react-native-gesture-handler'; from my index.js file
  2. Remove GestureHandlerRootView from wrapping my app

@bhandanyan-nomad
Copy link
Author

Another interesting tidbit is that besides by react-hook-form submissions not work, any asynchronous function call await fn() hangs after i upgrade to 2.13.2

@m-bert
Copy link
Contributor

m-bert commented Jan 31, 2024

I appreciate your descriptions, but as I said earlier, we are not able to do much unless you provide reproduction.

@bhandanyan-nomad
Copy link
Author

bhandanyan-nomad commented May 21, 2024

The following patch resolves the issue (react-native version 0.71.19):

diff --git a/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts b/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
index b057029..245f871 100644
--- a/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
+++ b/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
@@ -1,5 +1,4 @@
 // `queueMicrotask` was introduced to react-native in version 0.66 (https://github.com/react-native-community/releases/blob/master/CHANGELOG.md#v0660)
 // Because Gesture Handler supports versions 0.64+, we have to handle situations where someone uses older version of react native.
 // That's why if `queueMicrotask` doesn't exist, we use `setImmediate` instead, since it was used before we switched to `queueMicrotask` in version 2.11.0
-export const ghQueueMicrotask =
-  typeof queueMicrotask === 'function' ? queueMicrotask : setImmediate;
+export const ghQueueMicrotask = setImmediate;

I plan to try updating specific instances of ghQueueMicrotask to see which one is problematic with queueMicrotask

@m-bert
Copy link
Contributor

m-bert commented May 24, 2024

That's strange since #2612 added backward compatibility for React Native < 0.66. queueMicrotask was already used before 2.13.2 and basically what you're doing is changing it to setImmediate. In that case your app shouldn't work also with previous versions of Gesture Handler.

@bhandanyan-nomad
Copy link
Author

bhandanyan-nomad commented May 24, 2024

What's even more strange is that this patch, which is essentially a revert of #2612, makes my app work again. Something about simply importing it from a different file that seems to make my app freeze up.

diff --git a/node_modules/react-native-gesture-handler/.DS_Store b/node_modules/react-native-gesture-handler/.DS_Store
new file mode 100644
index 0000000..5172429
Binary files /dev/null and b/node_modules/react-native-gesture-handler/.DS_Store differ
diff --git a/node_modules/react-native-gesture-handler/src/handlers/createHandler.tsx b/node_modules/react-native-gesture-handler/src/handlers/createHandler.tsx
index 7f991fe..15fb320 100644
--- a/node_modules/react-native-gesture-handler/src/handlers/createHandler.tsx
+++ b/node_modules/react-native-gesture-handler/src/handlers/createHandler.tsx
@@ -29,7 +29,6 @@ import { isFabric, isJestEnv, tagMessage } from '../utils';
 import { ActionType } from '../ActionType';
 import { PressabilityDebugView } from './PressabilityDebugView';
 import GestureHandlerRootViewContext from '../GestureHandlerRootViewContext';
-import { ghQueueMicrotask } from '../ghQueueMicrotask';
 
 const UIManagerAny = UIManager as any;
 
@@ -215,7 +214,7 @@ export default function createHandler<
         // queueMicrotask. This makes it so update() function gets called after all
         // react components are mounted and we expect the missing ref object to
         // be resolved by then.
-        ghQueueMicrotask(() => {
+        queueMicrotask(() => {
           this.update(UNRESOLVED_REFS_RETRY_LIMIT);
         });
       }
@@ -379,7 +378,7 @@ export default function createHandler<
       // `ref={refObject}` it's possible that it won't be resolved in time. Seems like trying
       // again is easy enough fix.
       if (hasUnresolvedRefs(props) && remainingTries > 0) {
-        ghQueueMicrotask(() => {
+        queueMicrotask(() => {
           this.update(remainingTries - 1);
         });
       } else {
diff --git a/node_modules/react-native-gesture-handler/src/handlers/gestureHandlerCommon.ts b/node_modules/react-native-gesture-handler/src/handlers/gestureHandlerCommon.ts
index 9e795e1..7f28fd0 100644
--- a/node_modules/react-native-gesture-handler/src/handlers/gestureHandlerCommon.ts
+++ b/node_modules/react-native-gesture-handler/src/handlers/gestureHandlerCommon.ts
@@ -11,7 +11,6 @@ import { ValueOf } from '../typeUtils';
 import { handlerIDToTag } from './handlersRegistry';
 import { toArray } from '../utils';
 import RNGestureHandlerModule from '../RNGestureHandlerModule';
-import { ghQueueMicrotask } from '../ghQueueMicrotask';
 
 const commonProps = [
   'id',
@@ -236,7 +235,7 @@ let flushOperationsScheduled = false;
 export function scheduleFlushOperations() {
   if (!flushOperationsScheduled) {
     flushOperationsScheduled = true;
-    ghQueueMicrotask(() => {
+    queueMicrotask(() => {
       RNGestureHandlerModule.flushOperations();
 
       flushOperationsScheduled = false;
diff --git a/node_modules/react-native-gesture-handler/src/handlers/gestures/GestureDetector.tsx b/node_modules/react-native-gesture-handler/src/handlers/gestures/GestureDetector.tsx
index 588984b..27e1415 100644
--- a/node_modules/react-native-gesture-handler/src/handlers/gestures/GestureDetector.tsx
+++ b/node_modules/react-native-gesture-handler/src/handlers/gestures/GestureDetector.tsx
@@ -51,7 +51,6 @@ import { RNRenderer } from '../../RNRenderer';
 import { isNewWebImplementationEnabled } from '../../EnableNewWebImplementation';
 import { nativeViewGestureHandlerProps } from '../NativeViewGestureHandler';
 import GestureHandlerRootViewContext from '../../GestureHandlerRootViewContext';
-import { ghQueueMicrotask } from '../../ghQueueMicrotask';
 
 declare const global: {
   isFormsStackingContext: (node: unknown) => boolean | null; // JSI function
@@ -160,7 +159,7 @@ function attachHandlers({
 
   // use queueMicrotask to extract handlerTags, because all refs should be initialized
   // when it's ran
-  ghQueueMicrotask(() => {
+  queueMicrotask(() => {
     if (!mountedRef.current) {
       return;
     }
@@ -180,7 +179,7 @@ function attachHandlers({
 
   // use queueMicrotask to extract handlerTags, because all refs should be initialized
   // when it's ran
-  ghQueueMicrotask(() => {
+  queueMicrotask(() => {
     if (!mountedRef.current) {
       return;
     }
@@ -268,7 +267,7 @@ function updateHandlers(
   // use queueMicrotask to extract handlerTags, because when it's ran, all refs should be updated
   // and handlerTags in BaseGesture references should be updated in the loop above (we need to wait
   // in case of external relations)
-  ghQueueMicrotask(() => {
+  queueMicrotask(() => {
     if (!mountedRef.current) {
       return;
     }
diff --git a/node_modules/react-native-gesture-handler/src/web_hammer/GestureHandler.ts b/node_modules/react-native-gesture-handler/src/web_hammer/GestureHandler.ts
index 4ff297f..1fcc27e 100644
--- a/node_modules/react-native-gesture-handler/src/web_hammer/GestureHandler.ts
+++ b/node_modules/react-native-gesture-handler/src/web_hammer/GestureHandler.ts
@@ -6,7 +6,6 @@ import { findNodeHandle } from 'react-native';
 import { State } from '../State';
 import { EventMap } from './constants';
 import * as NodeManager from './NodeManager';
-import { ghQueueMicrotask } from '../ghQueueMicrotask';
 
 // TODO(TS) Replace with HammerInput if https://github.com/DefinitelyTyped/DefinitelyTyped/pull/50438/files is merged
 export type HammerInputExt = Omit<HammerInput, 'destroy' | 'handler' | 'init'>;
@@ -509,7 +508,7 @@ abstract class GestureHandler {
         .filter((v) => v);
 
       if (shouldUseTouchEvents !== this.shouldUseTouchEvents(props)) {
-        ghQueueMicrotask(() => {
+        queueMicrotask(() => {
           // if the undelying event API needs to be changed, we need to unmount and mount
           // the hammer instance again.
           this.destroy();

I also tried this patch, which didn't fix my issues either:

diff --git a/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts b/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
index b057029..245f871 100644
--- a/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
+++ b/node_modules/react-native-gesture-handler/src/ghQueueMicrotask.ts
@@ -1,5 +1,4 @@
 // `queueMicrotask` was introduced to react-native in version 0.66 (https://github.com/react-native-community/releases/blob/master/CHANGELOG.md#v0660)
 // Because Gesture Handler supports versions 0.64+, we have to handle situations where someone uses older version of react native.
 // That's why if `queueMicrotask` doesn't exist, we use `setImmediate` instead, since it was used before we switched to `queueMicrotask` in version 2.11.0
-export const ghQueueMicrotask =
-  typeof queueMicrotask === 'function' ? queueMicrotask : setImmediate;
+export const ghQueueMicrotask = queueMicrotask;

Something extremely subtle is going on here which is a bit worrisome, and I'm not sure if it's best for me to remain on 2.13.1 or to upgrade with one of the patches i've shared that makes my app work. Any suggestions?

@bhandanyan-nomad
Copy link
Author

I have no clue if it's related or not, but separately I am seeing my app freeze up in the exact same manner when upgrading to react-native v0.72. I recently figured out that a small tweak to the metro config fixed the issue so that I can move forward with the upgrade.

Sharing in case it provides any hint as to what might actually be underlying these issues.

react-native-community/upgrade-support#277 (comment)

@m-bert
Copy link
Contributor

m-bert commented May 27, 2024

I don't know what's going on here, maybe it is something with your bundler configuration.

(...) I'm not sure if it's best for me to remain on 2.13.1 or to upgrade with one of the patches i've shared that makes my app work. Any suggestions?

I think it will be better to upgrade Gesture Handler and apply one of your patches (preferably the one with replacing ghQueueMicrotask with queueMicrotask). We don't have plans to change it anymore (at least for now) and 2.13.1 was released long time ago.

@bhandanyan-nomad
Copy link
Author

@m-bert version 2.18 fixed this issue for me without requiring a patch. Specifically, this PR #2966 is essentially doing what I had patched which is to use setImmediate instead of queueMicrotask

Is this at risk of regressing again in a future release? If so, is there some mechanism by which that can be avoided?

@m-bert
Copy link
Contributor

m-bert commented Aug 2, 2024

Hi! Hopefully there won't be any regression in the future. Thanks for your investigation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Close when stale The issue will be closed automatically if it remains inactive Missing repro Platform: Android This issue is specific to Android Platform: iOS This issue is specific to iOS
Projects
None yet
Development

No branches or pull requests

4 participants