fix: #417 ensure BrowserEventEmitter off removes listeners (#419)

Author: seratch

.changeset/tender-crews-promise.md

@@ -0,0 +1,5 @@
+---
+"@openai/agents-core": patch
+---
+
+fix: #417 ensure BrowserEventEmitter off removes listeners

packages/agents-core/src/shims/shims-browser.ts

@@ -16,28 +16,49 @@ export class BrowserEventEmitter<
 > implements EventEmitter<EventTypes>
 {
   #target = new EventTarget();
+  #listenerWrappers = new Map<
+    string,
+    Map<(...args: EventTypes[any]) => void, Set<EventListener>>
+  >();
 
   on<K extends keyof EventTypes>(
     type: K,
     listener: (...args: EventTypes[K]) => void,
   ) {
-    this.#target.addEventListener(
-      type as string,
-      ((event: CustomEvent) =>
-        listener(...(event.detail ?? []))) as EventListener,
-    );
+    const eventType = type as string;
+    let listenersForType = this.#listenerWrappers.get(eventType);
+    if (!listenersForType) {
+      listenersForType = new Map();
+      this.#listenerWrappers.set(eventType, listenersForType);
+    }
+    let wrappers = listenersForType.get(listener);
+    if (!wrappers) {
+      wrappers = new Set();
+      listenersForType.set(listener, wrappers);
+    }
+    const wrapper = ((event: CustomEvent) =>
+      listener(...(event.detail ?? []))) as EventListener;
+    wrappers.add(wrapper);
+    this.#target.addEventListener(eventType, wrapper);
     return this;
   }
 
   off<K extends keyof EventTypes>(
     type: K,
     listener: (...args: EventTypes[K]) => void,
   ) {
-    this.#target.removeEventListener(
-      type as string,
-      ((event: CustomEvent) =>
-        listener(...(event.detail ?? []))) as EventListener,
-    );
+    const eventType = type as string;
+    const listenersForType = this.#listenerWrappers.get(eventType);
+    const wrappers = listenersForType?.get(listener);
+    if (wrappers?.size) {
+      for (const wrapper of wrappers) {
+        this.#target.removeEventListener(eventType, wrapper);
+      }
+      listenersForType?.delete(listener);
+      if (listenersForType?.size === 0) {
+        this.#listenerWrappers.delete(eventType);
+      }
+    }
     return this;
   }
 

packages/agents-core/test/shims/browser-event-emitter.test.ts

@@ -0,0 +1,55 @@
+import { describe, expect, test } from 'vitest';
+
+import { BrowserEventEmitter } from '../../src/shims/shims-browser';
+
+describe('BrowserEventEmitter', () => {
+  test('off removes previously registered listener', () => {
+    const emitter = new BrowserEventEmitter<{ foo: [string] }>();
+    const calls: string[] = [];
+
+    const handler = (value: string) => {
+      calls.push(value);
+    };
+
+    emitter.on('foo', handler);
+    emitter.emit('foo', 'first');
+    emitter.off('foo', handler);
+    emitter.emit('foo', 'second');
+
+    expect(calls).toEqual(['first']);
+  });
+
+  test('once triggers listener only once', () => {
+    const emitter = new BrowserEventEmitter<{ foo: [string] }>();
+    let callCount = 0;
+
+    emitter.once('foo', () => {
+      callCount += 1;
+    });
+
+    emitter.emit('foo', 'first');
+    emitter.emit('foo', 'second');
+
+    expect(callCount).toBe(1);
+  });
+
+  test('multiple identical listeners fire for each registration and are removed by off', () => {
+    const emitter = new BrowserEventEmitter<{ foo: [string] }>();
+    const calls: string[] = [];
+
+    const handler = (value: string) => {
+      calls.push(value);
+    };
+
+    emitter.on('foo', handler);
+    emitter.on('foo', handler);
+
+    emitter.emit('foo', 'first');
+    expect(calls).toEqual(['first', 'first']);
+
+    emitter.off('foo', handler);
+    emitter.emit('foo', 'second');
+
+    expect(calls).toEqual(['first', 'first']);
+  });
+});