Add off() method to Listenable event interface (#23046)

## Description

This exposes the `off()` method on the `Listenable` event interface,
providing an additional way to unsubscribe from events. The current way
of unsubscribing, via the deregistration function returned by `on()`, is
still supported. Having both options available is meant to give
customers options without being opinionated about the best pattern, as
it depends on preference and use case.

To prevent the two strategies from interfering with each other, it has
also been made illegal to register the same listener more than once for
the same event. It is now documented as throwing an error rather than as
undefined behavior.

This also adds a test for registering events with keys that are JS
symbols and does some minor doc cleanup.

.changeset/lovely-taxis-notice.md

@@ -0,0 +1,27 @@
+---
+"fluid-framework": minor
+"@fluidframework/tree": minor
+---
+---
+"section": tree
+---
+
+SharedTree event listeners that implement `Listenable` now allow deregistration of event listeners via an `off()` function.
+
+The ability to deregister events via a callback returned by `on()` remains the same.
+Both strategies will remain supported and consumers of SharedTree events may choose which method of deregistration they prefer in a given instance.
+
+```typescript
+// The new behavior
+function deregisterViaOff(view: TreeView<MySchema>): {
+	const listener = () => { /* ... */ };
+	view.events.on("commitApplied", listener); // Register
+	view.events.off("commitApplied", listener); // Deregister
+}
+
+// The existing behavior (still supported)
+function deregisterViaCallback(view: TreeView<MySchema>): {
+	const off = view.events.on("commitApplied", () => { /* ... */ }); // Register
+	off(); // Deregister
+}
+```

packages/dds/tree/api-report/tree.alpha.api.md

@@ -417,6 +417,7 @@ export type LazyItem<Item = unknown> = Item | (() => Item);
 
 // @public @sealed
 export interface Listenable<TListeners extends object> {
+    off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
     on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
 

packages/dds/tree/api-report/tree.beta.api.md

@@ -225,6 +225,7 @@ export type LazyItem<Item = unknown> = Item | (() => Item);
 
 // @public @sealed
 export interface Listenable<TListeners extends object> {
+    off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
     on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
 

packages/dds/tree/api-report/tree.legacy.alpha.api.md

@@ -225,6 +225,7 @@ export type LazyItem<Item = unknown> = Item | (() => Item);
 
 // @public @sealed
 export interface Listenable<TListeners extends object> {
+    off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
     on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
 

packages/dds/tree/api-report/tree.legacy.public.api.md

@@ -225,6 +225,7 @@ export type LazyItem<Item = unknown> = Item | (() => Item);
 
 // @public @sealed
 export interface Listenable<TListeners extends object> {
+    off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
     on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
 

packages/dds/tree/api-report/tree.public.api.md

@@ -225,6 +225,7 @@ export type LazyItem<Item = unknown> = Item | (() => Item);
 
 // @public @sealed
 export interface Listenable<TListeners extends object> {
+    off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
     on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
 

packages/dds/tree/src/events/emitter.ts

@@ -3,7 +3,8 @@
  * Licensed under the MIT License.
  */
 
-import { setInNestedMap } from "../util/index.js";
+import { UsageError } from "@fluidframework/telemetry-utils/internal";
+import { getOrCreate } from "../util/index.js";
 import type { Listenable, Listeners, Off } from "./listeners.js";
 
 /**
@@ -103,7 +104,7 @@ export interface HasListeners<TListeners extends Listeners<TListeners>> {
  * ```typescript
  * class MyExposingClass {
  * 	private readonly _events = createEmitter<MyEvents>();
- *  public readonly events: Listenable<MyEvents> = this._events;
+ * 	public readonly events: Listenable<MyEvents> = this._events;
  *
  * 	private load() {
  * 		this._events.emit("loaded");
@@ -117,7 +118,7 @@ export class EventEmitter<TListeners extends Listeners<TListeners>>
 {
 	protected readonly listeners = new Map<
 		keyof TListeners,
-		Map<Off, (...args: any[]) => TListeners[keyof TListeners]>
+		Set<(...args: any[]) => TListeners[keyof TListeners]>
 	>();
 
 	// Because this is protected and not public, calling this externally (not from a subclass) makes sending events to the constructed instance impossible.
@@ -132,11 +133,10 @@ export class EventEmitter<TListeners extends Listeners<TListeners>>
 		if (listeners !== undefined) {
 			// Current tsc (5.4.5) cannot spread `args` into `listener()`.
 			const argArray: unknown[] = args;
-
 			// This explicitly copies listeners so that new listeners added during this call to emit will not receive this event.
-			for (const [off, listener] of [...listeners]) {
+			for (const listener of [...listeners]) {
 				// If listener has been unsubscribed while invoking other listeners, skip it.
-				if (listeners.has(off)) {
+				if (listeners.has(listener)) {
 					listener(...argArray);
 				}
 			}
@@ -159,29 +159,32 @@ export class EventEmitter<TListeners extends Listeners<TListeners>>
 		return [];
 	}
 
-	/**
-	 * Register an event listener.
-	 * @param eventName - the name of the event
-	 * @param listener - the handler to run when the event is fired by the emitter
-	 * @returns a function which will deregister the listener when run.
-	 * This function will error if called more than once.
-	 */
 	public on<K extends keyof Listeners<TListeners>>(
 		eventName: K,
 		listener: TListeners[K],
 	): Off {
-		const off: Off = () => {
-			const currentListeners = this.listeners.get(eventName);
-			if (currentListeners?.delete(off) === true) {
-				if (currentListeners.size === 0) {
-					this.listeners.delete(eventName);
-					this.noListeners?.(eventName);
-				}
-			}
-		};
+		const listeners = getOrCreate(this.listeners, eventName, () => new Set());
+		if (listeners.has(listener)) {
+			const eventDescription =
+				typeof eventName === "symbol" ? eventName.description : String(eventName.toString());
 
-		setInNestedMap(this.listeners, eventName, off, listener);
-		return off;
+			throw new UsageError(
+				`Attempted to register the same listener object twice for event ${eventDescription}`,
+			);
+		}
+		listeners.add(listener);
+		return () => this.off(eventName, listener);
+	}
+
+	public off<K extends keyof Listeners<TListeners>>(
+		eventName: K,
+		listener: TListeners[K],
+	): void {
+		const listeners = this.listeners.get(eventName);
+		if (listeners?.delete(listener) === true && listeners.size === 0) {
+			this.listeners.delete(eventName);
+			this.noListeners?.(eventName);
+		}
 	}
 
 	public hasListeners(eventName?: keyof TListeners): boolean {
@@ -239,6 +242,10 @@ class ComposableEventEmitter<TListeners extends Listeners<TListeners>>
  * 	public on<K extends keyof MyEvents>(eventName: K, listener: MyEvents[K]): Off {
  * 		return this.events.on(eventName, listener);
  * 	}
+ *
+ * 	public off<K extends keyof MyEvents>(eventName: K, listener: MyEvents[K]): void {
+ * 		return this.events.off(eventName, listener);
+ * 	}
  * }
  * ```
  */

packages/dds/tree/src/events/listeners.ts

@@ -50,13 +50,25 @@ export interface Listenable<TListeners extends object> {
 	/**
 	 * Register an event listener.
 	 * @param eventName - The name of the event.
-	 * @param listener - the handler to run when the event is fired by the emitter
-	 * @returns a {@link Off | function} which will deregister the listener when called.
-	 * This deregistration function is idempotent and therefore may be safely called more than once with no effect.
-	 * @remarks Do not register the exact same `listener` object for the same event more than once.
-	 * Doing so will result in undefined behavior, and is not guaranteed to behave the same in future versions of this library.
+	 * @param listener - The listener function to run when the event is fired.
+	 * @returns A {@link Off | function} which will deregister the listener when called.
+	 * Calling the deregistration function more than once will have no effect.
+	 *
+	 * Listeners may also be deregistered by passing the listener to {@link Listenable.off | off()}.
+	 * @remarks Registering the exact same `listener` object for the same event more than once will throw an error.
+	 * If registering the same listener for the same event multiple times is desired, consider using a wrapper function for the second subscription.
 	 */
 	on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
+
+	/**
+	 * Deregister an event listener.
+	 * @param eventName - The name of the event.
+	 * @param listener - The listener function to remove from the current set of event listeners.
+	 * @remarks If `listener` is not currently registered, this method will have no effect.
+	 *
+	 * Listeners may also be deregistered by calling the {@link Off | deregistration function} returned when they are {@link Listenable.on | registered}.
+	 */
+	off<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): void;
 }
 
 /**