refactor(packages): replace disposer with abort controller (#449)

This commit is contained in:
rahim
2026-02-04 15:32:59 +11:00
committed by GitHub
parent 05af832632
commit b75316d23f
10 changed files with 93 additions and 281 deletions
+11 -9
View File
@@ -141,8 +141,8 @@ describe('SnapshotController', () => { ... });
// provider-mixin.test.ts — factory function export
describe('createStoreProviderMixin', () => { ... });
// disposer.test.ts — lowercase module/export
describe('disposer', () => { ... });
// event-like.test.ts — lowercase module/export
describe('event-like', () => { ... });
```
## Guidelines
@@ -337,20 +337,22 @@ destroy(): void {
### Cleanup Pattern
Use `Disposer` from `@videojs/utils/events` when managing multiple cleanup functions:
Use `AbortController` when managing multiple cleanups. It works with `listen` and any API that accepts a `signal`:
```ts
import { Disposer } from '@videojs/utils/events';
#disposer = new Disposer();
#disconnect: AbortController | null = null;
connect(): void {
this.#disposer.add(store.subscribe(...));
this.#disposer.add(listen(element, 'click', handler));
this.#disconnect?.abort();
this.#disconnect = new AbortController();
store.subscribe(() => {}, { signal: this.#disconnect.signal });
listen(element, 'click', handler, { signal: this.#disconnect.signal });
}
disconnect(): void {
this.#disposer.dispose();
this.#disconnect?.abort();
this.#disconnect = null;
}
```
+7 -4
View File
@@ -1,7 +1,6 @@
import { ContextConsumer } from '@lit/context';
import type { MediaContainer, PlayerStore, PlayerTarget } from '@videojs/core/dom';
import { listen, querySlot } from '@videojs/utils/dom';
import { Disposer } from '@videojs/utils/events';
import { noop } from '@videojs/utils/function';
import type { MediaElementConstructor } from '@/ui/media-element';
import type { PlayerContext } from '../player/context';
@@ -15,7 +14,7 @@ export function createContainerMixin<Store extends PlayerStore>(context: PlayerC
return <Class extends MediaElementConstructor>(BaseClass: Class) => {
class PlayerContainerElement extends BaseClass implements PlayerConsumer<Store>, MediaContainer {
#detach = noop;
#disposer = new Disposer();
#disconnect: AbortController | null = null;
#consumer = new ContextConsumer(this, {
context,
@@ -30,9 +29,12 @@ export function createContainerMixin<Store extends PlayerStore>(context: PlayerC
override connectedCallback() {
super.connectedCallback();
this.#disconnect?.abort();
this.#disconnect = new AbortController();
if (this.shadowRoot) {
const slot = querySlot(this.shadowRoot, '');
if (slot) this.#disposer.add(listen(slot, 'slotchange', () => this.#attachMedia()));
if (slot) listen(slot, 'slotchange', () => this.#attachMedia(), { signal: this.#disconnect.signal });
}
this.#attachMedia();
@@ -40,7 +42,8 @@ export function createContainerMixin<Store extends PlayerStore>(context: PlayerC
override disconnectedCallback() {
super.disconnectedCallback();
this.#disposer.dispose();
this.#disconnect?.abort();
this.#disconnect = null;
this.#detach();
}
+18
View File
@@ -292,6 +292,19 @@ const unsubscribe = store.subscribe(() => {
Mutations are auto-batched—multiple changes in the same tick trigger only one notification.
You can also pass an abort signal to clean up automatically:
```ts
const controller = new AbortController();
store.subscribe(() => {
const { volume } = store;
console.log('State changed:', volume);
}, { signal: controller.signal });
controller.abort(); // unsubscribes
```
### Pending Tasks
Track in-flight async operations:
@@ -491,6 +504,11 @@ state.subscribe(() => {
console.log('Changed:', volume);
});
// Optional abort signal for cleanup
const controller = new AbortController();
state.subscribe(() => {}, { signal: controller.signal });
controller.abort();
// Check if value is state
isState(state); // true
+23 -3
View File
@@ -1,10 +1,16 @@
import { noop } from '@videojs/utils/function';
export type StateChange = () => void;
export type UnknownState = Record<string, unknown>;
export interface SubscribeOptions {
signal?: AbortSignal;
}
export interface State<T> {
readonly current: Readonly<T>;
subscribe(callback: StateChange): () => void;
subscribe(callback: StateChange, options?: SubscribeOptions): () => void;
}
export interface WritableState<T> extends State<T> {
@@ -63,9 +69,23 @@ class StateContainer<T> implements WritableState<T> {
}
}
subscribe(callback: StateChange): () => void {
subscribe(callback: StateChange, options?: SubscribeOptions): () => void {
const signal = options?.signal;
if (signal?.aborted) return noop;
this.#listeners.add(callback);
return () => this.#listeners.delete(callback);
if (!signal) {
return () => this.#listeners.delete(callback);
}
const onAbort = () => this.#listeners.delete(callback);
signal.addEventListener('abort', onAbort, { once: true });
return () => {
signal.removeEventListener('abort', onAbort);
this.#listeners.delete(callback);
};
}
flush(): void {
+4 -4
View File
@@ -2,7 +2,7 @@ import { isNull, isObject } from '@videojs/utils/predicate';
import type { StoreCallbacks } from './config';
import { throwDestroyedError, throwNoTargetError } from './errors';
import type { AttachContext, Slice, StateContext } from './slice';
import type { StateChange, UnknownState, WritableState } from './state';
import type { StateChange, SubscribeOptions, UnknownState, WritableState } from './state';
import { createState } from './state';
const STORE_SYMBOL = Symbol('@videojs/store');
@@ -139,8 +139,8 @@ export function createStore<Target = unknown>(): <State>(
setupAbort.abort();
}
function subscribe(callback: StateChange): () => void {
return state.subscribe(callback);
function subscribe(callback: StateChange, options?: SubscribeOptions): () => void {
return state.subscribe(callback, options);
}
function reportError(error: unknown): void {
@@ -168,7 +168,7 @@ export interface BaseStore<Target = unknown, State = UnknownState> {
readonly state: State;
attach(target: Target): () => void;
destroy(): void;
subscribe(callback: StateChange): () => void;
subscribe(callback: StateChange, options?: SubscribeOptions): () => void;
}
export type Store<Target = unknown, State = UnknownState> = BaseStore<Target, State> & State;
@@ -103,6 +103,19 @@ describe('createState', () => {
flush();
expect(listener).toHaveBeenCalledOnce(); // still 1
});
it('respects abort signal', () => {
const state = createTestState();
const listener = vi.fn();
const controller = new AbortController();
state.subscribe(listener, { signal: controller.signal });
controller.abort();
state.patch({ volume: 0.5 });
flush();
expect(listener).not.toHaveBeenCalled();
});
});
describe('isState', () => {
@@ -203,6 +203,23 @@ describe('store', () => {
expect(listener).not.toHaveBeenCalled();
});
it('respects abort signal', () => {
const store = createStore<MockMedia>()(audioSlice);
const media = new MockMedia();
store.attach(media);
const listener = vi.fn();
const controller = new AbortController();
store.subscribe(listener, { signal: controller.signal });
controller.abort();
store.setVolume(0.5);
flush();
expect(listener).not.toHaveBeenCalled();
});
});
describe('destroy', () => {
-48
View File
@@ -1,48 +0,0 @@
/**
* A cleanup function that may be sync or async.
*/
export type CleanupFn = () => void | Promise<void>;
/**
* A collector for cleanup functions.
*
* Allows registering multiple cleanup functions and disposing them all at once.
*
* @example
* ```ts
* const disposer = new Disposer();
*
* disposer.add(listen(video, 'play', handlePlay));
* disposer.add(listen(video, 'pause', handlePause));
* disposer.add(animationFrame(render));
*
* // Later, clean up everything at once
* disposer.dispose();
* // or for async cleanups:
* await disposer.disposeAsync();
* ```
*/
export class Disposer {
#cleanups = new Set<CleanupFn>();
get size(): number {
return this.#cleanups.size;
}
add(cleanup: CleanupFn): void {
this.#cleanups.add(cleanup);
}
/** Run all cleanups sync. Use `disposeAsync()` for async cleanups. */
dispose(): void {
for (const cleanup of this.#cleanups) {
cleanup();
}
this.#cleanups.clear();
}
async disposeAsync(): Promise<void> {
await Promise.all([...this.#cleanups].map((cleanup) => cleanup()));
this.#cleanups.clear();
}
}
-1
View File
@@ -1,3 +1,2 @@
export * from './abort';
export * from './disposer';
export * from './event-like';
@@ -1,212 +0,0 @@
import { describe, expect, it, vi } from 'vitest';
import { Disposer } from '../disposer';
describe('disposer', () => {
describe('constructor', () => {
it('creates a disposer with size 0', () => {
const disposer = new Disposer();
expect(disposer.size).toBe(0);
});
it('tracks size as cleanups are added', () => {
const disposer = new Disposer();
disposer.add(() => {});
expect(disposer.size).toBe(1);
disposer.add(() => {});
expect(disposer.size).toBe(2);
});
});
describe('add', () => {
it('adds cleanup functions', () => {
const disposer = new Disposer();
const cleanup1 = vi.fn();
const cleanup2 = vi.fn();
disposer.add(cleanup1);
disposer.add(cleanup2);
expect(disposer.size).toBe(2);
});
it('does not call cleanups when adding', () => {
const disposer = new Disposer();
const cleanup = vi.fn();
disposer.add(cleanup);
expect(cleanup).not.toHaveBeenCalled();
});
});
describe('dispose', () => {
it('calls all cleanup functions', () => {
const disposer = new Disposer();
const cleanup1 = vi.fn();
const cleanup2 = vi.fn();
const cleanup3 = vi.fn();
disposer.add(cleanup1);
disposer.add(cleanup2);
disposer.add(cleanup3);
disposer.dispose();
expect(cleanup1).toHaveBeenCalledOnce();
expect(cleanup2).toHaveBeenCalledOnce();
expect(cleanup3).toHaveBeenCalledOnce();
});
it('clears the disposer after dispose', () => {
const disposer = new Disposer();
disposer.add(() => {});
disposer.add(() => {});
expect(disposer.size).toBe(2);
disposer.dispose();
expect(disposer.size).toBe(0);
});
it('can be called multiple times safely', () => {
const disposer = new Disposer();
const cleanup = vi.fn();
disposer.add(cleanup);
disposer.dispose();
disposer.dispose();
expect(cleanup).toHaveBeenCalledOnce();
});
it('allows adding new cleanups after dispose', () => {
const disposer = new Disposer();
const cleanup1 = vi.fn();
const cleanup2 = vi.fn();
disposer.add(cleanup1);
disposer.dispose();
disposer.add(cleanup2);
disposer.dispose();
expect(cleanup1).toHaveBeenCalledOnce();
expect(cleanup2).toHaveBeenCalledOnce();
});
});
describe('disposeAsync', () => {
it('calls all cleanup functions', async () => {
const disposer = new Disposer();
const cleanup1 = vi.fn();
const cleanup2 = vi.fn();
disposer.add(cleanup1);
disposer.add(cleanup2);
await disposer.disposeAsync();
expect(cleanup1).toHaveBeenCalledOnce();
expect(cleanup2).toHaveBeenCalledOnce();
});
it('awaits async cleanup functions', async () => {
const disposer = new Disposer();
const order: string[] = [];
disposer.add(async () => {
await Promise.resolve();
order.push('async1');
});
disposer.add(() => {
order.push('sync');
});
disposer.add(async () => {
await Promise.resolve();
order.push('async2');
});
await disposer.disposeAsync();
expect(order).toContain('async1');
expect(order).toContain('sync');
expect(order).toContain('async2');
});
it('clears the disposer after disposeAsync', async () => {
const disposer = new Disposer();
disposer.add(async () => {});
expect(disposer.size).toBe(1);
await disposer.disposeAsync();
expect(disposer.size).toBe(0);
});
it('handles mixed sync and async cleanups', async () => {
const disposer = new Disposer();
const results: number[] = [];
disposer.add(() => {
results.push(1);
});
disposer.add(async () => {
await new Promise((r) => setTimeout(r, 10));
results.push(2);
});
disposer.add(() => {
results.push(3);
});
await disposer.disposeAsync();
expect(results).toHaveLength(3);
expect(results).toContain(1);
expect(results).toContain(2);
expect(results).toContain(3);
});
});
describe('integration', () => {
it('works with real-world cleanup patterns', () => {
const disposer = new Disposer();
// Simulating event listener cleanup
const listeners = new Map<string, () => void>();
const addEventListener = (type: string, handler: () => void) => {
listeners.set(type, handler);
return () => {
listeners.delete(type);
};
};
disposer.add(addEventListener('click', () => {}));
disposer.add(addEventListener('keydown', () => {}));
expect(listeners.size).toBe(2);
disposer.dispose();
expect(listeners.size).toBe(0);
});
it('works with timer cleanup patterns', () => {
vi.useFakeTimers();
const disposer = new Disposer();
let timerFired = false;
const id = setTimeout(() => {
timerFired = true;
}, 1000);
disposer.add(() => clearTimeout(id));
disposer.dispose();
vi.advanceTimersByTime(2000);
expect(timerFired).toBe(false);
vi.useRealTimers();
});
});
});