Patch (#21568): Remove O(n) slowdown from event listeners (#21573)

## Description

This is a cherry-pick of #21568 

There are currently scenarios in which hundreds of thousands of event
subscriptions can occur on nodes in a SharedTree (that is a separate bug
that is also being fixed) and that case, the linear walk of the event
registrations that is required to validate that an event listener is not
a duplicate is prohibitively expensive. This removes the error, and
slightly relaxes the contract to be that duplicate event listeners cause
undefined behavior, rather than an error.

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

@@ -4,7 +4,7 @@
  */
 
 import type { IEvent } from "@fluidframework/core-interfaces";
-import { UsageError } from "@fluidframework/telemetry-utils/internal";
+import { getOrCreate } from "../util/index.js";
 
 /**
  * Convert a union of types to an intersection of those types. Useful for `TransformEvents`.
@@ -95,7 +95,7 @@ export interface Listenable<TListeners extends object> {
 	 * @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 an error.
+	 * Doing so will result in undefined behavior, and is not guaranteed to behave the same in future versions of this library.
 	 */
 	on<K extends keyof Listeners<TListeners>>(eventName: K, listener: TListeners[K]): Off;
 }
@@ -262,10 +262,6 @@ export class EventEmitter<TListeners extends Listeners<TListeners>>
 	 * @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.
-	 * @privateRemarks
-	 * TODO:
-	 * invoking the returned callback can error even if its only called once if the same listener was provided to two calls to "on".
-	 * This behavior is not documented and its unclear if its a bug or not: see note on listeners.
 	 */
 	public on<K extends keyof Listeners<TListeners>>(
 		eventName: K,
@@ -281,23 +277,7 @@ export class EventEmitter<TListeners extends Listeners<TListeners>>
 			}
 		};
 
-		const listeners = this.listeners.get(eventName);
-		if (listeners === undefined) {
-			const map = new Map([[off, listener]]);
-			this.listeners.set(eventName, map);
-		} else {
-			// If the same listener function is already registered, error.
-			// This policy may change in the future, but in the meantime this is a conservative choice that can accommodate multiple future eventing API/implementation options.
-			// For example, adding an `Listenable.off()` method in the future could be problematic if we allowed registering the same function twice (should `off(f)` deregister _both_ `f`s or just one?).
-			for (const l of listeners.values()) {
-				if (Object.is(l, listener)) {
-					throw new UsageError(
-						"The same listener may not be registered more than once for the same event",
-					);
-				}
-			}
-			listeners.set(off, listener);
-		}
+		getOrCreate(this.listeners, eventName, () => new Map()).set(off, listener);
 		return off;
 	}
 

packages/dds/tree/src/test/events/eventEmitter.spec.ts

@@ -96,12 +96,11 @@ describe("EventEmitter", () => {
 		assert(!closed);
 	});
 
-	it("correctly handles multiple event listeners", () => {
+	it("correctly handles multiple registrations for the same event", () => {
 		const emitter = createEmitter<TestEvents>();
 		let count: number;
 		const listener = () => (count += 1);
 		const off1 = emitter.on("open", listener);
-		assert.throws(() => emitter.on("open", listener)); // Registering the exact same function is currently forbidden.
 		const off2 = emitter.on("open", () => listener());
 
 		count = 0;
@@ -119,6 +118,30 @@ describe("EventEmitter", () => {
 		assert.strictEqual(count, 0);
 	});
 
+	// Note: This behavior is not contractually required (see docs for `Listenable.on()`),
+	// but is tested here to check for changes or regressions.
+	it("correctly handles multiple registrations of the same listener", () => {
+		const emitter = createEmitter<TestEvents>();
+		let count: number;
+		const listener = () => (count += 1);
+		const off1 = emitter.on("open", listener);
+		const off2 = emitter.on("open", listener);
+
+		count = 0;
+		emitter.emit("open");
+		assert.strictEqual(count, 2); // Listener should be fired twice
+
+		count = 0;
+		off1();
+		emitter.emit("open");
+		assert.strictEqual(count, 1);
+
+		count = 0;
+		off2();
+		emitter.emit("open");
+		assert.strictEqual(count, 0);
+	});
+
 	it("allows repeat deregistrations", () => {
 		const emitter = createEmitter<TestEvents>();
 		const deregister = emitter.on("open", () => {});