🧪 [Earwurm] Various bug fixes and test improvements (#22)

.changeset/lazy-rings-cough.md

@@ -0,0 +1,19 @@
+---
+'earwurm': minor
+---
+
+Bump node to `18.14.2`.
+
+Bump various dependencies.
+
+Fix issue with `Earworm > state` being set to `suspended` even after `closed`.
+
+Fix bug with `Sound` throwing an error on subsequent calls to `.play()`.
+
+Fix bug with `Sound > pause()` not working.
+
+Fix bug with `volume` and `mute` setters not actually changing `gain.value`.
+
+Both `Stack` and `Sound` can now accept a `GainNode` _(in addition to an `AudioNode`)_ as their `destination`.
+
+Simplify exported `types`.

.nvmrc

@@ -1 +1 @@
-18.14.1
+18.14.2

docs/roadmap.md

@@ -64,3 +64,19 @@ While we are intentionally keeping the scope of `Earwurm` to an absolute minimum
     - Example: This would allow me to know what `Stack` manages a particular `Sound`.
 16. Consider setting `Stack > state` to `closed` on `teardown`.
     - This way, there is a better mechanism to removing a `Stack` after listening to a `statechange`.
+
+### Additional
+
+Maybe there is some value in passing an array of `GainNode/AudioNode` for `destination`? If so, that might look something like:
+
+```ts
+export type AudioOutputs = [...GainNode[], AudioNode];
+
+constructor(readonly outputs: AudioOutputs);
+
+let lastConnection: GainNode | AudioNode = this.#gainNode;
+
+outputs.forEach((node) => {
+  lastConnection = lastConnection.connect(node);
+});
+```

package.json

@@ -57,22 +57,22 @@
   "devDependencies": {
     "@changesets/changelog-github": "^0.4.8",
     "@changesets/cli": "^2.26.0",
-    "@types/node": "^18.14.0",
-    "@typescript-eslint/eslint-plugin": "^5.52.0",
-    "@vitest/coverage-c8": "^0.28.5",
-    "@vitest/ui": "^0.28.5",
+    "@types/node": "^18.14.1",
+    "@typescript-eslint/eslint-plugin": "^5.53.0",
+    "@vitest/coverage-c8": "^0.29.1",
+    "@vitest/ui": "^0.29.1",
     "eslint": "^8.34.0",
     "eslint-config-prettier": "^8.6.0",
     "eslint-config-standard-with-typescript": "^34.0.0",
     "eslint-plugin-import": "^2.27.5",
     "eslint-plugin-n": "^15.6.1",
     "eslint-plugin-prettier": "^4.2.1",
     "eslint-plugin-promise": "^6.1.1",
-    "happy-dom": "^8.4.4",
+    "happy-dom": "^8.9.0",
     "typescript": "^4.9.5",
-    "vite": "^4.1.2",
-    "vite-plugin-dts": "^1.7.2",
-    "vitest": "^0.28.5"
+    "vite": "^4.1.4",
+    "vite-plugin-dts": "^2.0.2",
+    "vitest": "^0.29.1"
   },
   "peerDependencies": {
     "emitten": "^0.4.2"

src/Earwurm.ts

@@ -26,7 +26,6 @@ export class Earwurm extends EmittenCommon<ManagerEventMap> {
 
   #context = new AudioContext();
   #gainNode = this.#context.createGain();
-  #outputNode: AudioNode;
 
   #fadeSec = 0;
   #request: ManagerConfig['request'];
@@ -44,8 +43,8 @@ export class Earwurm extends EmittenCommon<ManagerEventMap> {
     this._volume = config?.volume ?? this._volume;
     this.#fadeSec = config?.fadeMs ? msToSec(config.fadeMs) : this.#fadeSec;
     this.#request = config?.request ?? undefined;
-    this.#outputNode = this.#gainNode.connect(this.#context.destination);
 
+    this.#gainNode.connect(this.#context.destination);
     this.#gainNode.gain.setValueAtTime(this._volume, this.#context.currentTime);
     this.#autoSuspend();
 
@@ -127,7 +126,7 @@ export class Earwurm extends EmittenCommon<ManagerEventMap> {
     const newStacks = entries.map(({id, path}) => {
       newKeys.push(id);
 
-      const newStack = new Stack(id, path, this.#context, this.#outputNode, {
+      const newStack = new Stack(id, path, this.#context, this.#gainNode, {
         fadeMs: secToMs(this.#fadeSec),
         request: this.#request,
       });
@@ -263,7 +262,12 @@ export class Earwurm extends EmittenCommon<ManagerEventMap> {
     this.#setState('suspending');
 
     const resolveSuspension = () => {
-      this.#setState('suspended');
+      if (this._state !== 'closed') {
+        // Because all of these `AudioContext > state`
+        // methods are async, we need to make sure we don't
+        // set `suspended` after already `closed`.
+        this.#setState('suspended');
+      }
 
       if (this.#suspendId) clearTimeout(this.#suspendId);
       this.#suspendId = 0;

src/Sound.ts

@@ -1,6 +1,7 @@
 import {EmittenCommon} from 'emitten';
 
 import {clamp, msToSec} from './utilities';
+import {tokens} from './tokens';
 import type {SoundId, SoundState, SoundEventMap, SoundConfig} from './types';
 
 export class Sound extends EmittenCommon<SoundEventMap> {
@@ -12,27 +13,26 @@ export class Sound extends EmittenCommon<SoundEventMap> {
   // "True private" properties
   #source: AudioBufferSourceNode;
   #gainNode: GainNode;
-  #outputNode: AudioNode;
   #fadeSec = 0;
+  #started = false;
 
   constructor(
     readonly id: SoundId,
     readonly buffer: AudioBuffer,
     readonly context: AudioContext,
-    readonly destination: AudioNode,
+    readonly destination: GainNode | AudioNode,
     config?: SoundConfig,
   ) {
     super();
 
     this._volume = config?.volume ?? this._volume;
     this.#fadeSec = config?.fadeMs ? msToSec(config.fadeMs) : this.#fadeSec;
 
-    this.#source = this.context.createBufferSource();
     this.#gainNode = this.context.createGain();
-    this.#outputNode = this.#gainNode.connect(this.destination);
+    this.#source = this.context.createBufferSource();
     this.#source.buffer = buffer;
 
-    this.#source.connect(this.#outputNode);
+    this.#source.connect(this.#gainNode).connect(this.destination);
     this.#gainNode.gain.setValueAtTime(this._volume, this.context.currentTime);
 
     // We could `emit` a "created" event, but it wouldn't get caught
@@ -97,18 +97,31 @@ export class Sound extends EmittenCommon<SoundEventMap> {
   }
 
   play() {
-    this.#source.start();
+    if (!this.#started) {
+      this.#source.start();
+      this.#started = true;
+    }
+
+    if (this._state === 'paused') {
+      this.#source.playbackRate.value = 1;
+      this.mute = false;
+    }
+
     this.#setState('playing');
 
     return this;
   }
 
   pause() {
-    // There is no `pause/resume` API for a `AudioBufferSourceNode`,
-    // so we may have to set `playbackRate.value = 0` instead.
+    if (this._state === 'paused') return this;
+
+    // There is no `pause/resume` API for a `AudioBufferSourceNode`.
+    // Lowering the `playbackRate` isn't ideal as technically the
+    // audio is still playing in the background and using resources.
     // https://github.com/WebAudio/web-audio-api-v2/issues/105
+    this.#source.playbackRate.value = tokens.minPlaybackRate;
+    this.mute = true;
 
-    this.#source.playbackRate.value = 0;
     this.#setState('paused');
 
     return this;
@@ -131,6 +144,8 @@ export class Sound extends EmittenCommon<SoundEventMap> {
   }
 
   #handleEnded = () => {
+    // Intentionally not setting `stopping` state here,
+    // but we may want ot consider a "ending" state instead.
     this.emit('ended', {id: this.id, source: this.#source});
   };
 }

src/Stack.ts

@@ -11,7 +11,7 @@ import type {
   StackEventMap,
   StackConfig,
   SoundId,
-  SoundEndedEvent,
+  SoundEventMap,
 } from './types';
 
 import {Sound} from './Sound';
@@ -34,7 +34,6 @@ export class Stack extends EmittenCommon<StackEventMap> {
   private _state: StackState = 'idle';
 
   #gainNode: GainNode;
-  #outputNode: AudioNode;
   #fadeSec = 0;
   #totalSoundsCreated = 0;
   #request: StackConfig['request'];
@@ -44,7 +43,7 @@ export class Stack extends EmittenCommon<StackEventMap> {
     readonly id: StackId,
     readonly path: string,
     readonly context: AudioContext,
-    readonly destination: AudioNode,
+    readonly destination: GainNode | AudioNode,
     config?: StackConfig,
   ) {
     super();
@@ -54,8 +53,8 @@ export class Stack extends EmittenCommon<StackEventMap> {
     this.#request = config?.request ?? undefined;
 
     this.#gainNode = this.context.createGain();
-    this.#outputNode = this.#gainNode.connect(this.destination);
 
+    this.#gainNode.connect(this.destination);
     this.#gainNode.gain.setValueAtTime(this._volume, this.context.currentTime);
   }
 
@@ -173,11 +172,11 @@ export class Stack extends EmittenCommon<StackEventMap> {
   }
 
   #create(id: SoundId, buffer: AudioBuffer) {
-    const newSound = new Sound(id, buffer, this.context, this.#outputNode, {
+    const newSound = new Sound(id, buffer, this.context, this.#gainNode, {
       fadeMs: secToMs(this.#fadeSec),
     });
 
-    newSound.on('statechange', this.#handleStateFromQueue);
+    newSound.on('statechange', this.#handleSoundState);
     newSound.once('ended', this.#handleSoundEnded);
 
     // We do not filter out identical `id` values,
@@ -219,7 +218,15 @@ export class Stack extends EmittenCommon<StackEventMap> {
     this.#setState(this.playing ? 'playing' : 'idle');
   };
 
-  #handleSoundEnded = (event: SoundEndedEvent) => {
+  #handleSoundState: SoundEventMap['statechange'] = (_state) => {
+    this.#handleStateFromQueue();
+  };
+
+  #handleSoundEnded: SoundEventMap['ended'] = (event) => {
     this.#setQueue(this.#queue.filter(({id}) => id !== event.id));
+
+    // We only set `stopping` state when `.stop()` is called.
+    // There is no `statechange` specifically for "ended".
+    this.#handleStateFromQueue();
   };
 }

src/tests/Earwurm.test.ts

@@ -1,8 +1,110 @@
-import {describe, it, expect} from 'vitest';
+import {describe, it, expect, vi} from 'vitest';
+
 import {Earwurm} from '../Earwurm';
+import {tokens} from '../tokens';
+import type {ManagerEventMap} from '../types';
+import {mockData} from './mock';
 
 describe('Earwurm component', () => {
-  it('is defined', () => {
-    expect(Earwurm).not.toBeNull();
+  describe('initialization', () => {
+    const testManager = new Earwurm();
+
+    it('is initialized with default values', () => {
+      expect(testManager).toBeInstanceOf(Earwurm);
+
+      // Class static properties
+      expect(Earwurm).toHaveProperty('maxStackSize', tokens.maxStackSize);
+      expect(Earwurm).toHaveProperty('suspendAfterMs', tokens.suspendAfterMs);
+
+      // Instance properties
+      expect(testManager).toHaveProperty('volume', 1);
+      expect(testManager).toHaveProperty('mute', false);
+      expect(testManager).toHaveProperty('unlocked', false);
+      expect(testManager).toHaveProperty('keys', []);
+      expect(testManager).toHaveProperty('state', 'suspended');
+      expect(testManager).toHaveProperty('playing', false);
+    });
+  });
+
+  // `mute` accessor is covered in `Abstract.test.ts`.
+  // describe('mute', () => {});
+
+  // `volume` accessor is covered in `Abstract.test.ts`.
+  // describe('volume', () => {});
+
+  // `unlocked` getter is covered in `unlock()` test.
+  // describe('unlocked', () => {});
+
+  describe('keys', () => {
+    it('contains ids of each active Stack', async () => {
+      const testManager = new Earwurm();
+
+      expect(testManager.keys).toHaveLength(0);
+
+      testManager.add(
+        {id: 'One', path: mockData.audio},
+        {id: 'Two', path: 'to/no/file.mp3'},
+        {id: 'Three', path: ''},
+      );
+
+      expect(testManager.keys).toStrictEqual(['One', 'Two', 'Three']);
+      testManager.remove('One');
+      expect(testManager.keys).toStrictEqual(['Two', 'Three']);
+    });
+  });
+
+  describe('state', () => {
+    it('triggers `statechange` event for every state', async () => {
+      const testManager = new Earwurm();
+      const spyState: ManagerEventMap['statechange'] = vi.fn((_state) => {});
+
+      testManager.on('statechange', spyState);
+
+      expect(spyState).not.toBeCalled();
+      expect(testManager.state).toBe('suspended');
+
+      testManager.unlock();
+
+      const clickEvent = new Event('click');
+      document.dispatchEvent(clickEvent);
+
+      // await vi.advanceTimersToNextTimerAsync();
+
+      expect(spyState).toBeCalledTimes(1);
+      expect(spyState).toBeCalledWith('running');
+
+      testManager.stop();
+
+      // `suspending/suspended` are called in rapid succession.
+      expect(spyState).toBeCalledTimes(3);
+      expect(spyState).toBeCalledWith('suspending');
+      expect(spyState).toBeCalledWith('suspended');
+
+      testManager.teardown();
+
+      expect(spyState).toBeCalledTimes(4);
+      expect(spyState).toBeCalledWith('closed');
+
+      // TODO: Find a way to mock an "interruption" so we
+      // can test the `interrupted` state change.
+      expect(spyState).not.toBeCalledTimes(5);
+    });
+  });
+
+  describe('playing', () => {
+    it('is `true` when any Sound is `playing`', async () => {
+      const testManager = new Earwurm();
+
+      testManager.add({id: 'One', path: mockData.audio});
+
+      const stack = testManager.get('One');
+      const sound = await stack?.prepare();
+
+      expect(testManager.playing).toBe(false);
+      sound?.play();
+      expect(testManager.playing).toBe(true);
+      vi.advanceTimersByTime(10);
+      expect(testManager.playing).toBe(false);
+    });
   });
 });