fix: keep track of loading effects and dispose them if unrequired aft…

…er load (#48)

src/media/local-track.spec.ts

@@ -1,6 +1,22 @@
 import { createMockedStream } from '../util/test-utils';
 import { LocalTrack } from './local-track';
 
+/**
+ * Create a mocked track effect.
+ *
+ * @returns A mocked TrackEvent.
+ */
+const createMockedTrackEffect = () => {
+  const effectStream = createMockedStream();
+  const effect = {
+    dispose: jest.fn().mockResolvedValue(undefined),
+    getUnderlyingStream: jest.fn().mockReturnValue(effectStream),
+    load: jest.fn().mockResolvedValue(undefined),
+  };
+
+  return { effectStream, effect };
+};
+
 /**
  * A dummy LocalTrack implementation so we can instantiate it for testing.
  */
@@ -83,3 +99,60 @@ describe('LocalTrack getNumActiveSimulcastLayers', () => {
     expect(localTrack.getNumActiveSimulcastLayers()).toBe(1);
   });
 });
+
+describe('LocalTrack addEffect', () => {
+  // eslint-disable-next-line jsdoc/require-jsdoc
+  const setup = () => {
+    const localStream = createMockedStream();
+    const localTrack = new TestLocalTrack(localStream);
+    const { effectStream, effect } = createMockedTrackEffect();
+    const emitCounts = { [LocalTrack.Events.UnderlyingTrackChange]: 0 };
+
+    localTrack.on(LocalTrack.Events.UnderlyingTrackChange, () => {
+      emitCounts[LocalTrack.Events.UnderlyingTrackChange] += 1;
+    });
+
+    return { localStream, localTrack, effectStream, effect, emitCounts };
+  };
+
+  it('loads and uses the effect when there is no loading effect', async () => {
+    expect.hasAssertions();
+
+    const { localTrack, effectStream, effect, emitCounts } = setup();
+
+    const addEffectPromise = localTrack.addEffect('test-effect', effect);
+
+    await expect(addEffectPromise).resolves.toBeUndefined();
+    expect(localTrack.underlyingStream).toBe(effectStream);
+    expect(emitCounts[LocalTrack.Events.UnderlyingTrackChange]).toBe(1);
+  });
+
+  it('does not use the effect when the loading effect is cleared during load', async () => {
+    expect.hasAssertions();
+
+    const { localStream, localTrack, effect, emitCounts } = setup();
+
+    // Add effect and immediately dispose all effects to clear loading effects
+    const addEffectPromise = localTrack.addEffect('test-effect', effect);
+    await localTrack.disposeEffects();
+
+    await expect(addEffectPromise).rejects.toThrow('not required after loading');
+    expect(localTrack.underlyingStream).toBe(localStream);
+    expect(emitCounts[LocalTrack.Events.UnderlyingTrackChange]).toBe(0);
+  });
+
+  it('loads and uses the latest effect when the loading effect changes during load', async () => {
+    expect.hasAssertions();
+
+    const { effect: firstEffect } = createMockedTrackEffect();
+    const { localTrack, effectStream, effect: secondEffect, emitCounts } = setup();
+
+    const firstAddEffectPromise = localTrack.addEffect('test-effect', firstEffect);
+    const secondAddEffectPromise = localTrack.addEffect('test-effect', secondEffect);
+
+    await expect(firstAddEffectPromise).rejects.toThrow('not required after loading');
+    await expect(secondAddEffectPromise).resolves.toBeUndefined();
+    expect(localTrack.underlyingStream).toBe(effectStream);
+    expect(emitCounts[LocalTrack.Events.UnderlyingTrackChange]).toBe(1);
+  });
+});

src/media/local-track.ts

@@ -77,6 +77,8 @@ export abstract class LocalTrack extends EventEmitter<TrackEvents> {
 
   private isPublished = false;
 
+  private loadingEffects: Map<string, TrackEffect> = new Map();
+
   private effects: Map<string, TrackEffect> = new Map();
 
   label: string;
@@ -238,9 +240,20 @@ export abstract class LocalTrack extends EventEmitter<TrackEvents> {
    * @param effect - The effect to add.
    */
   async addEffect(name: string, effect: TrackEffect): Promise<void> {
+    // Load the effect
+    this.loadingEffects.set(name, effect);
     await effect.load(this.underlyingStream);
+
+    // Check that the loaded effect is the latest one and dispose if not
+    if (effect !== this.loadingEffects.get(name)) {
+      await effect.dispose();
+      throw new Error(`Effect "${name}" not required after loading`);
+    }
+
+    // Use the loaded effect
     this.underlyingStream = effect.getUnderlyingStream();
     this.effects.set(name, effect);
+    this.loadingEffects.delete(name);
     this.emit(LocalTrackEvents.UnderlyingTrackChange);
   }
 
@@ -273,6 +286,10 @@ export abstract class LocalTrack extends EventEmitter<TrackEvents> {
    * Cleanup the local microphone track.
    */
   async disposeEffects(): Promise<void> {
+    // Clear effects that are loading to indicate that they are not needed
+    this.loadingEffects.clear();
+
+    // Dispose of any effects currently in use
     if (this.effects.size > 0) {
       await Promise.all(
         Array.from(this.effects.values(), (effect: TrackEffect) => effect.dispose())