From addcb0aa1242fb66bff83b08629c901947ca3c02 Mon Sep 17 00:00:00 2001 From: Rahim Date: Sat, 31 Jan 2026 18:44:20 +1100 Subject: [PATCH 1/2] refactor(store): simplify queue - remove task state tracking BREAKING CHANGE: Queue no longer tracks task state (pending/success/error). - Remove task.ts and all task state types - Remove queue.tasks, queue.subscribe, queue.reset - Remove store.queue getter (queue is now internal) - Remove guard combinators (all, any, timeout) - Keep only: queue.enqueue, queue.abort, queue.destroy - Wrap handlers with abortable() for consistent abort behavior --- packages/store/src/core/feature.ts | 5 - packages/store/src/core/guard.ts | 67 -- packages/store/src/core/index.ts | 5 +- packages/store/src/core/queue.ts | 229 +------ packages/store/src/core/request.ts | 2 +- packages/store/src/core/store.ts | 39 +- packages/store/src/core/task.ts | 75 --- packages/store/src/core/tests/guard.test.ts | 150 ----- .../src/core/tests/integration/store.test.ts | 25 - packages/store/src/core/tests/queue.test.ts | 626 ++++++------------ .../store/src/core/tests/queue.types.test.ts | 71 -- packages/store/src/core/tests/store.test.ts | 21 - .../store/src/core/tests/store.types.test.ts | 81 +-- packages/store/src/core/tests/task.test.ts | 239 ------- .../store/src/core/tests/task.types.test.ts | 75 --- 15 files changed, 235 insertions(+), 1475 deletions(-) delete mode 100644 packages/store/src/core/task.ts delete mode 100644 packages/store/src/core/tests/guard.test.ts delete mode 100644 packages/store/src/core/tests/queue.types.test.ts delete mode 100644 packages/store/src/core/tests/task.test.ts delete mode 100644 packages/store/src/core/tests/task.types.test.ts diff --git a/packages/store/src/core/feature.ts b/packages/store/src/core/feature.ts index 7edc10bdc..037a4cbd7 100644 --- a/packages/store/src/core/feature.ts +++ b/packages/store/src/core/feature.ts @@ -1,5 +1,4 @@ import type { UnionToIntersection } from '@videojs/utils/types'; -import type { EnsureTaskRecord } from './queue'; import type { Request, RequestConfig, @@ -78,10 +77,6 @@ export type UnionFeatureRequests[]> = Un ResolveFeatureRequestHandlers >; -export type UnionFeatureTasks[]> = EnsureTaskRecord< - UnionToIntersection> ->; - // ---------------------------------------- // createFeature // ---------------------------------------- diff --git a/packages/store/src/core/guard.ts b/packages/store/src/core/guard.ts index 5f7208780..1e6643f9d 100644 --- a/packages/store/src/core/guard.ts +++ b/packages/store/src/core/guard.ts @@ -1,7 +1,3 @@ -import { isBoolean } from '@videojs/utils/predicate'; - -import { StoreError } from './errors'; - /** * Result of a guard check. * @@ -25,66 +21,3 @@ export interface GuardContext { * A guard gates request execution. */ export type Guard = (ctx: GuardContext) => GuardResult; - -/** - * Combine guards: All must pass (truthy). - */ -export function all(...guards: Guard[]): Guard { - return async (ctx) => { - for (const guard of guards) { - const result = await guard(ctx); - if (!result) return false; - } - - return true; - }; -} - -/** - * Combine guards: Any must pass (first truthy wins). - */ -export function any(...guards: Guard[]): Guard { - return (ctx) => { - const results = guards.map((g) => g(ctx)); - - // Check sync results first - if (results.includes(true)) return true; - - // Filter to promises only - const promises = results.filter((r): r is Promise => !isBoolean(r)); - - if (promises.length === 0) return false; - - // Race: first truthy wins, all falsy = false - return new Promise((resolve, reject) => { - let pending = promises.length; - for (const p of promises) { - p.then((value) => { - if (value) resolve(value); - else if (--pending === 0) resolve(false); - }, reject); - } - }); - }; -} - -/** - * Add timeout to a guard. - */ -export function timeout(guard: Guard, ms: number, name = 'guard'): Guard { - return async (ctx) => { - const result = guard(ctx); - - if (isBoolean(result)) { - return result; - } - - return Promise.race([ - result, - new Promise((_, reject) => { - const timer = setTimeout(() => reject(new StoreError('TIMEOUT', { message: `Timeout: ${name}` })), ms); - ctx.signal.addEventListener('abort', () => clearTimeout(timer)); - }), - ]); - }; -} diff --git a/packages/store/src/core/index.ts b/packages/store/src/core/index.ts index d7fc01796..712129af9 100644 --- a/packages/store/src/core/index.ts +++ b/packages/store/src/core/index.ts @@ -1,9 +1,8 @@ export * from './computed'; export * from './errors'; export * from './feature'; -export * from './guard'; -export * from './queue'; +export type { Guard, GuardContext, GuardResult } from './guard'; +export type { TaskKey } from './queue'; export * from './request'; export * from './state'; export * from './store'; -export * from './task'; diff --git a/packages/store/src/core/queue.ts b/packages/store/src/core/queue.ts index c917e375e..71e444b1b 100644 --- a/packages/store/src/core/queue.ts +++ b/packages/store/src/core/queue.ts @@ -1,250 +1,79 @@ import { abortable } from '@videojs/utils/events'; -import { isUndefined } from '@videojs/utils/predicate'; import { StoreError } from './errors'; -import type { Request, RequestMeta, RequestMode } from './request'; -import type { StateChange, WritableState } from './state'; -import { createState } from './state'; -import type { ErrorTask, PendingTask, SuccessTask, Task, TaskContext, TaskKey } from './task'; +import type { RequestMode } from './request'; // ---------------------------------------- // Types // ---------------------------------------- -export type TaskRecord = { - [K in TaskKey]: Request; -}; +export type TaskKey = string | symbol; -export type DefaultTaskRecord = Record>; - -export type EnsureTaskRecord = T extends TaskRecord ? T : never; - -export interface QueueTask { - name: string; - key: Key; +export interface QueueTask { + key: TaskKey; mode?: RequestMode; - input?: Input; - meta?: RequestMeta | null; - handler: (ctx: TaskContext) => Promise; + handler: (ctx: { signal: AbortSignal }) => Promise; } -export type TasksRecord = { - [K in keyof Tasks]?: Task, Tasks[K]['input'], Tasks[K]['output']>; -}; - // ---------------------------------------- // Implementation // ---------------------------------------- -export class Queue { - readonly #tasks: WritableState>; - readonly #sharedPromises = new Map>(); - +export class Queue { + #pending = new Map(); + #shared = new Map>(); #destroyed = false; - /** Current task records. */ - get tasks(): Readonly> { - return this.#tasks.current; - } - - /** Subscribe to task changes. */ - subscribe(callback: StateChange): () => void { - return this.#tasks.subscribe(callback); - } - - constructor() { - this.#tasks = createState({}); - } - get destroyed(): boolean { return this.#destroyed; } - /** Clear settled task(s). If name provided, clears that task. If no name, clears all settled. */ - reset(name?: keyof Tasks): void { - if (!isUndefined(name)) { - const task = this.#tasks.current[name]; - if (!task || task.status === 'pending') return; - - this.#tasks.delete(name); - - return; - } - - for (const key of Reflect.ownKeys(this.#tasks.current) as (keyof Tasks)[]) { - const task = this.#tasks.current[key]; - - if (task && task.status !== 'pending') { - this.#tasks.delete(key); - } - } - } - - enqueue( - task: QueueTask, Tasks[K]['input'], Tasks[K]['output']> - ): Promise { - const { name, key, mode = 'exclusive', input, meta = null, handler } = task; - + enqueue({ key, mode = 'exclusive', handler }: QueueTask): Promise { if (this.#destroyed) { return Promise.reject(new StoreError('DESTROYED')); } - // Shared mode: join existing pending task with same key + // Shared mode: join existing if (mode === 'shared') { - const existingPromise = this.#sharedPromises.get(key); - if (existingPromise) { - return existingPromise; - } + const existing = this.#shared.get(key); + if (existing) return existing as Promise; } - // Supersede any pending task with the same key (may have different name) - for (const existingTask of Object.values(this.#tasks.current)) { - if (existingTask?.key === key && existingTask.status === 'pending') { - existingTask.abort.abort(new StoreError('SUPERSEDED')); - } - } + // Supersede pending with same key + this.#pending.get(key)?.abort(new StoreError('SUPERSEDED')); + + const abort = new AbortController(); + this.#pending.set(key, abort); - const promise = new Promise((resolve, reject) => { - this.#executeNow({ - id: Symbol('@videojs/task'), - name, - key, - input, - meta, - handler, - resolve, - reject, - }); + // Wrap with abortable so promise rejects on abort even if handler doesn't handle signal + const promise = abortable(handler({ signal: abort.signal }), abort.signal).finally(() => { + this.#pending.delete(key); + this.#shared.delete(key); }); - // Track promise for shared mode if (mode === 'shared') { - this.#sharedPromises.set(key, promise); - // Use .then() to avoid unhandled rejection from .finally() propagating errors - promise.then( - () => this.#sharedPromises.delete(key), - () => this.#sharedPromises.delete(key) - ); + this.#shared.set(key, promise); } return promise; } - /** Abort task(s). If name provided, aborts that task. If no name, aborts all. */ - abort(name?: keyof Tasks): void { - if (!isUndefined(name)) { - const task = this.#tasks.current[name]; - if (task?.status === 'pending') { - task.abort.abort(new StoreError('ABORTED')); - } - + abort(key?: TaskKey): void { + if (key !== undefined) { + this.#pending.get(key)?.abort(new StoreError('ABORTED')); return; } const error = new StoreError('ABORTED'); - - for (const task of Object.values(this.#tasks.current)) { - if (task?.status === 'pending') { - task.abort.abort(error); - } + for (const controller of this.#pending.values()) { + controller.abort(error); } } destroy(): void { if (this.#destroyed) return; - this.#destroyed = true; this.abort(); - - // Clear all tasks - for (const key of Reflect.ownKeys(this.#tasks.current)) { - this.#tasks.delete(key); - } - - this.#sharedPromises.clear(); + this.#pending.clear(); + this.#shared.clear(); } - - async #executeNow(params: { - id: symbol; - name: string; - key: TaskKey; - input: Tasks[K]['input']; - meta: RequestMeta | null; - handler: (ctx: TaskContext) => Promise; - resolve: (value: Tasks[K]['output']) => void; - reject: (error: unknown) => void; - }): Promise { - const { id, name, key, input, meta, handler, resolve, reject } = params; - - const abort = new AbortController(); - const startedAt = Date.now(); - - const pendingTask: PendingTask = { - status: 'pending', - id, - name, - key, - input, - startedAt, - abort, - meta, - }; - - // Store tasks by name for controller access - this.#tasks.set(name as keyof Tasks, pendingTask); - - try { - const result = await abortable(handler({ input, signal: abort.signal }), abort.signal); - - resolve(result); - - // Only update if we're still the current task for this name - const currentTask = this.#tasks.current[name]; - - if (currentTask?.id === id) { - this.#tasks.set(name as keyof Tasks, { - ...currentTask, - status: 'success', - settledAt: Date.now(), - output: result, - } satisfies SuccessTask); - } - } catch (error) { - reject(error); - - // Only update if we're still the current task for this name - const currentTask = this.#tasks.current[name as keyof Tasks]; - - if (currentTask?.id === id) { - this.#tasks.set(name as keyof Tasks, { - ...currentTask, - status: 'error', - settledAt: Date.now(), - error, - cancelled: abort.signal.aborted, - } satisfies ErrorTask); - } - } - } -} - -// ---------------------------------------- -// Factory -// ---------------------------------------- - -/** - * Create a queue for managing task execution. - * - * @example - * // Loose typing (default) - * const queue = createQueue(); - * - * @example - * // Strongly typed keys - * const queue = createQueue<{ - * 'playback': Request; - * 'volume': Request; - * }>(); - */ -export function createQueue(): Queue { - return new Queue(); } diff --git a/packages/store/src/core/request.ts b/packages/store/src/core/request.ts index e04758fed..48a4b2653 100644 --- a/packages/store/src/core/request.ts +++ b/packages/store/src/core/request.ts @@ -1,7 +1,7 @@ import type { EventLike } from '@videojs/utils/events'; import { isFunction, isObject } from '@videojs/utils/predicate'; import type { Guard } from './guard'; -import type { TaskKey } from './task'; +import type { TaskKey } from './queue'; // ---------------------------------------- // Symbols diff --git a/packages/store/src/core/store.ts b/packages/store/src/core/store.ts index 167f00470..c7faeddc2 100644 --- a/packages/store/src/core/store.ts +++ b/packages/store/src/core/store.ts @@ -1,25 +1,18 @@ import { abortable } from '@videojs/utils/events'; import { isNull } from '@videojs/utils/predicate'; import { StoreError } from './errors'; -import type { - AnyFeature, - FeatureUpdate, - UnionFeatureRequests, - UnionFeatureState, - UnionFeatureTarget, - UnionFeatureTasks, -} from './feature'; +import type { AnyFeature, FeatureUpdate, UnionFeatureRequests, UnionFeatureState, UnionFeatureTarget } from './feature'; + import { Queue } from './queue'; import type { RequestMeta, RequestMetaInit, ResolvedRequestConfig } from './request'; import { CANCEL_ALL, createRequestMeta, resolveRequestCancel, resolveRequestKey } from './request'; import type { StateChange, WritableState } from './state'; import { createState } from './state'; -import type { PendingTask, Task, TaskContext } from './task'; export class Store[] = AnyFeature[]> { readonly #config: StoreConfig; readonly #features: Features; - readonly #queue: Queue>; + readonly #queue: Queue; readonly #request: UnionFeatureRequests; readonly #requestConfigs: Map>; readonly #setupAbort = new AbortController(); @@ -33,7 +26,7 @@ export class Store[] = AnyFeature>(); + this.#queue = new Queue(); this.#state = createState(this.#createInitialState() as UnionFeatureState & object); this.#requestConfigs = this.#buildRequestConfigs(); @@ -66,10 +59,6 @@ export class Store[] = AnyFeature> { - return this.#queue; - } - get features(): Features { return this.#features; } @@ -229,7 +218,7 @@ export class Store[] = AnyFeature, input: unknown, meta: RequestMeta | null @@ -245,7 +234,7 @@ export class Store[] = AnyFeature { + const handler = async ({ signal }: { signal: AbortSignal }) => { const target = this.#target; if (!target) { @@ -265,22 +254,12 @@ export class Store[] = AnyFeature; - const task = tasks[name]; - - this.#handleError({ - request: task?.status === 'pending' ? task : undefined, - error, - }); - + this.#handleError({ error }); throw error; } } @@ -322,7 +301,6 @@ export type AnyStoreConfig = StoreConfig; export interface StoreConfig[]> { features: Features; - queue?: Queue>; onSetup?: (ctx: StoreSetupContext) => void; onAttach?: (ctx: StoreAttachContext) => void; onError?: (ctx: StoreErrorContext) => void; @@ -340,7 +318,6 @@ export interface StoreAttachContext[ } export interface StoreErrorContext[]> { - request?: PendingTask | undefined; store: Store; error: unknown; } @@ -364,5 +341,3 @@ export type InferStoreFeatures = S extends Store = UnionFeatureState>; export type InferStoreRequests = UnionFeatureRequests>; - -export type InferStoreTasks = UnionFeatureTasks>; diff --git a/packages/store/src/core/task.ts b/packages/store/src/core/task.ts deleted file mode 100644 index 4d685bfed..000000000 --- a/packages/store/src/core/task.ts +++ /dev/null @@ -1,75 +0,0 @@ -import type { RequestMeta } from './request'; - -// ---------------------------------------- -// Types -// ---------------------------------------- - -export type TaskKey = T & (string | symbol); - -export type EnsureTaskKey = T extends string | symbol ? T : never; - -export interface TaskBase { - id: symbol; - name: string; - key: Key; - input: Input; - startedAt: number; - meta: RequestMeta | null; -} - -export interface PendingTask extends TaskBase { - status: 'pending'; - abort: AbortController; -} - -export interface SuccessTask - extends TaskBase { - status: 'success'; - settledAt: number; - output: Output; -} - -export interface ErrorTask extends TaskBase { - status: 'error'; - settledAt: number; - error: unknown; - cancelled: boolean; -} - -export type Task = - | PendingTask - | SuccessTask - | ErrorTask; - -export type SettledTask = - | SuccessTask - | ErrorTask; - -export interface TaskContext { - input: Input; - signal: AbortSignal; -} - -// ---------------------------------------- -// Type Guards -// ---------------------------------------- - -/** Check if task is pending (in-flight). */ -export function isPendingTask(task: Task | undefined): task is PendingTask { - return task?.status === 'pending'; -} - -/** Check if task is settled (success or error). */ -export function isSettledTask(task: Task | undefined): task is SettledTask { - return task?.status === 'success' || task?.status === 'error'; -} - -/** Check if task is a success. */ -export function isSuccessTask(task: Task | undefined): task is SuccessTask { - return task?.status === 'success'; -} - -/** Check if task is an error. */ -export function isErrorTask(task: Task | undefined): task is ErrorTask { - return task?.status === 'error'; -} diff --git a/packages/store/src/core/tests/guard.test.ts b/packages/store/src/core/tests/guard.test.ts deleted file mode 100644 index a6c149364..000000000 --- a/packages/store/src/core/tests/guard.test.ts +++ /dev/null @@ -1,150 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; - -import { StoreError } from '../errors'; -import { all, any, timeout } from '../guard'; - -describe('guard', () => { - const createContext = () => ({ - target: {}, - signal: new AbortController().signal, - }); - - describe('all', () => { - it('passes when all guards return true', async () => { - const guard = all( - () => true, - () => true, - () => Promise.resolve(true) - ); - expect(await guard(createContext())).toBe(true); - }); - - it('fails on first falsy guard', async () => { - const thirdGuard = vi.fn(() => true); - const guard = all( - () => true, - () => false, - thirdGuard - ); - expect(await guard(createContext())).toBe(false); - expect(thirdGuard).not.toHaveBeenCalled(); - }); - - it('handles async guards', async () => { - const guard = all( - () => Promise.resolve(true), - async () => { - await new Promise((r) => setTimeout(r, 10)); - return true; - } - ); - expect(await guard(createContext())).toBe(true); - }); - - it('fails on async falsy result', async () => { - const guard = all( - () => true, - () => Promise.resolve(false) - ); - expect(await guard(createContext())).toBe(false); - }); - }); - - describe('any', () => { - it('passes on first truthy sync result', () => { - const guard = any( - () => false, - () => true, - () => false - ); - expect(guard(createContext())).toBe(true); - }); - - it('returns false when all sync guards fail', () => { - const guard = any( - () => false, - () => false - ); - expect(guard(createContext())).toBe(false); - }); - - it('races async guards - first truthy wins', async () => { - const guard = any( - () => new Promise((r) => setTimeout(() => r(false), 50)), - () => new Promise((r) => setTimeout(() => r(true), 10)), - () => new Promise((r) => setTimeout(() => r(false), 30)) - ); - expect(await guard(createContext())).toBe(true); - }); - - it('returns false if all async guards resolve falsy', async () => { - const guard = any( - () => Promise.resolve(false), - () => Promise.resolve(0), - () => Promise.resolve(null) - ); - expect(await guard(createContext())).toBe(false); - }); - - it('prefers sync truthy over pending async', () => { - const guard = any( - () => new Promise(() => {}), // never resolves - () => true - ); - expect(guard(createContext())).toBe(true); - }); - }); - - describe('timeout', () => { - it('passes sync truthy immediately', async () => { - const guard = timeout(() => true, 1000); - expect(await guard(createContext())).toBe(true); - }); - - it('fails sync falsy immediately', async () => { - const guard = timeout(() => false, 1000); - expect(await guard(createContext())).toBe(false); - }); - - it('passes async within timeout', async () => { - const guard = timeout(() => new Promise((r) => setTimeout(() => r(true), 10)), 1000); - expect(await guard(createContext())).toBe(true); - }); - - it('throws StoreError with TIMEOUT code', async () => { - vi.useFakeTimers(); - - const guard = timeout( - () => new Promise(() => {}), // never resolves - 100, - 'waitForReady' - ); - - const promise = guard(createContext()); - vi.advanceTimersByTime(100); - - await expect(promise).rejects.toThrow(StoreError); - await expect(promise).rejects.toMatchObject({ - code: 'TIMEOUT', - message: 'Timeout: waitForReady', - }); - - vi.useRealTimers(); - }); - - it('clears timeout on abort', async () => { - vi.useFakeTimers(); - - const controller = new AbortController(); - const guard = timeout(() => new Promise(() => {}), 100); - - guard({ target: {}, signal: controller.signal }); - controller.abort(); - - // Should not throw - timeout was cleared - vi.advanceTimersByTime(200); - - vi.useRealTimers(); - }); - }); -}); diff --git a/packages/store/src/core/tests/integration/store.test.ts b/packages/store/src/core/tests/integration/store.test.ts index e726e2540..5128f6331 100644 --- a/packages/store/src/core/tests/integration/store.test.ts +++ b/packages/store/src/core/tests/integration/store.test.ts @@ -553,29 +553,4 @@ describe('immediate execution', () => { expect(target.paused).toBe(false); expect(store.state.paused).toBe(false); }); - - it('task is pending synchronously after request', async () => { - const feature = createFeature()({ - initialState: {}, - getSnapshot: () => ({}), - subscribe: () => {}, - request: { - action: async () => { - await new Promise((r) => setTimeout(r, 10)); - return 'done'; - }, - }, - }); - - const store = createStore({ features: [feature] }); - store.attach({}); - - const promise = store.request.action(); - - // Synchronous check - task is pending immediately, no microtask needed - expect(store.queue.tasks.action?.status).toBe('pending'); - - await promise; - expect(store.queue.tasks.action?.status).toBe('success'); - }); }); diff --git a/packages/store/src/core/tests/queue.test.ts b/packages/store/src/core/tests/queue.test.ts index 00550eb22..e275b015b 100644 --- a/packages/store/src/core/tests/queue.test.ts +++ b/packages/store/src/core/tests/queue.test.ts @@ -1,52 +1,42 @@ import { describe, expect, it, vi } from 'vitest'; -import { createQueue } from '../queue'; - -/** Wait for microtask queue to flush. */ -const flush = () => new Promise((r) => queueMicrotask(r)); +import { Queue } from '../queue'; describe('Queue', () => { describe('enqueue', () => { - it('executes task immediately', async () => { - const queue = createQueue(); + it('executes handler immediately', async () => { + const queue = new Queue(); const handler = vi.fn().mockResolvedValue('result'); const promise = queue.enqueue({ - name: 'test', - key: 'test-key', + key: 'test', handler, }); - // Handler called synchronously expect(handler).toHaveBeenCalled(); await expect(promise).resolves.toBe('result'); }); - it('task is pending synchronously after enqueue', async () => { - const queue = createQueue(); + it('passes signal to handler', async () => { + const queue = new Queue(); + let receivedSignal: AbortSignal | undefined; - const promise = queue.enqueue({ - name: 'test', - key: 'test-key', - handler: async () => 'result', + await queue.enqueue({ + key: 'test', + handler: async ({ signal }) => { + receivedSignal = signal; + return 'result'; + }, }); - // Synchronous check - task is pending immediately - const { test: pendingTest } = queue.tasks; - expect(pendingTest?.status).toBe('pending'); - - await promise; - - const { test: settledTest } = queue.tasks; - expect(settledTest?.status).toBe('success'); + expect(receivedSignal).toBeInstanceOf(AbortSignal); }); - it('aborts pending task with same key', async () => { - const queue = createQueue(); + it('supersedes pending task with same key', async () => { + const queue = new Queue(); let aborted = false; - const longRunning = queue.enqueue({ - name: 'long', + const first = queue.enqueue({ key: 'shared', handler: async ({ signal }) => { await new Promise((resolve, reject) => { @@ -54,7 +44,7 @@ describe('Queue', () => { signal.addEventListener('abort', () => { clearTimeout(timeout); aborted = true; - reject(new Error('aborted')); + reject(signal.reason); }); }); }, @@ -63,23 +53,21 @@ describe('Queue', () => { // Let first task start await new Promise((r) => setTimeout(r, 10)); - const superseding = queue.enqueue({ - name: 'supersede', + const second = queue.enqueue({ key: 'shared', handler: async () => 'new result', }); - await expect(longRunning).rejects.toThrow(); - await expect(superseding).resolves.toBe('new result'); + await expect(first).rejects.toMatchObject({ code: 'SUPERSEDED' }); + await expect(second).resolves.toBe('new result'); expect(aborted).toBe(true); }); - it('parallel execution with different keys', async () => { - const queue = createQueue(); + it('runs tasks with different keys in parallel', async () => { + const queue = new Queue(); const results: string[] = []; const task1 = queue.enqueue({ - name: 'task1', key: 'key-a', handler: async () => { results.push('a-start'); @@ -90,7 +78,6 @@ describe('Queue', () => { }); const task2 = queue.enqueue({ - name: 'task2', key: 'key-b', handler: async () => { results.push('b-start'); @@ -106,507 +93,284 @@ describe('Queue', () => { }); }); - describe('abort', () => { - it('abort(name) aborts pending task', async () => { - const queue = createQueue(); - let aborted = false; + describe('mode', () => { + it('exclusive mode (default) supersedes same key', async () => { + const queue = new Queue(); - const promise = queue.enqueue({ - name: 'test', + const first = queue.enqueue({ key: 'k', handler: async ({ signal }) => { await new Promise((_, reject) => { - signal.addEventListener('abort', () => { - aborted = true; - reject(signal.reason); - }); + signal.addEventListener('abort', () => reject(signal.reason)); setTimeout(() => {}, 1000); }); }, }); await new Promise((r) => setTimeout(r, 10)); - queue.abort('test'); - - await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); - expect(aborted).toBe(true); - }); - }); - - describe('destroy', () => { - it('rejects after destroy', async () => { - const queue = createQueue(); - queue.destroy(); - - await expect(queue.enqueue({ name: 't', key: 'k', handler: vi.fn() })).rejects.toMatchObject({ - code: 'DESTROYED', - }); - }); - - it('aborts all pending on destroy', async () => { - const queue = createQueue(); - const aborted = vi.fn(); - - const promise = queue.enqueue({ - name: 'task', - key: 'k', - handler: async ({ signal }) => { - signal.addEventListener('abort', aborted); - await new Promise((r) => setTimeout(r, 100)); - }, - }); - - await new Promise((r) => setTimeout(r, 10)); - queue.destroy(); - - await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); - expect(aborted).toHaveBeenCalled(); - expect(queue.destroyed).toBe(true); - }); - - it('clears all task references on destroy', async () => { - const queue = createQueue(); - - await queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => 'result', - }); - - const { task } = queue.tasks; - expect(task?.status).toBe('success'); - - queue.destroy(); - - expect(Reflect.ownKeys(queue.tasks).length).toBe(0); - }); - }); - - describe('cleanup edge cases', () => { - it('allows pending tasks to self-cleanup after destroy', async () => { - const queue = createQueue(); - const cleanupSpy = vi.fn(); - const promise = queue.enqueue({ - name: 'task', + const second = queue.enqueue({ key: 'k', - handler: async ({ signal }) => { - signal.addEventListener('abort', () => cleanupSpy('aborted')); - try { - await new Promise((_, reject) => { - signal.addEventListener('abort', () => reject(signal.reason)); - }); - } finally { - cleanupSpy('cleanup'); - } - }, + mode: 'exclusive', + handler: async () => 'second', }); - await new Promise((r) => setTimeout(r, 10)); - - const { task: pendingTask } = queue.tasks; - expect(pendingTask?.status).toBe('pending'); - - queue.destroy(); - - await promise.catch(() => {}); - expect(cleanupSpy).toHaveBeenCalledWith('aborted'); - expect(cleanupSpy).toHaveBeenCalledWith('cleanup'); - - const { task: destroyedTask } = queue.tasks; - expect(destroyedTask).toBeUndefined(); - }); - }); - - describe('subscribe', () => { - it('subscribe returns an unsubscribe function', () => { - const queue = createQueue(); - const listener = vi.fn(); - - const unsubscribe = queue.subscribe(listener); - - expect(unsubscribe).toBeTypeOf('function'); + await expect(first).rejects.toMatchObject({ code: 'SUPERSEDED' }); + await expect(second).resolves.toBe('second'); }); - it('notifies when task changes', async () => { - const queue = createQueue(); - const listener = vi.fn(); + it('shared mode joins existing promise with same key', async () => { + const queue = new Queue(); + let callCount = 0; - queue.subscribe(listener); + const handler = async () => { + callCount++; + await new Promise((r) => setTimeout(r, 50)); + return 'result'; + }; - const promise = queue.enqueue({ - name: 'test', - key: 'test-key', - handler: vi.fn().mockResolvedValue('result'), - }); + const first = queue.enqueue({ key: 'shared', mode: 'shared', handler }); + const second = queue.enqueue({ key: 'shared', mode: 'shared', handler }); - await promise; - await flush(); + const [result1, result2] = await Promise.all([first, second]); - // Notified at least once (pending and settled may batch together) - expect(listener).toHaveBeenCalled(); + expect(callCount).toBe(1); + expect(result1).toBe('result'); + expect(result2).toBe('result'); }); - it('unsubscribe stops notifications', async () => { - const queue = createQueue(); - const listener = vi.fn(); + it('shared mode creates new task after first completes', async () => { + const queue = new Queue(); + let callCount = 0; - const unsubscribe = queue.subscribe(listener); - unsubscribe(); + const handler = async () => { + callCount++; + return `result-${callCount}`; + }; - await queue.enqueue({ - name: 'test', - key: 'test-key', - handler: vi.fn().mockResolvedValue('result'), - }); - await flush(); + const first = await queue.enqueue({ key: 'shared', mode: 'shared', handler }); + const second = await queue.enqueue({ key: 'shared', mode: 'shared', handler }); - expect(listener).not.toHaveBeenCalled(); - }); - - it('supports multiple subscribers', async () => { - const queue = createQueue(); - const listener1 = vi.fn(); - const listener2 = vi.fn(); - - queue.subscribe(listener1); - queue.subscribe(listener2); - - await queue.enqueue({ - name: 'test', - key: 'test-key', - handler: vi.fn().mockResolvedValue('result'), - }); - await flush(); - - // Called once per batch (pending + settled batched together) - expect(listener1).toHaveBeenCalled(); - expect(listener2).toHaveBeenCalled(); + expect(callCount).toBe(2); + expect(first).toBe('result-1'); + expect(second).toBe('result-2'); }); }); - describe('task lifecycle', () => { - it('task starts as pending and transitions to success', async () => { - const queue = createQueue(); + describe('abort', () => { + it('abort(key) aborts pending task with that key', async () => { + const queue = new Queue(); + let aborted = false; const promise = queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => { - await new Promise((r) => setTimeout(r, 10)); - return 'result'; + key: 'test', + handler: async ({ signal }) => { + await new Promise((_, reject) => { + signal.addEventListener('abort', () => { + aborted = true; + reject(signal.reason); + }); + setTimeout(() => {}, 1000); + }); }, }); - await new Promise((r) => setTimeout(r, 5)); - - const { task: pendingTask } = queue.tasks; - expect(pendingTask?.status).toBe('pending'); - expect(pendingTask?.name).toBe('task'); - - await promise; - - const { task: successTask } = queue.tasks; - expect(successTask?.status).toBe('success'); + await new Promise((r) => setTimeout(r, 10)); + queue.abort('test'); - if (successTask?.status === 'success') { - expect(successTask.output).toBe('result'); - expect(successTask.settledAt).toBeGreaterThan(successTask.startedAt); - } + await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); + expect(aborted).toBe(true); }); - it('task starts as pending and transitions to error', async () => { - const queue = createQueue(); - const error = new Error('test error'); + it('abort() without key aborts all pending tasks', async () => { + const queue = new Queue(); + const abortedKeys: string[] = []; - const promise = queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => { - await new Promise((r) => setTimeout(r, 10)); - throw error; + const taskA = queue.enqueue({ + key: 'a', + handler: async ({ signal }) => { + await new Promise((_, reject) => { + signal.addEventListener('abort', () => { + abortedKeys.push('a'); + reject(signal.reason); + }); + setTimeout(() => {}, 1000); + }); }, }); - await new Promise((r) => setTimeout(r, 5)); - - const { task: pendingTask } = queue.tasks; - expect(pendingTask?.status).toBe('pending'); - - await expect(promise).rejects.toThrow('test error'); - - const { task: errorTask } = queue.tasks; - expect(errorTask?.status).toBe('error'); - - if (errorTask?.status === 'error') { - expect(errorTask.error).toBe(error); - expect(errorTask.cancelled).toBe(false); - } - }); - - it('aborted task has cancelled flag set to true', async () => { - const queue = createQueue(); - - const promise = queue.enqueue({ - name: 'task', - key: 'k', + const taskB = queue.enqueue({ + key: 'b', handler: async ({ signal }) => { await new Promise((_, reject) => { - signal.addEventListener('abort', () => reject(signal.reason)); + signal.addEventListener('abort', () => { + abortedKeys.push('b'); + reject(signal.reason); + }); setTimeout(() => {}, 1000); }); }, }); await new Promise((r) => setTimeout(r, 10)); + queue.abort(); - const { task: pendingTask } = queue.tasks; - expect(pendingTask?.status).toBe('pending'); - - queue.abort('task'); - await promise.catch(() => {}); - - const { task: errorTask } = queue.tasks; - expect(errorTask?.status).toBe('error'); - - if (errorTask?.status === 'error') { - expect(errorTask.cancelled).toBe(true); - } + await expect(taskA).rejects.toMatchObject({ code: 'ABORTED' }); + await expect(taskB).rejects.toMatchObject({ code: 'ABORTED' }); + expect(abortedKeys).toContain('a'); + expect(abortedKeys).toContain('b'); }); - it('new request replaces settled task', async () => { - const queue = createQueue(); - - await queue.enqueue({ - name: 'first', - key: 'k', - handler: async () => 'first-result', - }); - - const { first } = queue.tasks; - expect(first?.status).toBe('success'); - - if (first?.status === 'success') { - expect(first.output).toBe('first-result'); - } - - await queue.enqueue({ - name: 'second', - key: 'k', - handler: async () => 'second-result', - }); - - const { second } = queue.tasks; - expect(second?.status).toBe('success'); - - if (second?.status === 'success') { - expect(second.output).toBe('second-result'); - expect(second.name).toBe('second'); - } + it('abort(key) is no-op for non-existent key', () => { + const queue = new Queue(); + // Should not throw + queue.abort('nonexistent'); }); }); - describe('reset', () => { - it('clears settled task', async () => { - const queue = createQueue(); + describe('destroy', () => { + it('rejects enqueue after destroy', async () => { + const queue = new Queue(); + queue.destroy(); - await queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => 'result', + await expect(queue.enqueue({ key: 'k', handler: vi.fn() })).rejects.toMatchObject({ + code: 'DESTROYED', }); + }); - const { task: settledTask } = queue.tasks; - expect(settledTask?.status).toBe('success'); - - queue.reset('task'); + it('sets destroyed flag', () => { + const queue = new Queue(); + expect(queue.destroyed).toBe(false); - const { task: clearedTask } = queue.tasks; - expect(clearedTask).toBeUndefined(); + queue.destroy(); + expect(queue.destroyed).toBe(true); }); - it('is no-op when task is pending', async () => { - const queue = createQueue(); + it('aborts all pending tasks on destroy', async () => { + const queue = new Queue(); + const aborted = vi.fn(); const promise = queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => { - await new Promise((r) => setTimeout(r, 50)); - return 'result'; + key: 'task', + handler: async ({ signal }) => { + signal.addEventListener('abort', aborted); + await new Promise((r) => setTimeout(r, 100)); }, }); await new Promise((r) => setTimeout(r, 10)); + queue.destroy(); - const { task: beforeReset } = queue.tasks; - expect(beforeReset?.status).toBe('pending'); - - queue.reset('task'); - - const { task: afterReset } = queue.tasks; - expect(afterReset?.status).toBe('pending'); - - await promise; + await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); + expect(aborted).toHaveBeenCalled(); }); - it('is no-op when task does not exist', () => { - const queue = createQueue(); - - queue.reset('nonexistent'); - - expect(queue.tasks.nonexistent).toBeUndefined(); + it('destroy is idempotent', () => { + const queue = new Queue(); + queue.destroy(); + queue.destroy(); // Should not throw + expect(queue.destroyed).toBe(true); }); + }); - it('notifies subscribers when reset clears a task', async () => { - const queue = createQueue(); - const listener = vi.fn(); + describe('cleanup', () => { + it('cleans up pending map after task completes', async () => { + const queue = new Queue(); await queue.enqueue({ - name: 'task', - key: 'k', + key: 'test', handler: async () => 'result', }); - queue.subscribe(listener); + // Enqueue same key should not supersede (no pending task exists) + const handler = vi.fn().mockResolvedValue('new'); + await queue.enqueue({ key: 'test', handler }); - queue.reset('task'); - await flush(); - - expect(listener).toHaveBeenCalledTimes(1); - expect(queue.tasks.task).toBeUndefined(); + expect(handler).toHaveBeenCalled(); }); - it('does not notify subscribers when task does not exist', async () => { - const queue = createQueue(); - const listener = vi.fn(); + it('cleans up pending map after task fails', async () => { + const queue = new Queue(); - queue.subscribe(listener); - queue.reset('nonexistent'); - await flush(); - - expect(listener).not.toHaveBeenCalled(); - }); + await queue + .enqueue({ + key: 'test', + handler: async () => { + throw new Error('fail'); + }, + }) + .catch(() => {}); - it('resets all settled tasks when no key provided', async () => { - const queue = createQueue(); + // Enqueue same key should work (no pending task to supersede) + const handler = vi.fn().mockResolvedValue('new'); + await queue.enqueue({ key: 'test', handler }); - await queue.enqueue({ - name: 'a', - key: 'a', - handler: async () => 'a-result', - }); - await queue.enqueue({ - name: 'b', - key: 'b', - handler: async () => 'b-result', - }); - - expect(queue.tasks.a?.status).toBe('success'); - expect(queue.tasks.b?.status).toBe('success'); - - queue.reset(); - - expect(queue.tasks.a).toBeUndefined(); - expect(queue.tasks.b).toBeUndefined(); + expect(handler).toHaveBeenCalled(); }); - it('preserves pending tasks when resetting all', async () => { - const queue = createQueue(); + it('cleans up shared map after task completes', async () => { + const queue = new Queue(); + let callCount = 0; await queue.enqueue({ - name: 'settled', - key: 'settled', - handler: async () => 'done', - }); - - const pendingPromise = queue.enqueue({ - name: 'pending', - key: 'pending', + key: 'shared', + mode: 'shared', handler: async () => { - await new Promise((r) => setTimeout(r, 100)); - return 'pending-done'; + callCount++; + return 'result'; }, }); - await new Promise((r) => setTimeout(r, 10)); - expect(queue.tasks.settled?.status).toBe('success'); - expect(queue.tasks.pending?.status).toBe('pending'); - - queue.reset(); - - expect(queue.tasks.settled).toBeUndefined(); - expect(queue.tasks.pending?.status).toBe('pending'); - - await pendingPromise; - }); - }); - - describe('tasks property', () => { - it('returns current snapshot', async () => { - const queue = createQueue(); + // Second call should create new task since first completed await queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => 'result', - }); - - // Tasks returns the current snapshot directly - expect(queue.tasks.task?.status).toBe('success'); - }); - - it('reflects changes immediately', async () => { - const queue = createQueue(); - - await queue.enqueue({ - name: 'first', - key: 'k', - handler: async () => 'first', - }); - - // Getter reflects updates - expect(queue.tasks.first?.status).toBe('success'); - - await queue.enqueue({ - name: 'second', - key: 'k', - handler: async () => 'second', + key: 'shared', + mode: 'shared', + handler: async () => { + callCount++; + return 'result2'; + }, }); - expect(queue.tasks.second?.status).toBe('success'); + expect(callCount).toBe(2); }); }); describe('symbol keys', () => { - it('supports symbol names', async () => { - const queue = createQueue(); - const name = Symbol('task'); + it('supports symbol as key', async () => { + const queue = new Queue(); + const key = Symbol('task'); - await queue.enqueue({ - name: name as unknown as string, - key: name, + const result = await queue.enqueue({ + key, handler: async () => 'result', }); - const task = queue.tasks[name as unknown as string]; - expect(task?.status).toBe('success'); - if (task?.status === 'success') { - expect(task.output).toBe('result'); - } + expect(result).toBe('result'); }); - }); - describe('meta propagation', () => { - it('meta defaults to null when not provided', async () => { - const queue = createQueue(); + it('supersedes by symbol key', async () => { + const queue = new Queue(); + const key = Symbol('task'); - await queue.enqueue({ - name: 'task', - key: 'k', - handler: async () => 'result', + const first = queue.enqueue({ + key, + handler: async ({ signal }) => { + await new Promise((_, reject) => { + signal.addEventListener('abort', () => reject(signal.reason)); + setTimeout(() => {}, 1000); + }); + }, + }); + + await new Promise((r) => setTimeout(r, 10)); + + const second = queue.enqueue({ + key, + handler: async () => 'new', }); - expect(queue.tasks.task?.meta).toBeNull(); + await expect(first).rejects.toMatchObject({ code: 'SUPERSEDED' }); + await expect(second).resolves.toBe('new'); }); }); }); diff --git a/packages/store/src/core/tests/queue.types.test.ts b/packages/store/src/core/tests/queue.types.test.ts deleted file mode 100644 index ce2ba5b5e..000000000 --- a/packages/store/src/core/tests/queue.types.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { describe, expectTypeOf, it } from 'vitest'; -import { createQueue } from '../queue'; - -describe('queue types', () => { - describe('createQueue', () => { - it('returns Queue with tasks getter and subscribe method', () => { - const queue = createQueue(); - - expectTypeOf(queue.tasks).toBeObject(); - expectTypeOf(queue.subscribe).toBeFunction(); - expectTypeOf(queue.destroyed).toBeBoolean(); - }); - }); - - describe('Queue methods', () => { - it('reset takes optional name parameter', () => { - const queue = createQueue(); - - expectTypeOf(queue.reset).toBeFunction(); - expectTypeOf(queue.reset).returns.toBeVoid(); - }); - - it('abort takes optional name parameter', () => { - const queue = createQueue(); - - expectTypeOf(queue.abort).toBeFunction(); - expectTypeOf(queue.abort).returns.toBeVoid(); - }); - - it('tasks is reactive', () => { - const queue = createQueue(); - - expectTypeOf(queue.tasks).toBeObject(); - }); - - it('destroy returns void', () => { - const queue = createQueue(); - - expectTypeOf(queue.destroy).toBeFunction(); - expectTypeOf(queue.destroy).returns.toBeVoid(); - }); - }); - - describe('enqueue', () => { - it('returns promise', async () => { - const queue = createQueue(); - - const result = queue.enqueue({ - name: 'test', - key: 'test', - handler: async () => 42, - }); - - // Verify it's a promise by checking it has then - expectTypeOf(result.then).toBeFunction(); - }); - - it('handler receives TaskContext with signal', async () => { - const queue = createQueue(); - - await queue.enqueue({ - name: 'test', - key: 'test', - handler: async (ctx) => { - expectTypeOf(ctx.signal).toEqualTypeOf(); - return 'done'; - }, - }); - }); - }); -}); diff --git a/packages/store/src/core/tests/store.test.ts b/packages/store/src/core/tests/store.test.ts index c105e8446..e21dad8da 100644 --- a/packages/store/src/core/tests/store.test.ts +++ b/packages/store/src/core/tests/store.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { createFeature } from '../feature'; -import { createQueue } from '../queue'; import { flush } from '../state'; import { createStore } from '../store'; @@ -74,25 +73,6 @@ describe('store', () => { }); }); - it('creates store with default queue', () => { - const store = createStore({ - features: [audioFeature], - }); - - expect(store.queue).toBeDefined(); - }); - - it('accepts custom queue', () => { - const queue = createQueue(); - - const store = createStore({ - features: [audioFeature], - queue, - }); - - expect(store.queue).toBe(queue); - }); - it('calls onSetup', () => { const onSetup = vi.fn(); const store = createStore({ @@ -316,7 +296,6 @@ describe('store', () => { expect(store.destroyed).toBe(true); expect(store.target).toBeNull(); - expect(store.queue.destroyed).toBe(true); }); it('rejects requests after destroy', async () => { diff --git a/packages/store/src/core/tests/store.types.test.ts b/packages/store/src/core/tests/store.types.test.ts index a19dba905..cbe664d20 100644 --- a/packages/store/src/core/tests/store.types.test.ts +++ b/packages/store/src/core/tests/store.types.test.ts @@ -1,7 +1,6 @@ import { describe, expectTypeOf, it } from 'vitest'; import { createFeature } from '../feature'; -import type { Queue } from '../queue'; -import type { InferStoreRequests, InferStoreState, InferStoreTarget, InferStoreTasks } from '../store'; +import type { InferStoreRequests, InferStoreState, InferStoreTarget } from '../store'; import { createStore } from '../store'; interface MockTarget { @@ -74,14 +73,6 @@ describe('store types', () => { expectTypeOf(store.request.setMuted).returns.toEqualTypeOf>(); }); - it('queue has correctly typed tasks', () => { - const store = createSingleFeatureStore(); - - expectTypeOf(store.queue).toExtend>(); - expectTypeOf(store.queue.tasks).toBeObject(); - expectTypeOf(store.queue.subscribe).toBeFunction(); - }); - it('target is nullable before attach', () => { const store = createSingleFeatureStore(); @@ -121,16 +112,6 @@ describe('store types', () => { }); }); - describe('InferStoreTasks', () => { - it('extracts task types from store', () => { - const _store = createSingleFeatureStore(); - type Tasks = InferStoreTasks; - - expectTypeOf().toHaveProperty('setVolume'); - expectTypeOf().toHaveProperty('setMuted'); - }); - }); - describe('subscribe', () => { it('state returns readonly snapshot', () => { const store = createSingleFeatureStore(); @@ -146,64 +127,4 @@ describe('store types', () => { expectTypeOf(store.state.muted).toEqualTypeOf(); }); }); - - describe('store queue integration types', () => { - it('queue.tasks has keys matching request names', () => { - const store = createSingleFeatureStore(); - - expectTypeOf(store.queue.tasks).toHaveProperty('setVolume'); - expectTypeOf(store.queue.tasks).toHaveProperty('setMuted'); - }); - - it('task input type matches request parameter', () => { - const store = createSingleFeatureStore(); - const task = store.queue.tasks.setVolume; - - if (task) { - expectTypeOf(task.input).toEqualTypeOf(); - } - }); - - it('task output type matches request return on success', () => { - const store = createSingleFeatureStore(); - const task = store.queue.tasks.setVolume; - - if (task?.status === 'success') { - expectTypeOf(task.output).toEqualTypeOf(); - } - }); - - it('multi-feature store has combined queue task types', () => { - const store = createTestStore(); - - expectTypeOf(store.queue.tasks).toHaveProperty('setVolume'); - expectTypeOf(store.queue.tasks).toHaveProperty('setMuted'); - expectTypeOf(store.queue.tasks).toHaveProperty('play'); - expectTypeOf(store.queue.tasks).toHaveProperty('pause'); - }); - - it('queue.reset accepts request names', () => { - const store = createSingleFeatureStore(); - - store.queue.reset('setVolume'); - store.queue.reset('setMuted'); - store.queue.reset(); // all - }); - - it('queue.abort accepts request names', () => { - const store = createSingleFeatureStore(); - - store.queue.abort('setVolume'); - store.queue.abort('setMuted'); - store.queue.abort(); // all - }); - - it('InferStoreTasks matches queue task record keys', () => { - const _store = createSingleFeatureStore(); - type Tasks = InferStoreTasks; - - expectTypeOf().toHaveProperty('setVolume'); - expectTypeOf().toHaveProperty('setMuted'); - }); - }); }); diff --git a/packages/store/src/core/tests/task.test.ts b/packages/store/src/core/tests/task.test.ts deleted file mode 100644 index e82aae6cb..000000000 --- a/packages/store/src/core/tests/task.test.ts +++ /dev/null @@ -1,239 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import type { ErrorTask, PendingTask, SuccessTask, Task } from '../task'; - -import { isErrorTask, isPendingTask, isSettledTask, isSuccessTask } from '../task'; - -describe('task', () => { - describe('isPendingTask', () => { - it('returns true for pending task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'pending', - abort: new AbortController(), - }; - - expect(isPendingTask(task)).toBe(true); - }); - - it('returns false for success task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'success', - settledAt: Date.now(), - output: 'result', - }; - - expect(isPendingTask(task)).toBe(false); - }); - - it('returns false for error task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'error', - settledAt: Date.now(), - error: new Error('test'), - cancelled: false, - }; - - expect(isPendingTask(task)).toBe(false); - }); - - it('returns false for undefined', () => { - expect(isPendingTask(undefined)).toBe(false); - }); - }); - - describe('isSettledTask', () => { - it('returns true for success task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'success', - settledAt: Date.now(), - output: 'result', - }; - - expect(isSettledTask(task)).toBe(true); - }); - - it('returns true for error task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'error', - settledAt: Date.now(), - error: new Error('test'), - cancelled: false, - }; - - expect(isSettledTask(task)).toBe(true); - }); - - it('returns false for pending task', () => { - const task: Task = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'pending', - abort: new AbortController(), - }; - - expect(isSettledTask(task)).toBe(false); - }); - - it('returns false for undefined', () => { - expect(isSettledTask(undefined)).toBe(false); - }); - }); - - describe('isSuccessTask', () => { - it('returns true for success task', () => { - const task: SuccessTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'success', - settledAt: Date.now(), - output: 'result', - }; - - expect(isSuccessTask(task)).toBe(true); - }); - - it('returns false for error task', () => { - const task: ErrorTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'error', - settledAt: Date.now(), - error: new Error('test'), - cancelled: false, - }; - - expect(isSuccessTask(task)).toBe(false); - }); - - it('returns false for pending task', () => { - const task: PendingTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'pending', - abort: new AbortController(), - }; - - expect(isSuccessTask(task)).toBe(false); - }); - - it('returns false for undefined', () => { - expect(isSuccessTask(undefined)).toBe(false); - }); - }); - - describe('isErrorTask', () => { - it('returns true for error task', () => { - const task: ErrorTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'error', - settledAt: Date.now(), - error: new Error('test'), - cancelled: false, - }; - - expect(isErrorTask(task)).toBe(true); - }); - - it('returns true for cancelled error task', () => { - const task: ErrorTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'error', - settledAt: Date.now(), - error: new Error('aborted'), - cancelled: true, - }; - - expect(isErrorTask(task)).toBe(true); - }); - - it('returns false for success task', () => { - const task: SuccessTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'success', - settledAt: Date.now(), - output: 'result', - }; - - expect(isErrorTask(task)).toBe(false); - }); - - it('returns false for pending task', () => { - const task: PendingTask = { - id: Symbol('task'), - name: 'test', - key: 'test', - input: undefined, - startedAt: Date.now(), - meta: null, - status: 'pending', - abort: new AbortController(), - }; - - expect(isErrorTask(task)).toBe(false); - }); - - it('returns false for undefined', () => { - expect(isErrorTask(undefined)).toBe(false); - }); - }); -}); diff --git a/packages/store/src/core/tests/task.types.test.ts b/packages/store/src/core/tests/task.types.test.ts deleted file mode 100644 index 11227923c..000000000 --- a/packages/store/src/core/tests/task.types.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { describe, expectTypeOf, it } from 'vitest'; -import type { ErrorTask, PendingTask, SuccessTask, Task } from '../task'; - -import { isErrorTask, isPendingTask, isSettledTask, isSuccessTask } from '../task'; - -describe('task types', () => { - describe('Task', () => { - it('is discriminated union of task states', () => { - const task: Task = {} as Task; - - if (task.status === 'pending') { - expectTypeOf(task).toExtend(); - expectTypeOf(task.abort).toExtend(); - } - - if (task.status === 'success') { - expectTypeOf(task).toExtend(); - expectTypeOf(task.output).toBeUnknown(); - expectTypeOf(task.settledAt).toBeNumber(); - } - - if (task.status === 'error') { - expectTypeOf(task).toExtend(); - expectTypeOf(task.error).toBeUnknown(); - expectTypeOf(task.cancelled).toBeBoolean(); - expectTypeOf(task.settledAt).toBeNumber(); - } - }); - - it('has common properties across all states', () => { - const task: Task = {} as Task; - - expectTypeOf(task.id).toEqualTypeOf(); - expectTypeOf(task.name).toEqualTypeOf(); - expectTypeOf(task.key).toExtend(); - expectTypeOf(task.startedAt).toBeNumber(); - }); - }); - - describe('type guards', () => { - it('isPendingTask narrows to PendingTask', () => { - const task: Task = {} as Task; - - if (isPendingTask(task)) { - expectTypeOf(task).toExtend(); - } - }); - - it('isSettledTask narrows to SuccessTask | ErrorTask', () => { - const task: Task = {} as Task; - - if (isSettledTask(task)) { - expectTypeOf(task.settledAt).toBeNumber(); - } - }); - - it('isSuccessTask narrows to SuccessTask', () => { - const task: Task = {} as Task; - - if (isSuccessTask(task)) { - expectTypeOf(task).toExtend(); - expectTypeOf(task.output).toBeUnknown(); - } - }); - - it('isErrorTask narrows to ErrorTask', () => { - const task: Task = {} as Task; - - if (isErrorTask(task)) { - expectTypeOf(task).toExtend(); - expectTypeOf(task.error).toBeUnknown(); - } - }); - }); -}); From 5aa5d4451894cd5d3b0b123db75492a876bdba2e Mon Sep 17 00:00:00 2001 From: Rahim Date: Sat, 31 Jan 2026 18:49:50 +1100 Subject: [PATCH 2/2] refactor(store): remove platform queue bindings BREAKING CHANGE: Remove queue-related bindings from Lit and React. Lit: - Remove QueueController and QueueControllerHost - Remove QueueController from createStore result React: - Remove useQueue hook - Remove useQueue from createStore result --- packages/store/src/lit/controllers/index.ts | 1 - .../src/lit/controllers/queue-controller.ts | 49 ----------- .../tests/queue-controller.test.ts | 86 ------------------- .../src/lit/controllers/tests/types.test.ts | 15 +--- packages/store/src/lit/create-store.ts | 43 +--------- packages/store/src/lit/index.ts | 1 - .../store/src/lit/tests/create-store.test.ts | 8 -- packages/store/src/react/create-store.tsx | 26 +----- packages/store/src/react/hooks/index.ts | 1 - .../src/react/hooks/tests/use-queue.test.tsx | 30 ------- packages/store/src/react/hooks/use-queue.ts | 35 -------- packages/store/src/react/index.ts | 2 +- .../src/react/tests/create-store.test.tsx | 15 ---- 13 files changed, 4 insertions(+), 308 deletions(-) delete mode 100644 packages/store/src/lit/controllers/queue-controller.ts delete mode 100644 packages/store/src/lit/controllers/tests/queue-controller.test.ts delete mode 100644 packages/store/src/react/hooks/tests/use-queue.test.tsx delete mode 100644 packages/store/src/react/hooks/use-queue.ts diff --git a/packages/store/src/lit/controllers/index.ts b/packages/store/src/lit/controllers/index.ts index cf97390c9..419f7b135 100644 --- a/packages/store/src/lit/controllers/index.ts +++ b/packages/store/src/lit/controllers/index.ts @@ -1,6 +1,5 @@ export type { AsyncStatus } from '../../shared/types'; -export { QueueController, type QueueControllerHost } from './queue-controller'; export { SnapshotController, type SnapshotControllerHost, diff --git a/packages/store/src/lit/controllers/queue-controller.ts b/packages/store/src/lit/controllers/queue-controller.ts deleted file mode 100644 index 823082f2a..000000000 --- a/packages/store/src/lit/controllers/queue-controller.ts +++ /dev/null @@ -1,49 +0,0 @@ -import type { AnyStore } from '../../core/store'; -import type { StoreSource } from '../store-accessor'; -import type { SubscriptionControllerHost } from './subscription-controller'; -import { SubscriptionController } from './subscription-controller'; - -export type QueueControllerHost = SubscriptionControllerHost; - -/** - * Subscribes to queue task changes. - * Triggers host updates when tasks change. - * - * Accepts either a direct store instance or a context that provides one. - * - * @example Direct store - * ```ts - * class MyElement extends LitElement { - * #queue = new QueueController(this, store); - * - * render() { - * const playTask = this.#queue.value.play; - * const isPending = playTask?.status === 'pending'; - * return html``; - * } - * } - * ``` - * - * @example Context source - * ```ts - * const { context } = createStore({ features: [playbackFeature] }); - * - * class MyElement extends LitElement { - * #queue = new QueueController(this, context); - * } - * ``` - */ -export class QueueController { - readonly #sub: SubscriptionController; - - constructor(host: QueueControllerHost, source: StoreSource) { - this.#sub = new SubscriptionController(host, source, { - subscribe: (store, onChange) => store.queue.subscribe(onChange), - getValue: (store) => store.queue.tasks, - }); - } - - get value(): Store['queue']['tasks'] { - return this.#sub.value; - } -} diff --git a/packages/store/src/lit/controllers/tests/queue-controller.test.ts b/packages/store/src/lit/controllers/tests/queue-controller.test.ts deleted file mode 100644 index 4017a1d23..000000000 --- a/packages/store/src/lit/controllers/tests/queue-controller.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest'; - -import { createCoreTestStore, createTestHost } from '../../tests/test-utils'; -import { QueueController } from '../queue-controller'; - -describe('QueueController', () => { - afterEach(() => { - document.body.innerHTML = ''; - }); - - it('returns tasks record', () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - - expect(controller.value).toEqual({}); - }); - - it('updates when task completes', async () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - document.body.appendChild(host); - - expect(controller.value.setVolume).toBeUndefined(); - - await store.request.setVolume!(0.5); - - expect(controller.value.setVolume).toBeDefined(); - expect(controller.value.setVolume?.status).toBe('success'); - expect(host.updateCount).toBeGreaterThan(0); - }); - - it('unsubscribes on disconnect', async () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - document.body.appendChild(host); - host.remove(); - - const updateCountBefore = host.updateCount; - await store.request.setVolume!(0.5); - - expect(host.updateCount).toBe(updateCountBefore); - }); - - it('handles multiple task updates', async () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - document.body.appendChild(host); - - await store.request.setVolume!(0.5); - await store.request.setMuted!(true); - - expect(controller.value.setVolume).toBeDefined(); - expect(controller.value.setMuted).toBeDefined(); - expect(controller.value.setVolume?.status).toBe('success'); - expect(controller.value.setMuted?.status).toBe('success'); - }); - - it('syncs to current tasks on reconnect', async () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - document.body.appendChild(host); - - await store.request.setVolume!(0.5); - expect(controller.value.setVolume?.status).toBe('success'); - - host.remove(); - - await store.request.setMuted!(true); - - // Reconnect - document.body.appendChild(host); - - expect(controller.value.setVolume?.status).toBe('success'); - expect(controller.value.setMuted?.status).toBe('success'); - }); -}); diff --git a/packages/store/src/lit/controllers/tests/types.test.ts b/packages/store/src/lit/controllers/tests/types.test.ts index d47489301..cd1e6079f 100644 --- a/packages/store/src/lit/controllers/tests/types.test.ts +++ b/packages/store/src/lit/controllers/tests/types.test.ts @@ -1,8 +1,7 @@ import { describe, expectTypeOf, it } from 'vitest'; import { createState } from '../../../core/state'; -import { createCoreTestStore, createTestHost } from '../../tests/test-utils'; -import { QueueController } from '../queue-controller'; +import { createTestHost } from '../../tests/test-utils'; import { SnapshotController } from '../snapshot-controller'; describe('controller types', () => { @@ -30,16 +29,4 @@ describe('controller types', () => { expectTypeOf(controller.value.muted).toEqualTypeOf(); }); }); - - describe('QueueController', () => { - it('value is tasks record', () => { - const { store } = createCoreTestStore(); - const host = createTestHost(); - - const controller = new QueueController(host, store); - - // Value type matches store.queue.tasks - expectTypeOf(controller.value).toMatchTypeOf(store.queue.tasks); - }); - }); }); diff --git a/packages/store/src/lit/create-store.ts b/packages/store/src/lit/create-store.ts index 32e8cf877..c3c348ec4 100644 --- a/packages/store/src/lit/create-store.ts +++ b/packages/store/src/lit/create-store.ts @@ -3,18 +3,10 @@ import { ContextConsumer, createContext } from '@lit/context'; import type { ReactiveControllerHost, ReactiveElement } from '@lit/reactive-element'; import { noop } from '@videojs/utils/function'; import type { Constructor } from '@videojs/utils/types'; -import type { - AnyFeature, - UnionFeatureRequests, - UnionFeatureState, - UnionFeatureTarget, - UnionFeatureTasks, -} from '../core/feature'; -import type { TasksRecord } from '../core/queue'; +import type { AnyFeature, UnionFeatureRequests, UnionFeatureState, UnionFeatureTarget } from '../core/feature'; import type { StoreConfig, StoreConsumer, StoreProvider } from '../core/store'; import { Store } from '../core/store'; -import { QueueController as QueueControllerBase } from './controllers'; import { createStoreAttachMixin, createStoreMixin, createStoreProviderMixin } from './mixins'; export const contextKey = Symbol('@videojs/store'); @@ -118,32 +110,6 @@ export interface CreateStoreResult { hostConnected: () => void; hostDisconnected: () => void; }; - - /** - * Queue controller bound to this store's context. - * Subscribes to queue task changes. - * - * @example - * ```ts - * const { QueueController } = createStore({ features: [playbackFeature] }); - * - * class MyElement extends LitElement { - * #queue = new QueueController(this); - * - * render() { - * const playTask = this.#queue.value.play; - * return html``; - * } - * } - * ``` - */ - QueueController: new ( - host: CreateStoreHost - ) => { - value: Readonly>>; - hostConnected: () => void; - hostDisconnected: () => void; - }; } /** @@ -240,12 +206,6 @@ export function createStore( } } - class QueueController extends QueueControllerBase { - constructor(host: CreateStoreHost) { - super(host, context); - } - } - return { StoreMixin, StoreProviderMixin, @@ -253,6 +213,5 @@ export function createStore( context, create, StoreController, - QueueController, }; } diff --git a/packages/store/src/lit/index.ts b/packages/store/src/lit/index.ts index a5a98b99d..296e5f7f2 100644 --- a/packages/store/src/lit/index.ts +++ b/packages/store/src/lit/index.ts @@ -1,6 +1,5 @@ export type { AsyncStatus } from './controllers'; export { - QueueController, SnapshotController, StoreController, SubscriptionController, diff --git a/packages/store/src/lit/tests/create-store.test.ts b/packages/store/src/lit/tests/create-store.test.ts index 7ba944f0c..f7aaf13e2 100644 --- a/packages/store/src/lit/tests/create-store.test.ts +++ b/packages/store/src/lit/tests/create-store.test.ts @@ -113,7 +113,6 @@ describe('createStore', () => { expect(result).toHaveProperty('context'); expect(result).toHaveProperty('create'); expect(result).toHaveProperty('StoreController'); - expect(result).toHaveProperty('QueueController'); }); }); @@ -125,13 +124,6 @@ describe('createStore', () => { expect(StoreController.prototype).toBeDefined(); }); - it('QueueController is a class', () => { - const { QueueController } = createStore({ features: [audioFeature] }); - - expect(typeof QueueController).toBe('function'); - expect(QueueController.prototype).toBeDefined(); - }); - // Note: Full integration tests with DOM and context would require // setting up a provider element hierarchy. The bound controllers // work via context, which is tested in integration tests. diff --git a/packages/store/src/react/create-store.tsx b/packages/store/src/react/create-store.tsx index 5b9a74a6d..5109cf330 100644 --- a/packages/store/src/react/create-store.tsx +++ b/packages/store/src/react/create-store.tsx @@ -1,14 +1,7 @@ import { isNull, isUndefined } from '@videojs/utils/predicate'; import type { FC, ReactNode } from 'react'; import { useEffect, useMemo, useState, useSyncExternalStore } from 'react'; -import type { - AnyFeature, - UnionFeatureRequests, - UnionFeatureState, - UnionFeatureTarget, - UnionFeatureTasks, -} from '../core/feature'; -import type { TasksRecord } from '../core/queue'; +import type { AnyFeature, UnionFeatureRequests, UnionFeatureState, UnionFeatureTarget } from '../core/feature'; import type { StoreConfig } from '../core/store'; import { Store } from '../core/store'; @@ -53,12 +46,6 @@ export interface CreateStoreResult { */ useStore: () => UseStoreResult; - /** - * Subscribes to queue task changes. - * Returns the current tasks map from the queue. - */ - useQueue: () => TasksRecord>; - /** * Creates a new store instance. * Useful for imperative access or creating a store before render. @@ -87,7 +74,6 @@ export function createStore( config: CreateStoreConfig ): CreateStoreResult { type Target = UnionFeatureTarget; - type Tasks = UnionFeatureTasks; type StoreType = Store; function create(): StoreType { @@ -152,19 +138,9 @@ export function createStore( ); } - function useQueue(): TasksRecord { - const store = useStoreContext() as StoreType; - return useSyncExternalStore( - (cb) => store.queue.subscribe(cb), - () => store.queue.tasks, - () => store.queue.tasks - ); - } - return { Provider, useStore, - useQueue, create, }; } diff --git a/packages/store/src/react/hooks/index.ts b/packages/store/src/react/hooks/index.ts index 94a4b1f99..8af577785 100644 --- a/packages/store/src/react/hooks/index.ts +++ b/packages/store/src/react/hooks/index.ts @@ -1,5 +1,4 @@ export type { AsyncStatus } from '../../shared/types'; -export { useQueue } from './use-queue'; export { useSnapshot } from './use-snapshot'; export { useStore } from './use-store'; diff --git a/packages/store/src/react/hooks/tests/use-queue.test.tsx b/packages/store/src/react/hooks/tests/use-queue.test.tsx deleted file mode 100644 index 1aa992e27..000000000 --- a/packages/store/src/react/hooks/tests/use-queue.test.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import { act, renderHook } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; - -import { useQueue } from '../use-queue'; -import { createTestStore } from './test-utils'; - -describe('useQueue', () => { - it('returns tasks record', () => { - const { store } = createTestStore(); - - const { result } = renderHook(() => useQueue(store)); - - expect(result.current).toEqual({}); - }); - - it('updates when task completes', async () => { - const { store } = createTestStore(); - - const { result } = renderHook(() => useQueue(store)); - - expect(result.current.setVolume).toBeUndefined(); - - await act(async () => { - await store.request.setVolume(0.5); - }); - - expect(result.current.setVolume).toBeDefined(); - expect(result.current.setVolume?.status).toBe('success'); - }); -}); diff --git a/packages/store/src/react/hooks/use-queue.ts b/packages/store/src/react/hooks/use-queue.ts deleted file mode 100644 index 59ecd972a..000000000 --- a/packages/store/src/react/hooks/use-queue.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { useSyncExternalStore } from 'react'; -import type { TasksRecord } from '../../core/queue'; -import type { AnyStore, InferStoreTasks } from '../../core/store'; - -/** - * Subscribe to queue task changes. - * - * Returns a record of all tasks keyed by request name. - * Re-renders when any task is added, updated, or removed. - * - * @example - * ```tsx - * function TaskStatus() { - * const tasks = useQueue(store); - * const playTask = tasks.play; - * - * if (playTask?.status === 'pending') { - * return Loading...; - * } - * - * return Ready; - * } - * ``` - */ -export function useQueue(store: Store): TasksRecord> { - return useSyncExternalStore( - (cb) => store.queue.subscribe(cb), - () => store.queue.tasks as TasksRecord>, - () => store.queue.tasks as TasksRecord> - ); -} - -export namespace useQueue { - export type Result = TasksRecord>; -} diff --git a/packages/store/src/react/index.ts b/packages/store/src/react/index.ts index 505e6cec0..6a29d3ff7 100644 --- a/packages/store/src/react/index.ts +++ b/packages/store/src/react/index.ts @@ -6,4 +6,4 @@ export type { } from './create-store'; export { createStore } from './create-store'; -export { useQueue, useSnapshot, useStore } from './hooks'; +export { useSnapshot, useStore } from './hooks'; diff --git a/packages/store/src/react/tests/create-store.test.tsx b/packages/store/src/react/tests/create-store.test.tsx index f27b19833..37e6d0f6c 100644 --- a/packages/store/src/react/tests/create-store.test.tsx +++ b/packages/store/src/react/tests/create-store.test.tsx @@ -124,19 +124,4 @@ describe('createStore', () => { expect(result.current.volume).toBe(0.5); }); }); - - describe('useQueue', () => { - it('returns tasks from context store', () => { - const { Provider, useQueue, create } = createStore({ - features: [audioFeature], - }); - const store = create(); - - const { result } = renderHook(() => useQueue(), { - wrapper: ({ children }: { children: ReactNode }) => {children}, - }); - - expect(result.current).toEqual({}); - }); - }); });