Skip to content

Commit 34efb3d

Browse files
authored
[General] Link late-mounted gesture objects and skip mount reactions without external relations (#4562)
## Description Follow-up to #4546. A relation that holds a gesture object (`simultaneousWithExternalGesture(gesture)`) to a gesture mounted by a different detector in a later commit never links on native. When the outer detector attaches, the object still has tag -1, and the mount listener only matches ref entries, so nothing re-sends the config once the object gets its tag. The relation stays dead until the outer detector happens to re-render, which hides the problem. The listener now also matches gesture objects by identity: the mount event carries the very object stored in the relation. Composition fills relations with sibling gesture objects, so without a guard every composed detector would update itself on mount. The listener skips mounts of the detector's own gestures, which requires assigning `attachedGestures` before the mount events fire in `attachHandlers`. Most detectors have nothing that can resolve late, yet every one of them still walks its relations on every mount of every other gesture. `AttachedGestureState` gets a `hasExternalRelations` flag, computed in the attach and update microtasks. It is true only when some relation entry is a ref or a gesture object owned by another detector; numeric tags and composition siblings never resolve late. Detectors with the flag unset return from the listener immediately. On a screen with 300 `LegacyPressable`s, mounting 50 more spent 38 ms in mount listeners before and 5.5 ms after. ## Test plan <details> <summary>Tested on the following code:</summary> ```tsx import React, { useLayoutEffect, useRef, useState, useSyncExternalStore, } from 'react'; import { Button, StyleSheet, Text, View } from 'react-native'; import type { GestureType } from 'react-native-gesture-handler'; import { Gesture, GestureDetector, LegacyPressable, } from 'react-native-gesture-handler'; // #4540 follow-up demo (v2 detector only, v3 has no MountRegistry). // // Late relations: the OUTER pan declares `simultaneousWithExternalGesture` to // an INNER pan whose detector mounts later (toggle). Without the link the inner // (deeper) pan wins and the outer counter never moves; with it both update. // Row 1: relation holds the inner Gesture OBJECT (fixed by the follow-up). // Row 2: relation holds a REF filled by `withRef` (fixed by PR #4546). // // Perf: N composed background detectors (LegacyPressable = Simultaneous of 3) // plus "mount 50 more", timing the commit of the newly mounted batch. // Tiny external store so the outer detectors never re-render: a v2 detector // re-sends its config on every re-render, which would repair the relation in // the same commit the inner gesture mounts and hide the bug. type Counters = { outerObj: number; innerObj: number; outerRef: number; innerRef: number; }; const store = { mounted: false, counters: { outerObj: 0, innerObj: 0, outerRef: 0, innerRef: 0, } as Counters, listeners: new Set<() => void>(), subscribe: (listener: () => void) => { store.listeners.add(listener); return () => store.listeners.delete(listener); }, emit: () => { store.listeners.forEach((l) => l()); }, toggle: () => { store.mounted = !store.mounted; store.emit(); }, bump: (key: keyof Counters) => { store.counters = { ...store.counters, [key]: store.counters[key] + 1 }; store.emit(); }, }; // Row 1: gesture object relation. const innerPanObj = Gesture.Pan() .runOnJS(true) .onUpdate(() => store.bump('innerObj')); const outerPanObj = Gesture.Pan() .runOnJS(true) .simultaneousWithExternalGesture(innerPanObj) .onUpdate(() => store.bump('outerObj')); // Row 2: ref relation. const innerRefHolder: React.MutableRefObject<GestureType | undefined> = { current: undefined, }; const innerPanRef = Gesture.Pan() .runOnJS(true) .withRef(innerRefHolder) .onUpdate(() => store.bump('innerRef')); const outerPanRef = Gesture.Pan() .runOnJS(true) .simultaneousWithExternalGesture(innerRefHolder) .onUpdate(() => store.bump('outerRef')); function InnerSlot({ inner }: { inner: GestureType }) { const mounted = useSyncExternalStore(store.subscribe, () => store.mounted); return mounted ? ( <GestureDetector gesture={inner}> <View style={styles.inner} /> </GestureDetector> ) : ( <View style={[styles.inner, styles.innerPlaceholder]} /> ); } function CounterLabel({ prefix, outerKey, innerKey, }: { prefix: string; outerKey: keyof Counters; innerKey: keyof Counters; }) { const counters = useSyncExternalStore(store.subscribe, () => store.counters); return ( <Text style={styles.label}> {`${prefix} outer ${counters[outerKey]} / inner ${counters[innerKey]}`} </Text> ); } function ToggleButton() { const mounted = useSyncExternalStore(store.subscribe, () => store.mounted); return ( <Button title={mounted ? 'Unmount inner pans' : 'Mount inner pans'} onPress={store.toggle} testID="toggleInner" /> ); } function LateRelations() { return ( <View style={styles.section}> <ToggleButton /> <View style={styles.row}> <CounterLabel prefix="object" outerKey="outerObj" innerKey="innerObj" /> <GestureDetector gesture={outerPanObj}> <View style={styles.outer}> <InnerSlot inner={innerPanObj} /> </View> </GestureDetector> </View> <View style={styles.row}> <CounterLabel prefix="ref " outerKey="outerRef" innerKey="innerRef" /> <GestureDetector gesture={outerPanRef}> <View style={styles.outer}> <InnerSlot inner={innerPanRef} /> </View> </GestureDetector> </View> <Text style={styles.hint}> Pan the dark inner box. Both counters move only when the outer pan is linked to the inner one. </Text> </View> ); } const BACKGROUND = 300; const BATCH = 50; function Batch({ onCommit }: { onCommit: () => void }) { useLayoutEffect(onCommit, [onCommit]); return ( <View style={styles.grid}> {Array.from({ length: BATCH }, (_, i) => ( <LegacyPressable key={i} style={styles.cell} /> ))} </View> ); } function MountPerf() { const [batches, setBatches] = useState(0); const [lastMs, setLastMs] = useState<number | null>(null); const start = useRef(0); const onCommit = () => { if (start.current !== 0) { setLastMs(Math.round(performance.now() - start.current)); start.current = 0; } }; return ( <View style={styles.section}> <Button title={`Mount ${BATCH} more (${batches} batches)`} onPress={() => { start.current = performance.now(); setBatches((b) => b + 1); }} testID="mountBatch" /> <Text style={styles.label}> {BACKGROUND} background composed detectors, last batch:{' '} {lastMs === null ? '-' : `${lastMs} ms`} </Text> <View style={styles.grid}> {Array.from({ length: BACKGROUND }, (_, i) => ( <LegacyPressable key={i} style={styles.cell} /> ))} </View> {Array.from({ length: batches }, (_, i) => ( <Batch key={i} onCommit={onCommit} /> ))} </View> ); } export default function EmptyExample() { return ( <View style={styles.container}> <LateRelations /> <MountPerf /> </View> ); } const styles = StyleSheet.create({ container: { flex: 1, padding: 16, gap: 24 }, section: { gap: 8 }, row: { flexDirection: 'row', alignItems: 'center', gap: 12 }, label: { fontVariant: ['tabular-nums'], width: 180 }, hint: { opacity: 0.6, fontSize: 12 }, outer: { width: 120, height: 80, backgroundColor: '#b7d1ff', alignItems: 'center', justifyContent: 'center', }, inner: { width: 70, height: 50, backgroundColor: '#1e3a8a' }, innerPlaceholder: { backgroundColor: 'transparent', borderWidth: 1 }, grid: { flexDirection: 'row', flexWrap: 'wrap' }, cell: { width: 6, height: 6, margin: 1, backgroundColor: '#ddd' }, }); ``` </details>
1 parent 52a9c06 commit 34efb3d

7 files changed

Lines changed: 201 additions & 12 deletions

File tree

‎packages/react-native-gesture-handler/src/__tests__/useMountReactions.test.ts‎

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import { renderHook } from '@testing-library/react-native';
22
import type { Platform as PlatformModule } from 'react-native';
33

4+
import type { GestureType } from '../handlers/gestures/gesture';
5+
import { BaseGesture } from '../handlers/gestures/gesture';
46
import type { AttachedGestureState } from '../handlers/gestures/GestureDetector/types';
57
import { useMountReactions } from '../handlers/gestures/GestureDetector/useMountReactions';
8+
import { hasExternalRelations } from '../handlers/gestures/GestureDetector/utils';
69
import { MountRegistry } from '../mountRegistry';
710

811
// The relation scan used to resolve entries through `transformIntoHandlerTags`,
@@ -36,13 +39,21 @@ const stateWithRelation = (
3639
animatedHandlers: null,
3740
shouldUseReanimated: false,
3841
isMounted: true,
42+
hasExternalRelations: true,
3943
}) as unknown as AttachedGestureState;
4044

