-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Pressable gets stuck under the new architecture #44610
Comments
Thanks for opening this @xdmx, we'll be looking into it in the next future |
@xdmx I've started looking into this. |
I am having this issue (managed Expo app with new architecture enabled, on both Android and iOS is happening). It seems like |
Hey all, Reproducer for React Native Community CLI app that uses no libraries at all is here: Also the issue is more evident on Android Physical devices as is more likely the |
@cortinico Does this also fix this issue: #44643 ? |
Yes those two are duplicates essentially |
Nice, thank you! I wasnt quite sure when I opened the issue as it similar yet a little different. Looking very much forward to a fix! |
Adding another datapoint as I'm investigating. <Pressable
onPress={() => {
setTimesPressed(current => current + 1);
}}
style={({pressed}) => [
{
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
},
styles.wrapperCustom,
]}>
+ <Text>Press Me</Text>
- {({pressed}) => (
- <>
- {pressed && <Text style={styles.text}>Pressed!</Text>}
- {!pressed && <Text style={styles.text}>Press Me</Text>}
- </>
- )}
</Pressable> as the problem seems related to scenarios where This also fixes the issue: <Pressable
onPress={() => {
setTimesPressed(current => current + 1);
}}
style={({pressed}) => [
{
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
},
styles.wrapperCustom,
]}>
{({pressed}) => (
<>
+ {pressed && <Text key="1" style={styles.text}>Pressed!</Text>}
+ {!pressed && <Text key="1" style={styles.text}>Press Me</Text>}
- {pressed && <Text style={styles.text}>Pressed!</Text>}
- {!pressed && <Text style={styles.text}>Press Me</Text>}
</>
)}
</Pressable> as there seems to be something going on with react reconciliation. |
The issue described in #44643 is happening with plain pressable - no dynamic child (as shown in the reproducer). If the thing you described above fixes the issue, then these two issues are not related. |
Cool thanks for the heads up. Then we might have two separate issues. |
Summary: I'm justing adding a repro to RN-Tester to furhter investigate: facebook#44610 Changelog: [Internal] [Changed] - Add reproducer to RN-Tester for facebook#44610 Differential Revision: D59375787
Summary: I'm justing adding a repro to RN-Tester to furhter investigate: facebook#44610 Changelog: [Internal] [Changed] - Add reproducer to RN-Tester for facebook#44610 Differential Revision: D59375787
Summary: I'm justing adding a repro to RN-Tester to furhter investigate: facebook#44610 Changelog: [Internal] [Changed] - Add reproducer to RN-Tester for facebook#44610 Differential Revision: D59375787
Just FYI - I was having the "stuck" Pressable issue as described above (where tapping a Pressable item would not complete and the screen would freeze) - upgraded to react-native 0.74.3 yesterday and now the "stuck" Pressable items cause the app to crash when tapped. So, something different, but likely worse. On Android, tapping a Pressable item causes the app to revert to the splash screen and freeze. On iOS the app simply crashes. |
Summary: This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
Just a heads up that we found the root cause of this issue and we're discussing a couple of potential fixes. See: |
…ure (facebook#45865) Summary: Pull Request resolved: facebook#45865 This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Closes facebook#45675 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
…ure (facebook#45865) Summary: Pull Request resolved: facebook#45865 This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Closes facebook#45675 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
Summary: This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
…ate the Pressable fix Summary: This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the fix for bug facebook#45126 and facebook#44610. Changelog: [Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix Differential Revision: D60908117
Summary: This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
…ate the Pressable fix (facebook#45930) Summary: Pull Request resolved: facebook#45930 This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the fix for bug facebook#45126 and facebook#44610. Changelog: [Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix Reviewed By: mdvacca Differential Revision: D60908117
Summary: This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
…ate the Pressable fix (facebook#45930) Summary: Pull Request resolved: facebook#45930 This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the fix for bug facebook#45126 and facebook#44610. Changelog: [Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix Differential Revision: D60908117
…ure (facebook#45865) Summary: Pull Request resolved: facebook#45865 This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Closes facebook#45675 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Reviewed By: mdvacca Differential Revision: D60594878
…ure (facebook#45865) Summary: Pull Request resolved: facebook#45865 This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Closes facebook#45675 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Reviewed By: mdvacca Differential Revision: D60594878
Summary: This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore. The JSTouchDispatcher already retains the information on which tag was the touch originally fired. We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted. This is causing a couple of bad bugs on Android: Fixes facebook#45126 Fixes facebook#44610 Changelog: [Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture Differential Revision: D60594878
…ate the Pressable fix (facebook#45930) Summary: Pull Request resolved: facebook#45930 This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the fix for bug facebook#45126 and facebook#44610. Changelog: [Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix Reviewed By: mdvacca Differential Revision: D60908117
…ate the Pressable fix (#45930) Summary: Pull Request resolved: #45930 This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the fix for bug #45126 and #44610. Changelog: [Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix Reviewed By: mdvacca Differential Revision: D60908117 fbshipit-source-id: 885917832718d9b90d043b2d7e2cdb47e0f01ea7
Thank you all for your patience. The issue was caused by the event emitter getting destroyed when the view was going off-screen. |
Description
I've noticed that using the new architecture under a brand new app (expo app) the pressable button gets stuck. The example is from the Pressable documentation, with a minor change on how it shows the components.
Disabling the new architecture it works as expected
Using a Pixel_3a_API_34_extension_level_7_x86_64 emulator
Steps to reproduce
npx create-expo-app@latest -e with-new-arch
React Native Version
0.74.1
Affected Platforms
Runtime - Android
Output of
npx react-native info
Reproducer
https://gist.github.com/xdmx/483cb093f59c23b1dee57b8b64c63696
https://github.com/xdmx/rn-pressable-bug
Screenshots and Videos
No response
The text was updated successfully, but these errors were encountered: