diff --git a/.claude/plans/store-factory-hooks.md b/.claude/plans/store-factory-hooks.md new file mode 100644 index 00000000..cca0afe3 --- /dev/null +++ b/.claude/plans/store-factory-hooks.md @@ -0,0 +1,827 @@ +# Store React/DOM Bindings + +## Goal + +Implement React and DOM bindings for Video.js 10's store, enabling: + +- Simple `createStore()` API that returns Provider + hooks/controllers +- Skins define their own store configs and export Provider + Skin + hooks +- Consumers can extend skin configs with additional slices +- Base hooks/controllers for testing and advanced use cases + +## Key Decisions + +| Decision | Resolution | +| ---------------------- | ----------------------------------------------------------------------- | +| Store creation | `createStore({ slices, displayName? })` - types inferred from slices | +| Hook naming | `useStore`, `useSelector`, `useRequest`, `usePending`, `useSlice` | +| Controller naming | `StoreController`, `SelectorController`, `RequestController`, etc | +| Selector hook | `useSelector(selector)` - requires selector (Redux-style) | +| Store hook | `useStore()` - returns store instance | +| Pending hook | `usePending()` - returns `store.queue.pending` (reactive) | +| Slice hook return | `{ state, request, isAvailable }` - state/request null when unavailable | +| Skin exports | `Provider`, `Skin`, hooks/controllers | +| Slice namespace | `export * as media` → `media.playbackSlice` | +| Video component | Generic, exported from `@videojs/react` (not from skins) | +| DOM provider | `withStoreProvider` mixin | +| Base hooks/controllers | Take store explicitly, for testing/advanced use | +| displayName | For React DevTools component naming | + +--- + +## Phase 1: React Bindings (`@videojs/store/react`) + +### 1.1 `createStore` + +**File:** `packages/store/src/react/create-store.ts` + +```typescript +import type { AnySlice, StoreConfig } from '../core'; +import type { ReactNode } from 'react'; + +export interface CreateStoreConfig { + slices: Slices; + displayName?: string; +} + +export interface CreateStoreResult { + Provider: FC<{ children: ReactNode }>; + useStore: () => Store, Slices>; + useSelector: (selector: (state: UnionSliceState) => T) => T; + useRequest: () => UnionSliceRequests; + usePending: () => PendingRecord>; + useSlice: (slice: S) => SliceResult; +} + +export function createStore(config: CreateStoreConfig): CreateStoreResult; +``` + +### 1.2 Types + +**File:** `packages/store/src/react/types.ts` + +```typescript +export type SliceResult = + | { state: InferSliceState; request: InferSliceRequests; isAvailable: true } + | { state: null; request: null; isAvailable: false }; +``` + +### 1.3 Base hooks + +**File:** `packages/store/src/react/hooks.ts` + +```typescript +// Base hooks - take store explicitly (for testing/advanced use) +export function useSelector(store: S, selector: (state: InferStoreState) => T): T; + +export function useRequest(store: S): InferStoreRequests; + +export function usePending(store: S): PendingRecord>; + +export function useSlice(store: S, slice: Slice): SliceResult; +``` + +**Implementation details:** + +- `Provider`: Creates store via `useState(() => new Store(config))`, provides via context +- `useStore()`: Returns store instance from context +- `useSelector(selector)`: Uses `useSyncExternalStore` with selector +- `useRequest()`: Returns stable `store.request` from context +- `usePending()`: Subscribes to `store.queue`, returns `queue.pending` +- `useSlice(slice)`: Returns `{ state, request, isAvailable }` with null narrowing +- Cleanup: `useEffect` calls `store.destroy()` on unmount + +### 1.4 Exports + +**File:** `packages/store/src/react/index.ts` + +```typescript +export { createStore } from './create-store'; +// Base hooks for testing/advanced use +export { usePending, useRequest, useSelector, useSlice } from './hooks'; + +export type { CreateStoreConfig, CreateStoreResult, SliceResult } from './types'; +``` + +--- + +## Phase 2: DOM Bindings (`@videojs/store/dom`) + +### 2.0 @lit/context Research + +Key findings from analyzing `@lit/context@1.1.6`: + +**Dynamic Value Updates:** + +- `ContextProvider.setValue(newValue, force?)` notifies all subscribed consumers +- Uses `Object.is()` for equality - swapping store objects triggers updates automatically +- `force = true` needed only for in-place mutations (same reference) + +**Subscription Model:** + +- Consumers must opt-in: `subscribe: true` in `@consume()` or `ContextConsumer` +- Without subscription, consumers only receive initial value +- Provider stores callbacks in `Map` +- `updateObservers()` iterates all callbacks on value change + +**Store Swapping Pattern (validated):** + +```typescript +class MySkin extends HTMLElement { + #provider = new ContextProvider(this, { context: storeContext }); + + set store(newStore: Store) { + this.#provider.setValue(newStore); // Notifies all subscribers + } +} +``` + +### 2.1 `createStore` + +**File:** `packages/store/src/dom/create-store.ts` + +```typescript +import type { Context } from '@lit/context'; +import type { ReactiveControllerHost } from '@lit/reactive-element'; + +import { createContext } from '@lit/context'; + +export interface CreateStoreConfig { + slices: Slices; +} + +export interface CreateStoreResult { + defineStoreProvider: (tagName: string) => void; + withStoreProvider: >(Base: T) => T; + StoreController: new (host: ReactiveControllerHost) => StoreControllerInstance; + SelectorController: ( + host: ReactiveControllerHost, + selector: (state: UnionSliceState) => T + ) => SelectorControllerInstance; + RequestController: new (host: ReactiveControllerHost) => RequestControllerInstance; + PendingController: new (host: ReactiveControllerHost) => PendingControllerInstance; + SliceController: (host: ReactiveControllerHost, slice: S) => SliceControllerInstance; + context: Context, Slices>>; +} + +export function createStore(config: CreateStoreConfig): CreateStoreResult; +``` + +### 2.2 Base controllers + +**File:** `packages/store/src/dom/controllers.ts` + +```typescript +import type { ReactiveController, ReactiveControllerHost } from '@lit/reactive-element'; + +// Base controllers - take store explicitly (for testing/advanced use) +export class SelectorController implements ReactiveController { + constructor(host: ReactiveControllerHost, store: S, selector: (state: InferStoreState) => T); + get value(): T; +} + +export class RequestController implements ReactiveController { + constructor(host: ReactiveControllerHost, store: S); + get value(): InferStoreRequests; +} + +export class PendingController implements ReactiveController { + constructor(host: ReactiveControllerHost, store: S); + get value(): PendingRecord>; +} + +export class SliceController implements ReactiveController { + constructor(host: ReactiveControllerHost, store: S, slice: Slice); + get state(): InferSliceState | null; + get request(): InferSliceRequests | null; + get isAvailable(): this is this & { state: InferSliceState; request: InferSliceRequests }; +} +``` + +**Implementation details:** + +- Uses `@lit/context` for W3C Context Protocol +- Context key auto-generated per `createStore()` call (unique Symbol) +- `withStoreProvider`: Mixin that: + - Creates store instance + - Uses `ContextProvider` internally + - Exposes `store` setter that calls `provider.setValue(newStore)` +- Controllers: + - Consume store via `ContextConsumer` with `subscribe: true` + - Subscribe to store changes, call `host.requestUpdate()` on change + - Cleanup on `hostDisconnected` (automatic via ContextConsumer) +- `SelectorController`: One-step instantiation `new SelectorController(this, selector)` + +### 2.3 Exports + +**File:** `packages/store/src/dom/index.ts` + +```typescript +// Base controllers for testing/advanced use +export { PendingController, RequestController, SelectorController, SliceController } from './controllers'; +export { createStore } from './create-store'; + +export type { CreateStoreConfig, CreateStoreResult } from './types'; +``` + +--- + +## Phase 3: Playback Slice (`@videojs/core/dom`) + +Based on [Issue #239](https://github.com/videojs/v10/issues/239). + +### 3.1 Playback slice + +**File:** `packages/core/src/dom/slices/playback.ts` + +```typescript +interface PlaybackState { + paused: boolean; + ended: boolean; + started: boolean; + waiting: boolean; + currentTime: number; + duration: number; + buffered: Array<[number, number]>; + seekable: Array<[number, number]>; + volume: number; + muted: boolean; + canPlay: boolean; + source: unknown; + streamType: 'on-demand' | 'live' | 'live-dvr' | 'unknown'; +} + +interface PlaybackRequests { + play: Request; + pause: Request; + seek: Request; + changeVolume: Request; + toggleMute: Request; + changeSource: Request; +} + +export const playbackSlice = createSlice()({ + initialState: { + /* ... */ + }, + getSnapshot: ({ target }) => ({ + /* ... */ + }), + subscribe: ({ target, update, signal }) => { + /* ... */ + }, + request: { + /* ... */ + }, +}); +``` + +### 3.2 Namespace export + +**File:** `packages/core/src/dom/slices/index.ts` + +```typescript +export { playbackSlice } from './playback'; + +// Namespace export +export * as media from './index'; +``` + +Usage: + +```typescript +import { media, playbackSlice } from '@videojs/core/dom'; + +media.playbackSlice; // via namespace +playbackSlice; // standalone export +``` + +### 3.3 Utilities + +**File:** `packages/core/src/dom/slices/utils.ts` + +```typescript +export function serializeTimeRanges(ranges: TimeRanges): Array<[number, number]>; +``` + +### 3.4 Type guards + +**File:** `packages/core/src/dom/guards.ts` + +```typescript +export function isHTMLVideo(target: unknown): target is HTMLVideoElement; +export function isHTMLAudio(target: unknown): target is HTMLAudioElement; +export function isHTMLMedia(target: unknown): target is HTMLMediaElement; +``` + +--- + +## Phase 4: React Package Setup (`@videojs/react`) + +### 4.1 Video component + +**File:** `packages/react/src/media/Video.tsx` + +```typescript +import type { VideoHTMLAttributes, RefCallback } from 'react'; +import { useCallback } from 'react'; +import { useStore } from '../store'; +import { useComposedRefs } from '../utils/use-composed-refs'; + +export interface VideoProps extends VideoHTMLAttributes { + ref?: RefCallback | React.RefObject; +} + +/** + * Video element that automatically attaches to the store. + * Uses React 19 ref cleanup pattern. + */ +export function Video({ children, ref, ...props }: VideoProps): JSX.Element { + const store = useStore(); + + const attachRef: RefCallback = useCallback((el) => { + if (el) { + const detach = store.attach(el); + // React 19: return cleanup function + return detach; + } + }, [store]); + + const composedRef = useComposedRefs(ref, attachRef); + + return ( + + ); +} +``` + +### 4.2 Package exports + +**File:** `packages/react/src/index.ts` + +```typescript +// Media elements +export { Video } from './media/Video'; +export type { VideoProps } from './media/Video'; + +export { media } from '@videojs/core/dom'; +// Re-export for extension +export { createStore } from '@videojs/store/react'; +``` + +--- + +## Phase 5: Frosted Skin (React) + +### 5.1 Store config + +**File:** `packages/react/src/skins/frosted/store.ts` + +```typescript +import { media } from '@videojs/core/dom'; +import { createStore } from '@videojs/store/react'; + +export const storeConfig = { + slices: [media.playback] as const, + displayName: 'FrostedSkin', +}; + +export const { Provider, useStore, useSelector, useRequest, usePending, useSlice } = createStore(storeConfig); +``` + +### 5.2 Skin component + +**File:** `packages/react/src/skins/frosted/Skin.tsx` + +```typescript +import type { PropsWithChildren } from 'react'; + +import { useRequest, useSelector } from './store'; + +export interface SkinProps extends PropsWithChildren<{ + className?: string; + theme?: 'light' | 'dark'; +}> {} + +export function Skin({ children, className, theme }: SkinProps): JSX.Element { + return ( +
+ {children} + +
+ ); +} + +function Controls() { + const paused = useSelector((s) => s.paused); + const { play, pause } = useRequest(); + // ... render controls +} +``` + +### 5.3 Exports + +**File:** `packages/react/src/skins/frosted/index.ts` + +```typescript +export { Skin } from './Skin'; +export type { SkinProps } from './Skin'; +export { Provider, storeConfig, usePending, useRequest, useSelector, useSlice, useStore } from './store'; +``` + +--- + +## Phase 6: Frosted Skin (HTML) + +### 6.1 Store config + +**File:** `packages/html/src/skins/frosted/store.ts` + +```typescript +import { media } from '@videojs/core/dom'; +import { createStore } from '@videojs/store/dom'; + +export const storeConfig = { + slices: [media.playback] as const, +}; + +export const { + defineStoreProvider, + withStoreProvider, + StoreController, + SelectorController, + RequestController, + PendingController, + SliceController, + context, +} = createStore(storeConfig); +``` + +### 6.2 Skin component + +**File:** `packages/html/src/skins/frosted/Skin.ts` + +```typescript +import { RequestController, SelectorController, withStoreProvider } from './store'; + +export class FrostedSkinElement extends HTMLElement { + #paused = new SelectorController(this, (s) => s.paused); + #request = new RequestController(this); + + /** + * Define a custom element with this skin, optionally with a custom provider mixin. + * Useful for extending the skin with additional slices. + */ + static define(tagName: string, providerMixin = withStoreProvider) { + const SkinWithProvider = providerMixin(this); + customElements.define(tagName, SkinWithProvider); + } + + connectedCallback() { + this.innerHTML = ` + +
+ +
+ `; + this.#updatePlayButton(); + this.querySelector('button')?.addEventListener('click', this.#handleClick); + } + + #handleClick = () => { + this.#paused.value ? this.#request.value.play() : this.#request.value.pause(); + }; + + #updatePlayButton() { + const btn = this.querySelector('.play-pause'); + if (btn) btn.textContent = this.#paused.value ? 'Play' : 'Pause'; + } +} + + #handleClick = () => { + this.#paused.value ? this.#request.value.play() : this.#request.value.pause(); + }; + + #updatePlayButton() { + const btn = this.querySelector('.play-pause'); + if (btn) btn.textContent = this.#paused.value ? 'Play' : 'Pause'; + } +} +``` + +### 6.3 Define export + +**File:** `packages/html/src/define/vjs-frosted-skin.ts` + +```typescript +import { FrostedSkinElement } from '../skins/frosted/Skin'; + +customElements.define('vjs-frosted-skin', FrostedSkinElement); +``` + +### 6.4 Exports + +**File:** `packages/html/src/skins/frosted/index.ts` + +```typescript +export { FrostedSkinElement } from './Skin'; +export { + context, + PendingController, + RequestController, + SelectorController, + SliceController, + storeConfig, + StoreController, + withStoreProvider, +} from './store'; +``` + +--- + +## Usage Examples + +### React: Custom UI + +```tsx +import { createStore, media, Video } from '@videojs/react'; + +const { Provider, useSelector, useRequest } = createStore({ + slices: [media.playbackSlice], +}); + +function App() { + return ( + + + ); +} + +function MyCustomControls() { + const currentTime = useSelector((s) => s.currentTime); + const { seek } = useRequest(); + return ; +} +``` + +### React: Frosted skin + +```tsx +import { Video } from '@videojs/react'; +import { Provider, Skin } from '@videojs/react/skins/frosted'; + +function App() { + return ( + + + + + ); +} +``` + +### React: Extending frosted with custom slices + +```tsx +import { createStore, Video } from '@videojs/react'; +import { Skin, storeConfig } from '@videojs/react/skins/frosted'; + +import { chaptersSlice } from './slices/chapters'; + +// Extend frosted config with custom slice +const { Provider, useSlice } = createStore({ + ...storeConfig, + slices: [...storeConfig.slices, chaptersSlice], +}); + +function App() { + return ( + + + + + + ); +} + +function ChaptersPanel() { + const chapters = useSlice(chaptersSlice); + if (!chapters.isAvailable) return null; + return
{chapters.state.markers.map(...)}
; +} +``` + +### HTML: Frosted skin (CDN) + +```html + + + + + +``` + +### HTML: Custom provider element (CDN) + +```html + + + + + + +``` + +Where `my-player.js` contains: + +```typescript +import { createStore, playbackSlice } from '@videojs/html'; + +const { defineStoreProvider } = createStore({ + slices: [playbackSlice], +}); + +defineStoreProvider('my-player'); +``` + +### HTML: Extending frosted with custom slices + +```html + + + + + +``` + +Where `my-extended-skin.js` contains: + +```typescript +import { FrostedSkinElement, storeConfig } from '@videojs/html/skins/frosted'; +import { createStore } from '@videojs/store/dom'; + +import { chaptersSlice } from './slices/chapters.js'; + +// Extend frosted config with custom slice +const { withStoreProvider } = createStore({ + ...storeConfig, + slices: [...storeConfig.slices, chaptersSlice], +}); + +FrostedSkinElement.define('my-extended-skin', withStoreProvider); +``` + +--- + +## File Structure + +``` +packages/store/src/ +├── core/ +│ ├── store.ts # existing +│ ├── slice.ts # existing +│ ├── queue.ts # existing +│ └── index.ts +├── react/ +│ ├── create-store.ts # NEW +│ ├── hooks.ts # NEW (base hooks) +│ ├── types.ts # NEW +│ └── index.ts +└── dom/ + ├── create-store.ts # NEW + ├── controllers.ts # NEW (base controllers) + ├── types.ts # NEW + └── index.ts + +packages/core/src/dom/ +├── slices/ +│ ├── playback.ts # NEW +│ ├── utils.ts # NEW +│ └── index.ts # NEW (+ media namespace) +├── guards.ts # NEW +└── index.ts + +packages/react/src/ +├── media/ +│ └── Video.tsx # NEW +├── skins/ +│ ├── frosted/ +│ │ ├── store.ts # NEW +│ │ ├── Skin.tsx # NEW +│ │ └── index.ts # NEW +│ └── minimal/ +│ └── ... +└── index.ts + +packages/html/src/ +├── define/ +│ └── vjs-frosted-skin.ts # NEW +├── skins/ +│ ├── frosted/ +│ │ ├── store.ts # NEW +│ │ ├── Skin.ts # NEW +│ │ ├── styles.css # NEW +│ │ └── index.ts # NEW +│ └── minimal/ +│ └── ... +└── index.ts +``` + +--- + +## Implementation Order + +1. **Phase 1**: React bindings (`@videojs/store/react`) + - `createStore()`, base hooks, types +2. **Phase 2**: DOM bindings (`@videojs/store/dom`) + - `createStore()`, `defineStoreProvider()`, base controllers, types, `@lit/context` integration +3. **Phase 3**: Playback slice (`@videojs/core/dom`) + - `media.playback`, utils, guards +4. **Phase 4**: React package setup (`@videojs/react`) + - `Video`, package exports +5. **Phase 5**: Frosted skin React (`@videojs/react/skins/frosted`) +6. **Phase 6**: Frosted skin HTML (`@videojs/html/skins/frosted`) + +Each phase includes tests. + +--- + +## PR Coordination + +### Related Issues + +| Issue | Title | Description | +| ----- | ------------------------ | ----------------------------- | +| #218 | Store | Parent tracking issue | +| #229 | React Bindings | `createStore`, hooks, context | +| #230 | ReactiveElement Bindings | Controllers, context | +| #231 | Frosted Skin | Frosted skin implementation | +| #239 | Playback Feature | Consolidated playback slice | + +### Existing PR + +- **PR #281** (`videojs-store-lit`) - Draft with WIP Lit controllers + - Has: `StoreController`, `SliceController`, `PendingController` + - Missing: `SelectorController`, `RequestController`, context-based factory, `withStoreProvider` mixin + - Action: Update with new API design + +### PR Strategy + +``` +PR 1: Store React Bindings +├── createStore (store/react) +├── Base hooks (store/react) +├── Types +├── References #218 +└── Closes #229 + +PR 2: Store DOM Bindings (update PR #281) +├── createStore (store/dom) +├── defineStoreProvider +├── Base controllers (store/dom) +├── withStoreProvider mixin +├── @lit/context integration +├── References #218 +└── Closes #230 + +PR 3: Playback Slice +├── media.playbackSlice (core/dom/slices) +├── Utils (serializeTimeRanges) +├── Type guards +├── References #218 +└── Closes #239 + +PR 4: React Package Setup +├── Video component +├── Package exports +└── References #218 + +PR 5: Frosted Skin +├── React skin (Provider, Skin, storeConfig, hooks) +├── HTML skin (FrostedSkinElement.define, storeConfig, controllers) +├── define/vjs-frosted-skin.ts +├── References #218 +└── Closes #231 +``` + +### Dependency Graph + +``` +PR 1 ──┬──> PR 2 (can parallel) + └──> PR 3 (can parallel) + └──> PR 4 (needs PR 1, PR 3) + └──> PR 5 (needs PR 2, PR 4) +``` + +--- + +## Deferred + +- Testing utilities (`@videojs/store/testing`) - separate plan +- Minimal skin implementation diff --git a/packages/store/README.md b/packages/store/README.md index 72e44c97..33edc112 100644 --- a/packages/store/README.md +++ b/packages/store/README.md @@ -416,7 +416,18 @@ request: { ### Guards -Guards gate request execution. A guard returns truthy to proceed, falsy to cancel. +Guards gate request execution. A guard returns a `GuardResult`: + +```ts +import type { Guard, GuardResult } from '@videojs/store'; + +// GuardResult = boolean | Promise +// - Truthy → proceed +// - Falsy → cancel (throws REJECTED) +// - Promise resolves truthy → proceed +// - Promise resolves falsy → cancel +// - Promise rejects → cancel +``` ```ts import { timeout } from '@videojs/store'; @@ -464,9 +475,25 @@ const timedPlay = timeout(canMediaPlay, 5000); ## Error Handling +All store errors include a `code` for programmatic handling: + +| Code | Description | +| ------------ | ---------------------------- | +| `ABORTED` | Request aborted via signal | +| `CANCELLED` | Cancelled by another request | +| `DESTROYED` | Store or queue destroyed | +| `DETACHED` | Target detached | +| `NO_TARGET` | No target attached | +| `REJECTED` | Guard returned falsy | +| `REMOVED` | Task dequeued or cleared | +| `SUPERSEDED` | Replaced by same-key request | +| `TIMEOUT` | Guard timed out | + Catch errors locally via the promise, or globally via `onError`: ```ts +import { isStoreError } from '@videojs/store'; + // 1. Global Error Handling const store = createStore({ slices: [playbackSlice], @@ -483,7 +510,21 @@ const store = createStore({ try { await store.request.play(); } catch (error) { - // ... + if (isStoreError(error)) { + switch (error.code) { + case 'SUPERSEDED': + // Another play/pause request took over - expected + break; + case 'REJECTED': + // Guard failed - blocked + break; + case 'TIMEOUT': + // Guard timed out waiting + break; + default: + console.error(`[${error.code}]`, error.message); + } + } } ``` diff --git a/packages/store/src/core/errors.ts b/packages/store/src/core/errors.ts index 5af6f2f0..9e9cff79 100644 --- a/packages/store/src/core/errors.ts +++ b/packages/store/src/core/errors.ts @@ -1,13 +1,53 @@ -interface ErrorOptions { +/** + * Error codes for store operations. + * + * @example + * ```ts + * if (isStoreError(error)) { + * switch (error.code) { + * case 'SUPERSEDED': + * // Request was replaced by another - expected behavior + * break; + * case 'REJECTED': + * // Guard condition failed + * break; + * } + * } + * ``` + */ +export type StoreErrorCode + /** Request was aborted via AbortSignal - user or system requested cancellation. */ + = | 'ABORTED' + /** Request was cancelled by another request's `cancel` configuration. */ + | 'CANCELLED' + /** Store or queue was destroyed - lifecycle ended. */ + | 'DESTROYED' + /** Target was detached while request was in flight. */ + | 'DETACHED' + /** No target is attached to the store. */ + | 'NO_TARGET' + /** Guard condition returned falsy - request preconditions not met. */ + | 'REJECTED' + /** Task was removed from queue via `dequeue()` or `clear()`. */ + | 'REMOVED' + /** Request was replaced by a newer request with the same key. */ + | 'SUPERSEDED' + /** Guard condition timed out waiting for a truthy result. */ + | 'TIMEOUT'; + +export interface StoreErrorOptions { cause?: unknown; + message?: string; } export class StoreError extends Error { + readonly code: StoreErrorCode; cause?: unknown; - constructor(message: string, options?: ErrorOptions) { - super(message); + constructor(code: StoreErrorCode, options?: StoreErrorOptions) { + super(options?.message ?? code); this.name = 'StoreError'; + this.code = code; this.cause = options?.cause; } } diff --git a/packages/store/src/core/guard.ts b/packages/store/src/core/guard.ts index e32f1cb1..31ac98ca 100644 --- a/packages/store/src/core/guard.ts +++ b/packages/store/src/core/guard.ts @@ -3,7 +3,7 @@ import { isBoolean } from '@videojs/utils/predicate'; import { StoreError } from './errors'; /** - * A guard gates request execution. + * Result of a guard check. * * - Truthy → proceed * - Falsy → cancel @@ -11,7 +11,20 @@ import { StoreError } from './errors'; * - Promise resolves falsy → cancel * - Promise rejects → cancel */ -export type Guard = (ctx: { target: Target; signal: AbortSignal }) => boolean | Promise; +export type GuardResult = boolean | Promise; + +/** + * Context passed to guard functions. + */ +export interface GuardContext { + target: Target; + signal: AbortSignal; +} + +/** + * A guard gates request execution. + */ +export type Guard = (ctx: GuardContext) => GuardResult; /** * Combine guards: All must pass (truthy). @@ -69,7 +82,7 @@ export function timeout(guard: Guard, ms: number, name = 'guard' return Promise.race([ result, new Promise((_, reject) => { - const timer = setTimeout(() => reject(new StoreError(`Timeout: ${name}`)), ms); + const timer = setTimeout(() => reject(new StoreError('TIMEOUT', { message: `Timeout: ${name}` })), ms); ctx.signal.addEventListener('abort', () => clearTimeout(timer)); }), ]); diff --git a/packages/store/src/core/queue.ts b/packages/store/src/core/queue.ts index 266698a6..a1c9bcb8 100644 --- a/packages/store/src/core/queue.ts +++ b/packages/store/src/core/queue.ts @@ -252,17 +252,17 @@ export class Queue { const { name, key, input, schedule, meta = null, handler } = task; if (this.#destroyed) { - return Promise.reject(new StoreError('Queue destroyed')); + return Promise.reject(new StoreError('DESTROYED')); } // Cancel any queued task with same key const queued = this.#queued[key]; queued?.invalidate?.(); - queued?.reject(new StoreError('Superseded')); + queued?.reject(new StoreError('SUPERSEDED')); delete this.#queued[key]; // Abort any pending task with same key - this.#pending[key]?.abort.abort(new StoreError('Superseded')); + this.#pending[key]?.abort.abort(new StoreError('SUPERSEDED')); return new Promise((resolve, reject) => { const task: QueuedTask = { @@ -312,7 +312,7 @@ export class Queue { if (!queued) return false; queued.invalidate?.(); - queued.reject(new StoreError('Dequeued')); + queued.reject(new StoreError('REMOVED')); delete this.#queued[key]; return true; @@ -321,7 +321,7 @@ export class Queue { clear(): void { for (const queued of Object.values(this.#queued)) { queued.invalidate?.(); - queued.reject(new StoreError('Cleared')); + queued.reject(new StoreError('REMOVED')); } this.#queued = {}; @@ -338,19 +338,19 @@ export class Queue { await Promise.allSettled(keys.map(k => this.#flushKey(k))); } - abort(key: K, reason = 'Aborted'): void { + abort(key: K): void { // Reject queued const queued = this.#queued[key]; queued?.invalidate?.(); - queued?.reject(new StoreError(reason)); + queued?.reject(new StoreError('ABORTED')); delete this.#queued[key]; - // Abort pending with reason - this.#pending[key]?.abort.abort(new StoreError(reason)); + // Abort pending + this.#pending[key]?.abort.abort(new StoreError('ABORTED')); } - abortAll(reason = 'All requests aborted'): void { - const error = new StoreError(reason); + abortAll(): void { + const error = new StoreError('ABORTED'); // Reject all queued for (const queued of Object.values(this.#queued)) { @@ -360,7 +360,7 @@ export class Queue { this.#queued = {}; - // Abort all pending with reason + // Abort all pending for (const pending of Object.values(this.#pending)) { pending.abort.abort(error); } @@ -370,7 +370,7 @@ export class Queue { if (this.#destroyed) return; this.#destroyed = true; - this.abortAll('Queue destroyed'); + this.abortAll(); this.#subscribers.clear(); } @@ -409,13 +409,13 @@ export class Queue { try { if (abort.signal.aborted) { - throw abort.signal.reason || new StoreError('Aborted'); + throw abort.signal.reason || new StoreError('ABORTED'); } const result = await handler({ input, signal: abort.signal }); if (abort.signal.aborted) { - throw abort.signal.reason || new StoreError('Aborted'); + throw abort.signal.reason || new StoreError('ABORTED'); } resolve(result); diff --git a/packages/store/src/core/store.ts b/packages/store/src/core/store.ts index 52535d6b..a0a7ea97 100644 --- a/packages/store/src/core/store.ts +++ b/packages/store/src/core/store.ts @@ -81,7 +81,7 @@ export class Store[] = AnySlice[ attach(newTarget: Target): () => void { if (this.#destroyed) { - throw new StoreError('Store destroyed'); + throw new StoreError('DESTROYED'); } this.#attachAbort?.abort(); @@ -138,7 +138,7 @@ export class Store[] = AnySlice[ this.#attachAbort?.abort(); this.#attachAbort = null; this.#target = null; - this.#queue.abortAll('Target detached'); + this.#queue.abortAll(); this.#resetState(); } @@ -264,7 +264,7 @@ export class Store[] = AnySlice[ for (const [name, config] of this.#requestConfigs) { proxy[name] = (input?: unknown, meta?: RequestMetaInit) => { if (this.#destroyed) { - return Promise.reject(new StoreError('Store destroyed')); + return Promise.reject(new StoreError('DESTROYED')); } return this.#execute(name, config, input, meta ? createRequestMeta(meta) : null); @@ -283,25 +283,25 @@ export class Store[] = AnySlice[ const key = resolveRequestKey(config.key, input); for (const cancelKey of resolveRequestCancelKeys(config.cancel, input)) { - this.#queue.abort(cancelKey, `Cancelled by ${name}`); + this.#queue.abort(cancelKey); } const handler = async ({ input, signal }: TaskContext) => { const target = this.#target; if (!target) { - throw new StoreError('No target attached'); + throw new StoreError('NO_TARGET'); } for (const guard of config.guard) { if (signal.aborted) { - throw new StoreError('Aborted'); + throw new StoreError('ABORTED'); } const result = await guard({ target, signal }); if (!result) { - throw new StoreError('Rejected'); + throw new StoreError('REJECTED'); } } diff --git a/packages/store/src/core/tests/errors.test.ts b/packages/store/src/core/tests/errors.test.ts index 02907179..89a84019 100644 --- a/packages/store/src/core/tests/errors.test.ts +++ b/packages/store/src/core/tests/errors.test.ts @@ -4,24 +4,40 @@ import { isStoreError, StoreError } from '../errors'; describe('errors', () => { describe('storeError', () => { - it('creates error with message', () => { - const error = new StoreError('test message'); - expect(error.message).toBe('test message'); + it('creates error with code only', () => { + const error = new StoreError('ABORTED'); + expect(error.code).toBe('ABORTED'); + expect(error.message).toBe('ABORTED'); expect(error.name).toBe('StoreError'); expect(error).toBeInstanceOf(Error); }); + it('creates error with code and message', () => { + const error = new StoreError('TIMEOUT', { message: 'Timeout: canPlay' }); + expect(error.code).toBe('TIMEOUT'); + expect(error.message).toBe('Timeout: canPlay'); + }); + it('supports cause for error chaining', () => { const cause = new Error('original error'); - const error = new StoreError('wrapped error', { cause }); - expect(error.message).toBe('wrapped error'); + const error = new StoreError('ABORTED', { cause }); + expect(error.code).toBe('ABORTED'); + expect(error.cause).toBe(cause); + }); + + it('supports both message and cause', () => { + const cause = new Error('original'); + const error = new StoreError('TIMEOUT', { message: 'Timeout: guard', cause }); + expect(error.code).toBe('TIMEOUT'); + expect(error.message).toBe('Timeout: guard'); expect(error.cause).toBe(cause); }); }); describe('type guard', () => { it('isStoreError identifies store errors', () => { - expect(isStoreError(new StoreError('test'))).toBe(true); + expect(isStoreError(new StoreError('ABORTED'))).toBe(true); + expect(isStoreError(new StoreError('REJECTED'))).toBe(true); expect(isStoreError(new Error('regular'))).toBe(false); expect(isStoreError(null)).toBe(false); }); diff --git a/packages/store/src/core/tests/guard.test.ts b/packages/store/src/core/tests/guard.test.ts index 24fa2639..e1ce330d 100644 --- a/packages/store/src/core/tests/guard.test.ts +++ b/packages/store/src/core/tests/guard.test.ts @@ -111,7 +111,7 @@ describe('guard', () => { expect(await guard(createContext())).toBe(true); }); - it('throws StoreError on timeout', async () => { + it('throws StoreError with TIMEOUT code', async () => { vi.useFakeTimers(); const guard = timeout( @@ -125,6 +125,7 @@ describe('guard', () => { await expect(promise).rejects.toThrow(StoreError); await expect(promise).rejects.toMatchObject({ + code: 'TIMEOUT', message: 'Timeout: waitForReady', }); diff --git a/packages/store/src/core/tests/integration/store.test.ts b/packages/store/src/core/tests/integration/store.test.ts index 17d49a07..c3949589 100644 --- a/packages/store/src/core/tests/integration/store.test.ts +++ b/packages/store/src/core/tests/integration/store.test.ts @@ -83,7 +83,7 @@ describe('store lifecycle integration', () => { // Test 1: Guard rejects when not ready const failPromise = store.request.delayedAction().catch(e => e); await vi.runAllTimersAsync(); - await expect(failPromise).resolves.toMatchObject({ message: 'Rejected' }); + await expect(failPromise).resolves.toMatchObject({ code: 'REJECTED' }); // Test 2: Guard passes when ready ready = true; diff --git a/packages/store/src/core/tests/queue.test.ts b/packages/store/src/core/tests/queue.test.ts index 89894e5b..53ce396d 100644 --- a/packages/store/src/core/tests/queue.test.ts +++ b/packages/store/src/core/tests/queue.test.ts @@ -1,6 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { StoreError } from '../errors'; import { createQueue, delay } from '../queue'; describe('queue', () => { @@ -89,7 +88,7 @@ describe('queue', () => { const promise1 = queue.enqueue({ name: 'a', key: 'same', handler: first }); const promise2 = queue.enqueue({ name: 'b', key: 'same', handler: second }); - await expect(promise1).rejects.toThrow(StoreError); + await expect(promise1).rejects.toMatchObject({ code: 'SUPERSEDED' }); await expect(promise2).resolves.toBe('second'); expect(first).not.toHaveBeenCalled(); expect(second).toHaveBeenCalledOnce(); @@ -177,7 +176,7 @@ describe('queue', () => { expect(queue.dequeue('k')).toBe(false); vi.advanceTimersByTime(100); - await expect(promise).rejects.toThrow(StoreError); + await expect(promise).rejects.toMatchObject({ code: 'REMOVED' }); expect(handler).not.toHaveBeenCalled(); }); }); @@ -192,8 +191,8 @@ describe('queue', () => { queue.clear(); vi.advanceTimersByTime(100); - await expect(p1).rejects.toThrow(StoreError); - await expect(p2).rejects.toThrow(StoreError); + await expect(p1).rejects.toMatchObject({ code: 'REMOVED' }); + await expect(p2).rejects.toMatchObject({ code: 'REMOVED' }); expect(Reflect.ownKeys(queue.queued).length).toBe(0); }); }); @@ -243,7 +242,7 @@ describe('queue', () => { await new Promise((_, reject) => { signal.addEventListener('abort', () => { aborted = true; - reject(new Error('aborted')); + reject(signal.reason); }); setTimeout(() => {}, 1000); }); @@ -251,9 +250,9 @@ describe('queue', () => { }); await new Promise(r => setTimeout(r, 10)); - queue.abort('k', 'test abort'); + queue.abort('k'); - await expect(promise).rejects.toThrow(); + await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); expect(aborted).toBe(true); }); }); @@ -351,7 +350,9 @@ describe('queue', () => { const queue = createQueue(); queue.destroy(); - await expect(queue.enqueue({ name: 't', key: 'k', handler: vi.fn() })).rejects.toThrow('Queue destroyed'); + await expect(queue.enqueue({ name: 't', key: 'k', handler: vi.fn() })).rejects.toMatchObject({ + code: 'DESTROYED', + }); }); it('aborts all pending on destroy', async () => { @@ -372,7 +373,7 @@ describe('queue', () => { await new Promise(r => setTimeout(r, 10)); queue.destroy(); - await expect(promise).rejects.toThrow(); + await expect(promise).rejects.toMatchObject({ code: 'ABORTED' }); expect(aborted).toHaveBeenCalled(); expect(queue.destroyed).toBe(true); }); @@ -401,7 +402,7 @@ describe('queue', () => { }); // First should be superseded - await expect(promise1).rejects.toMatchObject({ message: 'Superseded' }); + await expect(promise1).rejects.toMatchObject({ code: 'SUPERSEDED' }); // Queue should only have the second task (first was explicitly deleted) expect(Reflect.ownKeys(queue.queued).length).toBe(1); diff --git a/packages/store/src/core/tests/store.test.ts b/packages/store/src/core/tests/store.test.ts index 7d5b4b37..f8e23f7b 100644 --- a/packages/store/src/core/tests/store.test.ts +++ b/packages/store/src/core/tests/store.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { StoreError } from '../errors'; import { createQueue } from '../queue'; import { createSlice } from '../slice'; import { createStore } from '../store'; @@ -210,7 +209,7 @@ describe('store', () => { onError: () => {}, // silence errors }); - await expect(store.request.setVolume(0.5)).rejects.toThrow(StoreError); + await expect(store.request.setVolume(0.5)).rejects.toMatchObject({ code: 'NO_TARGET' }); }); it('coordinates requests with same key', async () => { @@ -225,7 +224,7 @@ describe('store', () => { const playPromise = store.request.play(); const pausePromise = store.request.pause(); - await expect(playPromise).rejects.toThrow(StoreError); + await expect(playPromise).rejects.toMatchObject({ code: 'SUPERSEDED' }); await pausePromise; expect(media.paused).toBe(true);