fix(client): Remove global emitter for EventNotifier and remove max…

… listener warning (#1163)

Addresses #868

Initially I just removed the max listener limit as the way we use it any
limit we set is going to be arbitrary and just an annoyance to real use
cases (like Kyle's).

Afterwards I figured it would also be pretty simple to remove the global
singleton emitter pattern altogether as it wasn't quite clear why it was
there, since we only ever instantiate one `EventNotifier` when we
electrify and that electric instance is meant to be shared across the
application instance.

Unless we have a particular reason for using a singleton I think
removing it is less likely to cause issues in the future (and help with
the work on being able to completely "clean up" electric resources)

.changeset/grumpy-years-cheer.md

@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Remove global `EventEmitter` and remove max listener warning.

clients/typescript/src/notifiers/event.ts

@@ -29,15 +29,6 @@ export const EVENT_NAMES = {
   connectivityStateChange: 'network:connectivity:changed',
 }
 
-// Global singleton that all event notifiers use by default. Emitting an event
-// on this object will notify all subscribers in the same thread. Cross thread
-// notifications use the `./bridge` notifiers.
-const globalEmitter = new EventEmitter()
-
-// Increase the maximum number of listeners because multiple components
-// use this same emitter instance.
-globalEmitter.setMaxListeners(250)
-
 export class EventNotifier implements Notifier {
   dbName: DbName
 
@@ -59,7 +50,12 @@ export class EventNotifier implements Notifier {
       byName: {},
     }
 
-    this.events = eventEmitter !== undefined ? eventEmitter : globalEmitter
+    this.events =
+      eventEmitter !== undefined
+        ? eventEmitter
+        : // initialise emitter with no limit to listeners as
+          // we don't want to limit the number of subscribers
+          new EventEmitter().setMaxListeners(Infinity)
   }
 
   attach(dbName: DbName, dbAlias: string): void {

clients/typescript/test/notifiers/event.test.ts

@@ -3,10 +3,12 @@ import { ConnectivityStateChangeNotification } from '../../src/notifiers'
 
 import { EventNotifier } from '../../src/notifiers/event'
 import { QualifiedTablename } from '../../src/util/tablename'
+import EventEmitter from 'events'
 
 test('subscribe to potential data changes', async (t) => {
-  const source = new EventNotifier('test.db')
-  const target = new EventNotifier('test.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('test.db', eventEmitter)
+  const target = new EventNotifier('test.db', eventEmitter)
 
   const notifications = []
 
@@ -20,9 +22,10 @@ test('subscribe to potential data changes', async (t) => {
 })
 
 test('potential data change subscriptions are scoped by dbName(s)', async (t) => {
-  const source = new EventNotifier('foo.db')
-  const t1 = new EventNotifier('foo.db')
-  const t2 = new EventNotifier('bar.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('foo.db', eventEmitter)
+  const t1 = new EventNotifier('foo.db', eventEmitter)
+  const t2 = new EventNotifier('bar.db', eventEmitter)
 
   const notifications = []
 
@@ -44,8 +47,9 @@ test('potential data change subscriptions are scoped by dbName(s)', async (t) =>
 })
 
 test('subscribe to actual data changes', async (t) => {
-  const source = new EventNotifier('test.db')
-  const target = new EventNotifier('test.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('test.db', eventEmitter)
+  const target = new EventNotifier('test.db', eventEmitter)
 
   const notifications = []
 
@@ -61,9 +65,10 @@ test('subscribe to actual data changes', async (t) => {
 })
 
 test('actual data change subscriptions are scoped by dbName', async (t) => {
-  const source = new EventNotifier('foo.db')
-  const t1 = new EventNotifier('foo.db')
-  const t2 = new EventNotifier('bar.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('foo.db', eventEmitter)
+  const t1 = new EventNotifier('foo.db', eventEmitter)
+  const t2 = new EventNotifier('bar.db', eventEmitter)
 
   const notifications = []
 
@@ -96,8 +101,9 @@ test('actual data change subscriptions are scoped by dbName', async (t) => {
 })
 
 test('subscribe to connectivity change events is scoped by dbName', async (t) => {
-  const source = new EventNotifier('test.db')
-  const target = new EventNotifier('test.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('test.db', eventEmitter)
+  const target = new EventNotifier('test.db', eventEmitter)
 
   const notifications: ConnectivityStateChangeNotification[] = []
 
@@ -115,8 +121,9 @@ test('subscribe to connectivity change events is scoped by dbName', async (t) =>
 })
 
 test('no more connectivity events after unsubscribe', async (t) => {
-  const source = new EventNotifier('test.db')
-  const target = new EventNotifier('test.db')
+  const eventEmitter = new EventEmitter()
+  const source = new EventNotifier('test.db', eventEmitter)
+  const target = new EventNotifier('test.db', eventEmitter)
 
   const notifications: ConnectivityStateChangeNotification[] = []