fix(core): handle GestureObservers same as event listeners (#10538)

shirakaba · web-flow · commit d323672b2926 · 2024-05-06T17:50:01.000-07:00
diff --git a/packages/core/ui/core/view/index.d.ts b/packages/core/ui/core/view/index.d.ts
@@ -587,7 +587,7 @@ export abstract class View extends ViewCommon {
 	 */
 	public focus(): boolean;
 
-	public getGestureObservers(type: GestureTypes): Array<GesturesObserver>;
+	public getGestureObservers(type: GestureTypes): Array<GesturesObserver> | undefined;
 
 	/**
 	 * Removes listener(s) for the specified event name.
diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts
@@ -278,7 +278,14 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 		}
 	}
 
-	_observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void {
+	protected _observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void {
+		thisArg = thisArg || undefined;
+
+		if (this._gestureObservers[type]?.find((observer) => observer.callback === callback && observer.context === thisArg)) {
+			// Already added.
+			return;
+		}
+
 		if (!this._gestureObservers[type]) {
 			this._gestureObservers[type] = [];
 		}
@@ -291,6 +298,8 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 	}
 
 	public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any) {
+		thisArg = thisArg || undefined;
+
 		// Normalize "ontap" -> "tap"
 		const normalizedName = getEventOrGestureName(eventNames);
 
@@ -300,14 +309,16 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 
 		// If it's a gesture (and this Observable declares e.g. `static tapEvent`)
 		if (gesture && !this._isEvent(normalizedName)) {
-			this._observe(gesture, callback as unknown as (data: GestureEventData) => void, thisArg);
+			this._observe(gesture, callback, thisArg);
 			return;
 		}
 
 		super.addEventListener(normalizedName, callback, thisArg);
 	}
 
 	public removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any) {
+		thisArg = thisArg || undefined;
+
 		// Normalize "ontap" -> "tap"
 		const normalizedName = getEventOrGestureName(eventNames);
 
@@ -317,7 +328,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 
 		// If it's a gesture (and this Observable declares e.g. `static tapEvent`)
 		if (gesture && !this._isEvent(normalizedName)) {
-			this._disconnectGestureObservers(gesture);
+			this._disconnectGestureObservers(gesture, callback, thisArg);
 			return;
 		}
 
@@ -479,14 +490,34 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 		return this.constructor && `${name}Event` in this.constructor;
 	}
 
-	private _disconnectGestureObservers(type: GestureTypes): void {
+	private _disconnectGestureObservers(type: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void {
+		// Largely mirrors the implementation of Observable.innerRemoveEventListener().
+
 		const observers = this.getGestureObservers(type);
 		if (!observers) {
 			return;
 		}
 
-		for (const observer of observers) {
+		for (let i = 0; i < observers.length; i++) {
+			const observer = observers[i];
+
+			// If we have a `thisArg`, refine on both `callback` and `thisArg`.
+			if (thisArg && (observer.callback !== callback || observer.context !== thisArg)) {
+				continue;
+			}
+
+			// If we don't have a `thisArg`, refine only on `callback`.
+			if (callback && observer.callback !== callback) {
+				continue;
+			}
+
 			observer.disconnect();
+			observers.splice(i, 1);
+			i--;
+		}
+
+		if (!observers.length) {
+			delete this._gestureObservers[type];
 		}
 	}
 
diff --git a/packages/core/ui/gestures/gestures-common.ts b/packages/core/ui/gestures/gestures-common.ts
@@ -338,7 +338,7 @@ export function fromString(type: string): GestureTypes | undefined {
 export abstract class GesturesObserverBase implements GesturesObserverDefinition {
 	private _callback: (args: GestureEventData) => void;
 	private _target: View;
-	private _context: any;
+	private _context?: any;
 
 	public type: GestureTypes;
 
@@ -354,7 +354,7 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition
 		return this._context;
 	}
 
-	constructor(target: View, callback: (args: GestureEventData) => void, context: any) {
+	constructor(target: View, callback: (args: GestureEventData) => void, context?: any) {
 		this._target = target;
 		this._callback = callback;
 		this._context = context;
@@ -364,21 +364,6 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition
 	public abstract observe(type: GestureTypes);
 
 	public disconnect() {
-		// remove gesture observer from map
-		if (this.target) {
-			const list = this.target.getGestureObservers(this.type);
-			if (list && list.length > 0) {
-				for (let i = 0; i < list.length; i++) {
-					if (list[i].callback === this.callback) {
-						break;
-					}
-				}
-				list.length = 0;
-
-				this.target._gestureObservers[this.type] = undefined;
-				delete this.target._gestureObservers[this.type];
-			}
-		}
 		this._target = null;
 		this._callback = null;
 		this._context = null;