4145
const mount = (handlerTag: number) =>
4246
MountRegistry.gestureHandlerWillMount({
4347
handlerTag,
4448
} as unknown as React.Component);
4549

50+
const gestureObject = (handlerTag: number, config: object = {}) => {
51+
const gesture = Object.create(BaseGesture.prototype) as GestureType;
52+
gesture.handlerTag = handlerTag;
53+
gesture.config = config as GestureType['config'];
54+
return gesture;
55+
};
56+
4657
describe('useMountReactions', () => {
4758
const relationKeys: RelationKey[] = [
4859
'blocksHandlers',
@@ -97,14 +108,12 @@ describe('useMountReactions', () => {
97108
unmount();
98109
});
99110

100-
test('ignores entries that already carried their tag when the detector attached', () => {
111+
test('ignores raw tags, which cannot change after the detector attached', () => {
101112
const updateDetector = jest.fn();
102-
// A gesture object and a raw tag are both resolved by the time the detector
103-
// attaches, so mounting cannot change what they point at.
104113
const { unmount } = renderHook(() =>
105114
useMountReactions(
106115
updateDetector,
107-
stateWithRelation('simultaneousWith', [{ handlerTag: 42 }, 42])
116+
stateWithRelation('simultaneousWith', [42])
108117
)
109118
);
110119

@@ -114,6 +123,77 @@ describe('useMountReactions', () => {
114123
unmount();
115124
});
116125

126+
test('updates the detector when a gesture object in a relation mounts later', () => {
127+
const updateDetector = jest.fn();
128+
// The object had no tag when this detector attached; its own detector
129+
// assigns one in `initialize` and then fires the mount event with it.
130+
const external = gestureObject(-1);
131+
const { unmount } = renderHook(() =>
132+
useMountReactions(
133+
updateDetector,
134+
stateWithRelation('simultaneousWith', [external])
135+
)
136+
);
137+
138+
external.handlerTag = 42;
139+
MountRegistry.gestureWillMount(external);
140+
141+
expect(updateDetector).toHaveBeenCalledTimes(1);
142+
unmount();
143+
});
144+
145+
test('does not match a different gesture object with the same tag', () => {
146+
const updateDetector = jest.fn();
147+
const { unmount } = renderHook(() =>
148+
useMountReactions(
149+
updateDetector,
150+
stateWithRelation('simultaneousWith', [gestureObject(42)])
151+
)
152+
);
153+
154+
MountRegistry.gestureWillMount(gestureObject(42));
155+
156+
expect(updateDetector).not.toHaveBeenCalled();
157+
unmount();
158+
});
159+
160+
test('skips the mount of its own gestures', () => {
161+
const updateDetector = jest.fn();
162+
// Composition fills relations with sibling gestures, so without the guard
163+
// every composed detector would update itself on mount.
164+
const first = gestureObject(1);
165+
const second = gestureObject(2);
166+
first.config = { simultaneousWith: [second] } as GestureType['config'];
167+
second.config = { simultaneousWith: [first] } as GestureType['config'];
168+
const state = stateWithRelation('simultaneousWith', []);
169+
state.attachedGestures = [first, second];
170+
const { unmount } = renderHook(() =>
171+
useMountReactions(updateDetector, state)
172+
);
173+
174+
MountRegistry.gestureWillMount(first);
175+
MountRegistry.gestureWillMount(second);
176+
177+
expect(updateDetector).not.toHaveBeenCalled();
178+
unmount();
179+
});
180+
181+
test('skips the scan when the detector has no external relations', () => {
182+
const updateDetector = jest.fn();
183+
const state = stateWithRelation('simultaneousWith', [
184+
{ current: { handlerTag: 42 } },
185+
]);
186+
state.hasExternalRelations = false;
187+
const { unmount } = renderHook(() =>
188+
useMountReactions(updateDetector, state)
189+
);
190+
191+
mount(42);
192+
193+
expect(updateDetector).not.toHaveBeenCalled();
194+
unmount();
195+
});
196+
117197
test('does not update a detector that is already unmounted', () => {
118198
const updateDetector = jest.fn();
119199
const state = stateWithRelation('simultaneousWith', [
@@ -130,3 +210,37 @@ describe('useMountReactions', () => {
130210
unmount();
131211
});
132212
});
213+
214+
describe('hasExternalRelations', () => {
215+
test('is false without relations or with raw tags only', () => {
216+
expect(hasExternalRelations([gestureObject(1)])).toBe(false);
217+
expect(
218+
hasExternalRelations([gestureObject(1, { requireToFail: [7] })])
219+
).toBe(false);
220+
});
221+
222+
test('is false when relations only point at sibling gestures', () => {
223+
const first = gestureObject(1);
224+
const second = gestureObject(2);
225+
first.config = { simultaneousWith: [second] } as GestureType['config'];
226+
second.config = { simultaneousWith: [first] } as GestureType['config'];
227+
228+
expect(hasExternalRelations([first, second])).toBe(false);
229+
});
230+
231+
test('is true for a ref', () => {
232+
expect(
233+
hasExternalRelations([
234+
gestureObject(1, { blocksHandlers: [{ current: null }] }),
235+
])
236+
).toBe(true);
237+
});
238+
239+
test('is true for a gesture object attached by another detector', () => {
240+
expect(
241+
hasExternalRelations([
242+
gestureObject(1, { simultaneousWith: [gestureObject(2)] }),
243+
])
244+
).toBe(true);
245+
});
246+
});

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/attachHandlers.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
ALLOWED_PROPS,
1717
checkGestureCallbacksForWorklets,
1818
extractGestureRelations,
19+
hasExternalRelations,
1920
} from './utils';
2021

2122
interface AttachHandlersConfig {
@@ -55,6 +56,9 @@ export function attachHandlers({
5556
registerHandler(handler.handlerTag, handler, handler.config.testId);
5657
}
5758

59+
// React to every mount until the microtask below classifies the new gesture set
60+
preparedGesture.hasExternalRelations = true;
61+
5862
// Use queueMicrotask to extract handlerTags, because all refs should be initialized
5963
// when it's ran
6064
ghQueueMicrotask(() => {
@@ -73,9 +77,16 @@ export function attachHandlers({
7377
);
7478
}
7579

80+
preparedGesture.hasExternalRelations =
81+
hasExternalRelations(gesturesToAttach);
82+
7683
scheduleFlushOperations();
7784
});
7885

86+
// Set before the mount events below fire, so this detector's own mount
87+
// listener can tell its gestures apart from external ones
88+
preparedGesture.attachedGestures = gesturesToAttach;
89+
7990
for (const gesture of gesturesToAttach) {
8091
const actionType = gesture.shouldUseReanimated
8192
? ActionType.REANIMATED_WORKLET
@@ -101,8 +112,6 @@ export function attachHandlers({
101112
MountRegistry.gestureWillMount(gesture);
102113
}
103114

104-
preparedGesture.attachedGestures = gesturesToAttach;
105-
106115
if (preparedGesture.animatedHandlers) {
107116
const isAnimatedGesture = (g: GestureType) => g.shouldUseReanimated;
108117

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/index.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,10 @@ export const GestureDetector = (props: GestureDetectorProps) => {
117117
animatedHandlers: null,
118118
shouldUseReanimated: shouldUseReanimated,
119119
isMounted: false,
120+
// The real value is computed in the attach microtask, which runs after
121+
// this commit's effects. Until then react to every mount, so a relation
122+
// to a gesture mounting in the same commit is never skipped.
123+
hasExternalRelations: true,
120124
}).current;
121125

122126
const updateAttachedGestures = useDetectorUpdater(

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ export interface AttachedGestureState {
1414
shouldUseReanimated: boolean;
1515
// Whether the GestureDetector is mounted
1616
isMounted: boolean;
17+
// Whether any attached gesture has a relation to a gesture outside this
18+
// detector (a ref or a gesture object), which may resolve on a later mount
19+
hasExternalRelations: boolean;
1720
}
1821

1922
export interface GestureDetectorState {

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/updateHandlers.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
ALLOWED_PROPS,
1010
checkGestureCallbacksForWorklets,
1111
extractGestureRelations,
12+
hasExternalRelations,
1213
} from './utils';
1314

1415
export function updateHandlers(
@@ -33,6 +34,9 @@ export function updateHandlers(
3334
// Store attached gestures to avoid crash when gestures changed after queueing micro task
3435
const attachedGestures = preparedGesture.attachedGestures;
3536

37+
// React to every mount until the microtask below classifies the new gesture set
38+
preparedGesture.hasExternalRelations = true;
39+
3640
// Use queueMicrotask to extract handlerTags, because when it's ran, all refs should be updated
3741
// and handlerTags in BaseGesture references should be updated in the loop above (we need to wait
3842
// in case of external relations)
@@ -78,6 +82,10 @@ export function updateHandlers(
7882
registerHandler(handler.handlerTag, handler, handler.config.testId);
7983
}
8084

85+
// Classify the new instances: their configs reference the new siblings,
86+
// which the old attached objects would mistake for external gestures
87+
preparedGesture.hasExternalRelations = hasExternalRelations(newGestures);
88+
8189
if (preparedGesture.animatedHandlers && shouldUpdateSharedValueIfUsed) {
8290
const newHandlersValue = attachedGestures
8391
.filter((g) => g.shouldUseReanimated) // Ignore gestures that shouldn't run on UI

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/useMountReactions.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
11
import { useEffect } from 'react';
22

3+
import type { GestureMountListener } from '../../../mountRegistry';
34
import { MountRegistry } from '../../../mountRegistry';
45
import type { GestureRef } from '../gesture';
56
import type { AttachedGestureState } from './types';
67

8+
type MountedGesture = Parameters<GestureMountListener>[0];
9+
710
function shouldUpdateDetector(
811
relation: GestureRef[] | undefined,
9-
gesture: { handlerTag: number }
12+
gesture: MountedGesture
1013
) {
1114
if (relation === undefined) {
1215
return false;
1316
}
1417

1518
for (const entry of relation) {
16-
// Only refs can start pointing at a different handler after the detector
17-
// attached, because `current` is filled in when the gesture they point at
18-
// mounts. Gestures and numeric tags already carry their tag by then, so a
19-
// mount can never change what they resolve to and scanning them is wasted
20-
// work on every mount of every detector.
19+
// A gesture object gets its tag in `initialize`, when its own detector
20+
// attaches. The mount event carries that very object, so identity is enough.
21+
if (entry === gesture) {
22+
return true;
23+
}
24+
2125
if (entry === null || typeof entry !== 'object' || !('current' in entry)) {
2226
continue;
2327
}
@@ -44,6 +48,17 @@ export function useMountReactions(
4448
return;
4549
}
4650

51+
// Nothing here can resolve late, skip the scan (most detectors)
52+
if (!state.hasExternalRelations) {
53+
return;
54+
}
55+
56+
// Own gestures are resolved by the attach microtask, and on a reattach
57+
// they would match the sibling entries added by composition
58+
if (state.attachedGestures.includes(gesture as never)) {
59+
return;
60+
}
61+
4762
// At this point the ref in the gesture config should be updated, so we can check if one of the gestures
4863
// set in a relation with the gesture got mounted. If so, we need to update the detector to propagate
4964
// the changes to the native side.

‎packages/react-native-gesture-handler/src/handlers/gestures/GestureDetector/utils.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,42 @@ function extractValidHandlerTags(interactionGroup: GestureRef[] | undefined) {
6464
);
6565
}
6666

67+
// Relations that may resolve to a different handler after this detector attached:
68+
// refs (filled when the target mounts) and gesture objects attached by another
69+
// detector (they get a tag in `initialize`). Siblings added by composition are
70+
// attached together with the gesture, so they never resolve late.
71+
export function hasExternalRelations(gestures: GestureType[]) {
72+
const ownGestures = new Set(gestures);
73+
74+
for (const gesture of gestures) {
75+
const relations = [
76+
gesture.config.blocksHandlers,
77+
gesture.config.requireToFail,
78+
gesture.config.simultaneousWith,
79+
];
80+
81+
for (const relation of relations) {
82+
if (relation === undefined) {
83+
continue;
84+
}
85+
86+
for (const entry of relation) {
87+
if (typeof entry === 'number') {
88+
continue;
89+
}
90+
91+
// Anything but our own gesture object is external: a ref, or a gesture
92+
// attached by another detector
93+
if (!(entry instanceof BaseGesture) || !ownGestures.has(entry)) {
94+
return true;
95+
}
96+
}
97+
}
98+
}
99+
100+
return false;
101+
}
102+
67103
export function extractGestureRelations(gesture: GestureType) {
68104
const requireToFail = extractValidHandlerTags(gesture.config.requireToFail);
69105
const simultaneousWith = extractValidHandlerTags(

0 commit comments

Comments
 (0)