diff --git a/.claude/plans/store/feature-api-redesign.md b/.claude/plans/store/feature-api-redesign.md index 501694cb..b09bd46a 100644 --- a/.claude/plans/store/feature-api-redesign.md +++ b/.claude/plans/store/feature-api-redesign.md @@ -1,5 +1,8 @@ # Feature API Redesign +**Status:** COMPLETED +**Branch:** `refactor/store-feature-attach` + Merge `getSnapshot` and `subscribe` into a single `attach` method with explicit `set()`. ## Overview diff --git a/.claude/plans/store/reactive.md b/.claude/plans/store/reactive.md index 0599e47c..dbf9fe27 100644 --- a/.claude/plans/store/reactive.md +++ b/.claude/plans/store/reactive.md @@ -7,13 +7,13 @@ Simplified state management with explicit mutations and computed values. -PR #311 introduced proxy-based reactivity (Valtio-style). PR #321 replaced it with a simpler `State` + `Computed` design - explicit mutations via `set`/`patch`/`delete` instead of proxy traps, and `Computed` for derived values. +PR #311 introduced proxy-based reactivity (Valtio-style). PR #321 replaced it with a simpler `State` + `Computed` design - explicit mutations via `patch` instead of proxy traps, and `Computed` for derived values. ## Key Decisions | Decision | Rationale | | ----------------------- | ------------------------------------------------------------ | -| Explicit mutations | `set`/`patch`/`delete` clearer than proxy assignment | +| Explicit mutations | `patch()` clearer than proxy assignment | | Frozen snapshots | `Object.freeze()` on `current` prevents accidental mutations | | Key-based subscriptions | Built into State, subscribe to specific keys for efficiency | | Computed class | Lazy derivation, notifies only when result actually changes | @@ -27,10 +27,8 @@ import { createComputed, createState, flush } from '@videojs/store'; // State const state = createState({ volume: 1, muted: false }); state.current; // readonly snapshot -state.set('volume', 0.5); // single key -state.patch({ volume: 0.8 }); // multiple keys +state.patch({ volume: 0.8 }); // update one or more keys state.subscribe(listener); // all changes -state.subscribe(['volume'], fn); // specific keys // Computed const effective = createComputed(state, ['volume', 'muted'], ({ volume, muted }) => (muted ? 0 : volume)); @@ -43,4 +41,6 @@ effective.destroy(); // cleanup Removed from PR #311: `reactive`, `snapshot`, `track`, `batch`, `subscribe`, `subscribeKeys` -Migration: Use `createState()` with explicit mutations and `createComputed()` for derived values. +Migration: Use `createState()` with `patch()` and `createComputed()` for derived values. + +**Note:** `WritableState` now only exposes `patch()`. The `set()` and `delete()` methods were removed as `patch()` covers all use cases. diff --git a/.claude/skills/api/references/typescript.md b/.claude/skills/api/references/typescript.md index ef53df01..eecf5b0d 100644 --- a/.claude/skills/api/references/typescript.md +++ b/.claude/skills/api/references/typescript.md @@ -40,12 +40,20 @@ Capture target/platform type once, let callbacks infer: ```ts // Capture Target type once -const feature = createFeature()({ - initialState: { volume: 1 }, - getSnapshot: ({ target }) => ({ volume: target.volume }), - subscribe: ({ target, update }) => { - target.addEventListener('volumechange', update); - return () => target.removeEventListener('volumechange', update); +const feature = defineFeature()({ + state: ({ task }) => ({ + volume: 1, + setVolume(v: number) { + return task({ handler: ({ target }) => { target.volume = v; } }); + }, + }), + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume }); + + sync(); + + target.addEventListener('volumechange', sync, { signal }); }, }); ``` diff --git a/.claude/skills/docs/templates/api-reference.md b/.claude/skills/docs/templates/api-reference.md index 3187e534..f40f36c1 100644 --- a/.claude/skills/docs/templates/api-reference.md +++ b/.claude/skills/docs/templates/api-reference.md @@ -318,29 +318,34 @@ Brief description of what state this feature manages. import { featureName } from '@videojs/core/dom'; // or -import { createFeature } from '@videojs/store'; +import { defineFeature } from '@videojs/store'; -const featureName = createFeature()({ -initialState: { -property1: defaultValue, -property2: defaultValue, -}, +const featureName = defineFeature()({ + state: ({ task }) => ({ + property1: defaultValue, + property2: defaultValue, -getSnapshot: ({ target }) => ({ -property1: target.property1, -property2: target.property2, -}), + actionName(input: InputType) { + return task({ + key: 'actionKey', + handler({ target }) { + target.property = input; + return target.property; + }, + }); + }, + }), -subscribe: ({ target, update, signal }) => { -listen(target, 'eventname', update, { signal }); -}, + attach({ target, signal, set }) { + const sync = () => set({ + property1: target.property1, + property2: target.property2, + }); -request: { -requestName: (input, { target }) => { -target.property = input; -return target.property; -}, -}, + sync(); + + listen(target, 'eventname', sync, { signal }); + }, }); ### State @@ -350,11 +355,11 @@ return target.property; | `property1` | `type` | Description | | `property2` | `type` | Description | -### Requests +### Actions -| Request | Input | Output | Description | -| ------------- | ----------- | ------------ | ------------ | -| `requestName` | `InputType` | `OutputType` | What it does | +| Action | Input | Output | Description | +| ------------ | ----------- | ------------ | ------------ | +| `actionName` | `InputType` | `OutputType` | What it does | ### Type Inference diff --git a/packages/core/src/dom/store/features/buffer.ts b/packages/core/src/dom/store/features/buffer.ts index bed47e7f..3922de61 100644 --- a/packages/core/src/dom/store/features/buffer.ts +++ b/packages/core/src/dom/store/features/buffer.ts @@ -11,14 +11,17 @@ export const bufferFeature = defineFeature()({ seekable: [] as [number, number][], }), - getSnapshot: ({ target }) => ({ - buffered: serializeTimeRanges(target.buffered), - seekable: serializeTimeRanges(target.seekable), - }), + attach({ target, signal, set }) { + const sync = () => + set({ + buffered: serializeTimeRanges(target.buffered), + seekable: serializeTimeRanges(target.seekable), + }); - subscribe: ({ target, update, signal }) => { - listen(target, 'progress', update, { signal }); - listen(target, 'emptied', update, { signal }); + sync(); + + listen(target, 'progress', sync, { signal }); + listen(target, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/playback.ts b/packages/core/src/dom/store/features/playback.ts index 48b9124e..8992d839 100644 --- a/packages/core/src/dom/store/features/playback.ts +++ b/packages/core/src/dom/store/features/playback.ts @@ -36,19 +36,22 @@ export const playbackFeature = defineFeature()({ }, }), - getSnapshot: ({ target }) => ({ - paused: target.paused, - ended: target.ended, - started: !target.paused || target.currentTime > 0, - waiting: target.readyState < HTMLMediaElement.HAVE_FUTURE_DATA && !target.paused, - }), + attach({ target, signal, set }) { + const sync = () => + set({ + paused: target.paused, + ended: target.ended, + started: !target.paused || target.currentTime > 0, + waiting: target.readyState < HTMLMediaElement.HAVE_FUTURE_DATA && !target.paused, + }); - subscribe: ({ target, update, signal }) => { - listen(target, 'play', update, { signal }); - listen(target, 'pause', update, { signal }); - listen(target, 'ended', update, { signal }); - listen(target, 'playing', update, { signal }); - listen(target, 'waiting', update, { signal }); + sync(); + + listen(target, 'play', sync, { signal }); + listen(target, 'pause', sync, { signal }); + listen(target, 'ended', sync, { signal }); + listen(target, 'playing', sync, { signal }); + listen(target, 'waiting', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/source.ts b/packages/core/src/dom/store/features/source.ts index faaeba1b..f7bf696b 100644 --- a/packages/core/src/dom/store/features/source.ts +++ b/packages/core/src/dom/store/features/source.ts @@ -9,6 +9,7 @@ export const sourceFeature = defineFeature()({ source: null as string | null, /** Whether enough data is loaded to begin playback. */ canPlay: false, + /** Load a new media source. Cancels all pending operations. Returns the new source URL. */ loadSource(src: string) { return task({ @@ -23,16 +24,19 @@ export const sourceFeature = defineFeature()({ }, }), - getSnapshot: ({ target }) => ({ - source: target.currentSrc || target.src || null, - canPlay: target.readyState >= HTMLMediaElement.HAVE_ENOUGH_DATA, - }), + attach({ target, signal, set }) { + const sync = () => + set({ + source: target.currentSrc || target.src || null, + canPlay: target.readyState >= HTMLMediaElement.HAVE_ENOUGH_DATA, + }); - subscribe: ({ target, update, signal }) => { - listen(target, 'canplay', update, { signal }); - listen(target, 'canplaythrough', update, { signal }); - listen(target, 'loadstart', update, { signal }); - listen(target, 'emptied', update, { signal }); + sync(); + + listen(target, 'canplay', sync, { signal }); + listen(target, 'canplaythrough', sync, { signal }); + listen(target, 'loadstart', sync, { signal }); + listen(target, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/tests/buffer.test.ts b/packages/core/src/dom/store/features/tests/buffer.test.ts index 4f87821a..636041b9 100644 --- a/packages/core/src/dom/store/features/tests/buffer.test.ts +++ b/packages/core/src/dom/store/features/tests/buffer.test.ts @@ -1,25 +1,21 @@ -import { describe, expect, it, vi } from 'vitest'; +import { createStore } from '@videojs/store'; +import { describe, expect, it } from 'vitest'; import { bufferFeature } from '../buffer'; describe('bufferFeature', () => { - describe('getSnapshot', () => { - it('captures buffered and seekable ranges from video element', () => { + describe('attach', () => { + it('syncs buffered and seekable ranges on attach', () => { const video = createMockVideo({ buffered: createTimeRanges([[0, 60]]), seekable: createTimeRanges([[0, 120]]), }); - const snapshot = bufferFeature.getSnapshot({ - target: video, - get: () => ({ buffered: [], seekable: [] }), - initialState: { buffered: [], seekable: [] }, - }); + const store = createStore({ features: [bufferFeature] }); + store.attach(video); - expect(snapshot).toEqual({ - buffered: [[0, 60]], - seekable: [[0, 120]], - }); + expect(store.state.buffered).toEqual([[0, 60]]); + expect(store.state.seekable).toEqual([[0, 120]]); }); it('handles multiple ranges', () => { @@ -31,56 +27,61 @@ describe('bufferFeature', () => { seekable: createTimeRanges([[0, 120]]), }); - const snapshot = bufferFeature.getSnapshot({ - target: video, - get: () => ({ buffered: [], seekable: [] }), - initialState: { buffered: [], seekable: [] }, - }); + const store = createStore({ features: [bufferFeature] }); + store.attach(video); - expect(snapshot.buffered).toEqual([ + expect(store.state.buffered).toEqual([ [0, 30], [60, 90], ]); }); - }); - describe('subscribe', () => { - it('calls update on progress event', () => { + it('updates on progress event', () => { const video = createMockVideo({ buffered: createTimeRanges([[0, 50]]), seekable: createTimeRanges([[0, 100]]), }); - const update = vi.fn(); - const controller = new AbortController(); - bufferFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: () => ({ buffered: [], seekable: [] }), + const store = createStore({ features: [bufferFeature] }); + store.attach(video); + + // Update the mock video's buffered range + Object.defineProperty(video, 'buffered', { + value: createTimeRanges([[0, 75]]), + writable: false, + configurable: true, }); + video.dispatchEvent(new Event('progress')); - expect(update).toHaveBeenCalled(); + expect(store.state.buffered).toEqual([[0, 75]]); }); - it('calls update on emptied event', () => { + it('updates on emptied event', () => { const video = createMockVideo({ - buffered: createTimeRanges([]), - seekable: createTimeRanges([]), + buffered: createTimeRanges([[0, 50]]), + seekable: createTimeRanges([[0, 100]]), }); - const update = vi.fn(); - const controller = new AbortController(); - bufferFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: () => ({ buffered: [], seekable: [] }), + const store = createStore({ features: [bufferFeature] }); + store.attach(video); + + // Update the mock video to have no buffered content + Object.defineProperty(video, 'buffered', { + value: createTimeRanges([]), + writable: false, + configurable: true, }); + Object.defineProperty(video, 'seekable', { + value: createTimeRanges([]), + writable: false, + configurable: true, + }); + video.dispatchEvent(new Event('emptied')); - expect(update).toHaveBeenCalled(); + expect(store.state.buffered).toEqual([]); + expect(store.state.seekable).toEqual([]); }); }); }); @@ -94,10 +95,10 @@ function createMockVideo( const video = document.createElement('video'); if (overrides.buffered !== undefined) { - Object.defineProperty(video, 'buffered', { value: overrides.buffered, writable: false }); + Object.defineProperty(video, 'buffered', { value: overrides.buffered, writable: false, configurable: true }); } if (overrides.seekable !== undefined) { - Object.defineProperty(video, 'seekable', { value: overrides.seekable, writable: false }); + Object.defineProperty(video, 'seekable', { value: overrides.seekable, writable: false, configurable: true }); } return video; diff --git a/packages/core/src/dom/store/features/tests/playback.test.ts b/packages/core/src/dom/store/features/tests/playback.test.ts index d8c1350f..6be0d1a5 100644 --- a/packages/core/src/dom/store/features/tests/playback.test.ts +++ b/packages/core/src/dom/store/features/tests/playback.test.ts @@ -1,23 +1,11 @@ import { createStore } from '@videojs/store'; -import { noop } from '@videojs/utils/function'; import { describe, expect, it, vi } from 'vitest'; -import type { PlaybackState } from '../playback'; import { playbackFeature } from '../playback'; -const mockState = () => - ({ - paused: true, - ended: false, - started: false, - waiting: false, - play: noop, - pause: noop, - }) as unknown as PlaybackState; - describe('playbackFeature', () => { - describe('getSnapshot', () => { - it('captures current playback state from video element', () => { + describe('attach', () => { + it('syncs playback state on attach', () => { const video = createMockVideo({ paused: false, ended: false, @@ -25,18 +13,13 @@ describe('playbackFeature', () => { readyState: HTMLMediaElement.HAVE_ENOUGH_DATA, }); - const snapshot = playbackFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); - expect(snapshot).toEqual({ - paused: false, - ended: false, - started: true, - waiting: false, - }); + expect(store.state.paused).toBe(false); + expect(store.state.ended).toBe(false); + expect(store.state.started).toBe(true); + expect(store.state.waiting).toBe(false); }); it('detects waiting state when buffering', () => { @@ -45,13 +28,10 @@ describe('playbackFeature', () => { readyState: HTMLMediaElement.HAVE_CURRENT_DATA, }); - const snapshot = playbackFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); - expect(snapshot.waiting).toBe(true); + expect(store.state.waiting).toBe(true); }); it('detects started from currentTime', () => { @@ -60,13 +40,10 @@ describe('playbackFeature', () => { currentTime: 5, }); - const snapshot = playbackFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); - expect(snapshot.started).toBe(true); + expect(store.state.started).toBe(true); }); it('detects started from playing state', () => { @@ -75,80 +52,71 @@ describe('playbackFeature', () => { currentTime: 0, }); - const snapshot = playbackFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); - expect(snapshot.started).toBe(true); + expect(store.state.started).toBe(true); }); - }); - describe('subscribe', () => { - it('calls update on play event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on play event', () => { + const video = createMockVideo({ paused: true }); - playbackFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); + + expect(store.state.paused).toBe(true); + + // Update mock to playing state + Object.defineProperty(video, 'paused', { value: false, writable: false, configurable: true }); video.dispatchEvent(new Event('play')); - expect(update).toHaveBeenCalled(); + expect(store.state.paused).toBe(false); }); - it('calls update on pause event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on pause event', () => { + const video = createMockVideo({ paused: false }); - playbackFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); + + expect(store.state.paused).toBe(false); + + // Update mock to paused state + Object.defineProperty(video, 'paused', { value: true, writable: false, configurable: true }); video.dispatchEvent(new Event('pause')); - expect(update).toHaveBeenCalled(); + expect(store.state.paused).toBe(true); }); - it('calls update on ended event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on ended event', () => { + const video = createMockVideo({ ended: false }); - playbackFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); + + expect(store.state.ended).toBe(false); + + // Update mock to ended state + Object.defineProperty(video, 'ended', { value: true, writable: false, configurable: true }); video.dispatchEvent(new Event('ended')); - expect(update).toHaveBeenCalled(); + expect(store.state.ended).toBe(true); }); - it('unsubscribes when signal aborted', () => { + it('stops listening when store is destroyed', () => { const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); - playbackFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); - controller.abort(); + const store = createStore({ features: [playbackFeature] }); + store.attach(video); + + store.destroy(); + + // Update mock to playing state + Object.defineProperty(video, 'paused', { value: false, writable: false, configurable: true }); video.dispatchEvent(new Event('play')); - expect(update).not.toHaveBeenCalled(); + // State should not update after destroy + expect(store.state.paused).toBe(true); }); }); @@ -190,16 +158,16 @@ function createMockVideo( const video = document.createElement('video'); if (overrides.paused !== undefined) { - Object.defineProperty(video, 'paused', { value: overrides.paused, writable: false }); + Object.defineProperty(video, 'paused', { value: overrides.paused, writable: false, configurable: true }); } if (overrides.ended !== undefined) { - Object.defineProperty(video, 'ended', { value: overrides.ended, writable: false }); + Object.defineProperty(video, 'ended', { value: overrides.ended, writable: false, configurable: true }); } if (overrides.currentTime !== undefined) { video.currentTime = overrides.currentTime; } if (overrides.readyState !== undefined) { - Object.defineProperty(video, 'readyState', { value: overrides.readyState, writable: false }); + Object.defineProperty(video, 'readyState', { value: overrides.readyState, writable: false, configurable: true }); } return video; diff --git a/packages/core/src/dom/store/features/tests/source.test.ts b/packages/core/src/dom/store/features/tests/source.test.ts index 60c2f2e1..2f776d8e 100644 --- a/packages/core/src/dom/store/features/tests/source.test.ts +++ b/packages/core/src/dom/store/features/tests/source.test.ts @@ -1,36 +1,22 @@ import { createStore } from '@videojs/store'; -import { noop } from '@videojs/utils/function'; import { describe, expect, it, vi } from 'vitest'; -import type { SourceState } from '../source'; import { sourceFeature } from '../source'; -const mockState = () => - ({ - source: null, - canPlay: false, - loadSource: noop, - }) as unknown as SourceState; - describe('sourceFeature', () => { - describe('getSnapshot', () => { - it('captures source state from video element', () => { + describe('attach', () => { + it('syncs source state on attach', () => { const video = createMockVideo({ currentSrc: 'https://example.com/video.mp4', src: 'https://example.com/video.mp4', readyState: HTMLMediaElement.HAVE_ENOUGH_DATA, }); - const snapshot = sourceFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [sourceFeature] }); + store.attach(video); - expect(snapshot).toEqual({ - source: 'https://example.com/video.mp4', - canPlay: true, - }); + expect(store.state.source).toBe('https://example.com/video.mp4'); + expect(store.state.canPlay).toBe(true); }); it('returns null source when no source set', () => { @@ -39,67 +25,78 @@ describe('sourceFeature', () => { Object.defineProperty(video, 'currentSrc', { value: '', writable: false }); Object.defineProperty(video, 'readyState', { value: HTMLMediaElement.HAVE_NOTHING, writable: false }); - const snapshot = sourceFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), + const store = createStore({ features: [sourceFeature] }); + store.attach(video); + + expect(store.state.source).toBe(null); + expect(store.state.canPlay).toBe(false); + }); + + it('updates on canplay event', () => { + const video = createMockVideo({ + currentSrc: '', + readyState: HTMLMediaElement.HAVE_NOTHING, }); - expect(snapshot.source).toBe(null); - expect(snapshot.canPlay).toBe(false); - }); - }); + const store = createStore({ features: [sourceFeature] }); + store.attach(video); - describe('subscribe', () => { - it('calls update on canplay event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); + expect(store.state.canPlay).toBe(false); - sourceFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, + // Update mock to ready state + Object.defineProperty(video, 'readyState', { + value: HTMLMediaElement.HAVE_ENOUGH_DATA, + writable: false, + configurable: true, }); video.dispatchEvent(new Event('canplay')); - expect(update).toHaveBeenCalled(); + expect(store.state.canPlay).toBe(true); }); - it('calls update on loadstart event', () => { + it('updates on loadstart event', () => { const video = createMockVideo({ - currentSrc: 'https://example.com/new.mp4', - src: 'https://example.com/new.mp4', + currentSrc: 'https://example.com/video.mp4', }); - const update = vi.fn(); - const controller = new AbortController(); - sourceFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, + const store = createStore({ features: [sourceFeature] }); + store.attach(video); + + expect(store.state.source).toBe('https://example.com/video.mp4'); + + // Update mock with new source + Object.defineProperty(video, 'currentSrc', { + value: 'https://example.com/new.mp4', + writable: false, + configurable: true, }); video.dispatchEvent(new Event('loadstart')); - expect(update).toHaveBeenCalled(); + expect(store.state.source).toBe('https://example.com/new.mp4'); }); - it('calls update on emptied event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on emptied event', () => { + const video = createMockVideo({ + currentSrc: 'https://example.com/video.mp4', + readyState: HTMLMediaElement.HAVE_ENOUGH_DATA, + }); - sourceFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, + const store = createStore({ features: [sourceFeature] }); + store.attach(video); + + expect(store.state.canPlay).toBe(true); + + // Update mock to empty state + Object.defineProperty(video, 'currentSrc', { value: '', writable: false, configurable: true }); + Object.defineProperty(video, 'readyState', { + value: HTMLMediaElement.HAVE_NOTHING, + writable: false, + configurable: true, }); video.dispatchEvent(new Event('emptied')); - expect(update).toHaveBeenCalled(); + expect(store.state.source).toBe(null); + expect(store.state.canPlay).toBe(false); }); }); @@ -132,13 +129,13 @@ function createMockVideo( const video = document.createElement('video'); if (overrides.currentSrc !== undefined) { - Object.defineProperty(video, 'currentSrc', { value: overrides.currentSrc, writable: false }); + Object.defineProperty(video, 'currentSrc', { value: overrides.currentSrc, writable: false, configurable: true }); } if (overrides.src !== undefined) { video.src = overrides.src; } if (overrides.readyState !== undefined) { - Object.defineProperty(video, 'readyState', { value: overrides.readyState, writable: false }); + Object.defineProperty(video, 'readyState', { value: overrides.readyState, writable: false, configurable: true }); } return video; diff --git a/packages/core/src/dom/store/features/tests/time.test.ts b/packages/core/src/dom/store/features/tests/time.test.ts index 2585eee8..bc699f9f 100644 --- a/packages/core/src/dom/store/features/tests/time.test.ts +++ b/packages/core/src/dom/store/features/tests/time.test.ts @@ -1,35 +1,21 @@ import { createStore } from '@videojs/store'; -import { noop } from '@videojs/utils/function'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; -import type { TimeState } from '../time'; import { timeFeature } from '../time'; -const mockState = () => - ({ - currentTime: 0, - duration: 0, - seek: noop, - }) as unknown as TimeState; - describe('timeFeature', () => { - describe('getSnapshot', () => { - it('captures current time state from video element', () => { + describe('attach', () => { + it('syncs time state on attach', () => { const video = createMockVideo({ currentTime: 30, duration: 120, }); - const snapshot = timeFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [timeFeature] }); + store.attach(video); - expect(snapshot).toEqual({ - currentTime: 30, - duration: 120, - }); + expect(store.state.currentTime).toBe(30); + expect(store.state.duration).toBe(120); }); it('handles NaN duration', () => { @@ -38,79 +24,71 @@ describe('timeFeature', () => { duration: Number.NaN, }); - const snapshot = timeFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [timeFeature] }); + store.attach(video); - expect(snapshot.duration).toBe(0); + expect(store.state.duration).toBe(0); }); - }); - describe('subscribe', () => { - it('calls update on timeupdate event', () => { - const video = createMockVideo({ currentTime: 42 }); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on timeupdate event', () => { + const video = createMockVideo({ currentTime: 0 }); - timeFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [timeFeature] }); + store.attach(video); + + expect(store.state.currentTime).toBe(0); + + // Update mock currentTime + video.currentTime = 42; video.dispatchEvent(new Event('timeupdate')); - expect(update).toHaveBeenCalled(); + expect(store.state.currentTime).toBe(42); }); - it('calls update on durationchange event', () => { - const video = createMockVideo({ duration: 100 }); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on durationchange event', () => { + const video = createMockVideo({ duration: 0 }); - timeFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [timeFeature] }); + store.attach(video); + + expect(store.state.duration).toBe(0); + + // Update mock duration + Object.defineProperty(video, 'duration', { value: 100, writable: false, configurable: true }); video.dispatchEvent(new Event('durationchange')); - expect(update).toHaveBeenCalled(); + expect(store.state.duration).toBe(100); }); - it('calls update on seeked event', () => { - const video = createMockVideo({ currentTime: 50 }); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on seeked event', () => { + const video = createMockVideo({ currentTime: 0 }); - timeFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [timeFeature] }); + store.attach(video); + + // Update mock currentTime + video.currentTime = 50; video.dispatchEvent(new Event('seeked')); - expect(update).toHaveBeenCalled(); + expect(store.state.currentTime).toBe(50); }); - it('calls update on emptied event', () => { - const video = createMockVideo({}); - const update = vi.fn(); - const controller = new AbortController(); - - timeFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, + it('updates on emptied event', () => { + const video = createMockVideo({ + currentTime: 30, + duration: 120, }); + + const store = createStore({ features: [timeFeature] }); + store.attach(video); + + // Update mock to empty state + video.currentTime = 0; + Object.defineProperty(video, 'duration', { value: Number.NaN, writable: false, configurable: true }); video.dispatchEvent(new Event('emptied')); - expect(update).toHaveBeenCalled(); + expect(store.state.currentTime).toBe(0); + expect(store.state.duration).toBe(0); }); }); @@ -147,7 +125,7 @@ function createMockVideo( video.currentTime = overrides.currentTime; } if (overrides.duration !== undefined) { - Object.defineProperty(video, 'duration', { value: overrides.duration, writable: false }); + Object.defineProperty(video, 'duration', { value: overrides.duration, writable: false, configurable: true }); } return video; diff --git a/packages/core/src/dom/store/features/tests/volume.test.ts b/packages/core/src/dom/store/features/tests/volume.test.ts index 512cdf9c..e8c098d8 100644 --- a/packages/core/src/dom/store/features/tests/volume.test.ts +++ b/packages/core/src/dom/store/features/tests/volume.test.ts @@ -1,54 +1,38 @@ import { createStore } from '@videojs/store'; -import { noop } from '@videojs/utils/function'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; -import type { VolumeState } from '../volume'; import { volumeFeature } from '../volume'; -const mockState = () => - ({ - volume: 1, - muted: false, - changeVolume: noop, - toggleMute: noop, - }) as unknown as VolumeState; - describe('volumeFeature', () => { - describe('getSnapshot', () => { - it('captures volume state from video element', () => { + describe('attach', () => { + it('syncs volume state on attach', () => { const video = createMockVideo({ volume: 0.8, muted: false, }); - const snapshot = volumeFeature.getSnapshot({ - target: video, - get: mockState, - initialState: mockState(), - }); + const store = createStore({ features: [volumeFeature] }); + store.attach(video); - expect(snapshot).toEqual({ - volume: 0.8, - muted: false, - }); + expect(store.state.volume).toBe(0.8); + expect(store.state.muted).toBe(false); }); - }); - describe('subscribe', () => { - it('calls update on volumechange event', () => { - const video = createMockVideo({ volume: 0.5, muted: true }); - const update = vi.fn(); - const controller = new AbortController(); + it('updates on volumechange event', () => { + const video = createMockVideo({ volume: 1, muted: false }); - volumeFeature.subscribe({ - target: video, - update, - signal: controller.signal, - get: mockState, - }); + const store = createStore({ features: [volumeFeature] }); + store.attach(video); + + expect(store.state.volume).toBe(1); + + // Update mock volume + video.volume = 0.5; + video.muted = true; video.dispatchEvent(new Event('volumechange')); - expect(update).toHaveBeenCalled(); + expect(store.state.volume).toBe(0.5); + expect(store.state.muted).toBe(true); }); }); diff --git a/packages/core/src/dom/store/features/time.ts b/packages/core/src/dom/store/features/time.ts index dfa449fa..69f48101 100644 --- a/packages/core/src/dom/store/features/time.ts +++ b/packages/core/src/dom/store/features/time.ts @@ -23,17 +23,20 @@ export const timeFeature = defineFeature()({ }, }), - getSnapshot: ({ target }) => ({ - currentTime: target.currentTime, - duration: target.duration || 0, - }), + attach({ target, signal, set }) { + const sync = () => + set({ + currentTime: target.currentTime, + duration: target.duration || 0, + }); - subscribe: ({ target, update, signal }) => { - listen(target, 'timeupdate', update, { signal }); - listen(target, 'durationchange', update, { signal }); - listen(target, 'seeked', update, { signal }); - listen(target, 'loadedmetadata', update, { signal }); - listen(target, 'emptied', update, { signal }); + sync(); + + listen(target, 'timeupdate', sync, { signal }); + listen(target, 'durationchange', sync, { signal }); + listen(target, 'seeked', sync, { signal }); + listen(target, 'loadedmetadata', sync, { signal }); + listen(target, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/volume.ts b/packages/core/src/dom/store/features/volume.ts index deda7bf7..83dd0fb2 100644 --- a/packages/core/src/dom/store/features/volume.ts +++ b/packages/core/src/dom/store/features/volume.ts @@ -33,13 +33,12 @@ export const volumeFeature = defineFeature()({ }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); - subscribe: ({ target, update, signal }) => { - listen(target, 'volumechange', update, { signal }); + sync(); + + listen(target, 'volumechange', sync, { signal }); }, }); diff --git a/packages/store/README.md b/packages/store/README.md index 727aba8a..aa5fa1e5 100644 --- a/packages/store/README.md +++ b/packages/store/README.md @@ -56,24 +56,16 @@ import { defineFeature } from '@videojs/store'; import { listen } from '@videojs/utils/dom'; const volumeFeature = defineFeature()({ - // State factory - returns initial state and actions - state: ({ task }) => ({ - // State (plain values) + state: ({ task, target }) => ({ volume: 1, muted: false, - // Action - async, tracked - changeVolume(volume: number) { - return task({ - key: 'volume', - handler({ target }) { - target.volume = Math.max(0, Math.min(1, volume)); - return target.volume; - }, - }); + // Sync - use target() directly + setVolume(value: number) { + target().volume = Math.max(0, Math.min(1, value)); }, - // Action - async, tracked + // Task - tracked, coordinated toggleMute() { return task({ key: 'mute', @@ -85,15 +77,12 @@ const volumeFeature = defineFeature()({ }, }), - // Sync state from target - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); - // Subscribe to target events - subscribe: ({ target, update, signal }) => { - listen(target, 'volumechange', update, { signal }); + sync(); + + listen(target, 'volumechange', sync, { signal }); }, }); ``` @@ -127,13 +116,13 @@ type MediaState = UnionFeatureState; ### Actions -Actions modify the target. Use `task()` for async operations with tracking, or access the target directly for simple sync mutations. +Actions modify the target. Use `task()` for operations—handlers receive `target` directly. ```ts -state: ({ task, target }) => ({ +state: ({ task }) => ({ volume: 1, - // Async action with tracking + // Action with tracking (has key) changeVolume(volume: number) { return task({ key: 'volume', @@ -144,12 +133,7 @@ state: ({ task, target }) => ({ }); }, - // Sync action - direct target access - setVolumeDirect(volume: number) { - target().volume = volume; - }, - - // Async action - fire-and-forget (no tracking) + // Fire-and-forget (no key) logVolume() { return task(({ target }) => { console.log('Current volume:', target.volume); @@ -424,7 +408,6 @@ All store errors include a `code` for programmatic handling: | ------------ | ---------------------------- | | `ABORTED` | Task aborted via signal | | `DESTROYED` | Store destroyed | -| `DETACHED` | Target detached | | `NO_TARGET` | No target attached | | `SUPERSEDED` | Replaced by same-key task | @@ -475,8 +458,8 @@ const state = createState({ volume: 1, muted: false }); // Read via .current const { volume } = state.current; // 1 -// Mutate via set() or patch() - changes are auto-batched -state.set('volume', 0.5); +// Mutate via patch() - changes are auto-batched +state.patch({ volume: 0.5 }); state.patch({ volume: 0.5, muted: true }); // Only ONE notification fires (after microtask) @@ -493,58 +476,13 @@ isState(state); // true flush(); ``` -### Capability Checking - -Features can expose capability via state. UI components check before rendering. - -```ts -const qualityFeature = defineFeature()({ - state: ({ task }) => ({ - supported: false, - levels: [] as QualityLevel[], - currentLevel: -1, - - setLevel(index: number) { - return task({ - key: 'quality', - handler: ({ target }) => target.setQualityLevel(index), - }); - }, - }), - - getSnapshot: ({ target, initialState }) => { - if (target.canSetVideoQuality) { - return { - supported: true, - levels: target.levels, - currentLevel: target.currentLevel, - }; - } - return initialState; // supported: false - }, - - subscribe: ({ target, update, signal }) => { - target.addEventListener('qualitychange', update, { signal }); - }, -}); - -// UI checks capability -function QualityMenu() { - const { supported, levels } = store; - - if (!supported) return null; - - return ; -} -``` - ## How It's Different | | Redux/Zustand | React Query | @videojs/store | | ----------------- | ---------------- | --------------------- | -------------------------- | | **Authority** | You own state | Server owns state | External system owns state | | **Mutations** | Sync reducers | Async server requests | Async tasks to target | -| **State source** | Internal store | HTTP cache | `getSnapshot` from target | +| **State source** | Internal store | HTTP cache | Synced from target | | **Subscriptions** | To store changes | To query cache | To target events | | **Use case** | App state | Server data | Media, WebSocket, hardware | diff --git a/packages/store/src/core/feature.ts b/packages/store/src/core/feature.ts index 0fd00963..de0faeb0 100644 --- a/packages/store/src/core/feature.ts +++ b/packages/store/src/core/feature.ts @@ -31,40 +31,25 @@ export interface TaskContext { } // ---------------------------------------- -// Sync Config +// Attach // ---------------------------------------- -export type GetSnapshot = (ctx: GetSnapshotContext) => Partial; +export type Attach = (ctx: AttachContext) => void; -export interface GetSnapshotContext { +export interface AttachContext { target: Target; - get: () => Readonly; - initialState: Readonly; -} - -export type Subscribe = (ctx: SubscribeContext) => void; - -export interface SubscribeContext { - target: Target; - update: () => void; signal: AbortSignal; get: () => Readonly; + set: (partial: Partial) => void; } // ---------------------------------------- // Feature Context // ---------------------------------------- -export interface FeatureContext { - task: Task; - get: () => Readonly; - target: () => Target; -} - /** Context passed to state factory - uses loose types to enable State inference. */ export interface StateFactoryContext { task: Task; - get: () => Readonly; target: () => Target; } @@ -76,8 +61,7 @@ export type StateFactory = (ctx: StateFactoryConte export interface FeatureConfig { state: StateFactory; - getSnapshot: GetSnapshot; - subscribe: Subscribe; + attach?: Attach; } export interface Feature extends FeatureConfig { diff --git a/packages/store/src/core/state.ts b/packages/store/src/core/state.ts index 54193a20..3ff80b0c 100644 --- a/packages/store/src/core/state.ts +++ b/packages/store/src/core/state.ts @@ -6,28 +6,21 @@ export interface State { } export interface WritableState extends State { - set: (key: K, value: T[K]) => void; patch: (partial: Partial) => void; - delete: (key: K) => void; } -let flushScheduled = false; - +let isFlushScheduled = false; function scheduleFlush(): void { - if (flushScheduled) return; - flushScheduled = true; + if (isFlushScheduled) return; + isFlushScheduled = true; queueMicrotask(flush); } const pendingContainers = new Set>(); export function flush(): void { - flushScheduled = false; - - for (const container of pendingContainers) { - container.flush(); - } - + isFlushScheduled = false; + for (const container of pendingContainers) container.flush(); pendingContainers.clear(); } @@ -46,21 +39,9 @@ class StateContainer implements WritableState { return this.#current; } - set(key: K, value: T[K]): void { - if (Object.is(this.#current[key], value)) return; - this.#current = Object.freeze({ ...this.#current, [key]: value }); - this.#markPending(); - } - - delete(key: K): void { - if (!(key in this.#current)) return; - const { [key]: _, ...rest } = this.#current; - this.#current = Object.freeze(rest as T); - this.#markPending(); - } - patch(partial: Partial): void { const next = { ...this.#current }; + let changed = false; for (const key in partial) { @@ -88,7 +69,6 @@ class StateContainer implements WritableState { flush(): void { if (!this.#pending) return; this.#pending = false; - for (const fn of this.#listeners) fn(); } diff --git a/packages/store/src/core/store.ts b/packages/store/src/core/store.ts index 25d40ff3..4654f090 100644 --- a/packages/store/src/core/store.ts +++ b/packages/store/src/core/store.ts @@ -4,6 +4,7 @@ import type { PendingTask, StoreConfig } from './config'; import { StoreError } from './errors'; import type { AnyFeature, + AttachContext, StateFactoryContext, TaskContext, TaskHandler, @@ -37,16 +38,15 @@ export function createStore(config: StoreConfig; - const ctx: StateFactoryContext = { + const stateFactoryCtx: StateFactoryContext = { task: executeTask, - get: () => state.current, target: () => { if (!target) throw new StoreError('NO_TARGET'); return target; }, }; - const featureState = buildFeatureState(ctx); + const featureState = buildFeatureState(stateFactoryCtx); state = createState(featureState); const store = { @@ -110,23 +110,22 @@ export function createStore(config: StoreConfig = { + target: newTarget, + signal, + get: () => state.current, + set: (partial) => state.patch(partial), + }; for (const feature of features) { try { - feature.subscribe({ - target: newTarget, - update: () => syncFeature(feature, newTarget), - signal, - get: () => state.current, - }); + feature.attach?.(attachCtx); } catch (error) { handleError(error); } } - syncAll(); - try { config.onAttach?.({ store, target: newTarget, signal }); } catch (error) { @@ -176,26 +175,6 @@ export function createStore(config: StoreConfig, t: Target): void { - try { - const snapshot = feature.getSnapshot({ - target: t, - get: () => state.current, - initialState: featureState, - }); - state.patch(snapshot as Partial); - } catch (error) { - handleError(error); - } - } - async function executeTask(handler: TaskHandler): Promise>; async function executeTask(options: TaskOptions): Promise>; async function executeTask( diff --git a/packages/store/src/core/tests/feature.test.ts b/packages/store/src/core/tests/feature.test.ts index 8b46f8dd..7e2cd433 100644 --- a/packages/store/src/core/tests/feature.test.ts +++ b/packages/store/src/core/tests/feature.test.ts @@ -3,20 +3,15 @@ import { describe, expect, it, vi } from 'vitest'; import { defineFeature, isFeature } from '../feature'; describe('defineFeature', () => { - it('creates feature with create function and config', () => { + it('creates feature with state factory and optional attach', () => { interface Target { value: number; } const feature = defineFeature()({ - state: ({ task, target }) => ({ - // State + state: ({ task }) => ({ count: 0, - // Actions increment(amount: number) { - target().value += amount; - }, - asyncIncrement(amount: number) { return task({ key: 'increment', handler: ({ target }) => { @@ -25,16 +20,17 @@ describe('defineFeature', () => { }); }, }), - getSnapshot: ({ target }) => ({ count: target.value }), - subscribe: vi.fn(), + + attach({ target, set }) { + set({ count: target.value }); + }, }); expect(feature.state).toBeTypeOf('function'); - expect(feature.getSnapshot).toBeTypeOf('function'); - expect(feature.subscribe).toBeTypeOf('function'); + expect(feature.attach).toBeTypeOf('function'); }); - it('factory receives task, get, and target helpers', () => { + it('factory receives task and target helpers', () => { interface Target { value: number; } @@ -43,8 +39,6 @@ describe('defineFeature', () => { defineFeature()({ state: factorySpy, - getSnapshot: () => ({ count: 0 }), - subscribe: () => {}, }); // Can't call the factory directly, but we can verify the shape @@ -52,20 +46,23 @@ describe('defineFeature', () => { expect(factorySpy).not.toHaveBeenCalled(); }); - it('allows sync actions using target()', () => { + it('allows sync actions using task handler', () => { interface Target { volume: number; } const feature = defineFeature()({ - state: ({ target }) => ({ + state: ({ task }) => ({ volume: 1, setVolume(value: number) { - target().volume = value; + return task({ + key: 'volume', + handler: ({ target }) => { + target.volume = value; + }, + }); }, }), - getSnapshot: ({ target }) => ({ volume: target.volume }), - subscribe: () => {}, }); expect(feature.state).toBeTypeOf('function'); @@ -86,8 +83,6 @@ describe('defineFeature', () => { }); }, }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); expect(feature.state).toBeTypeOf('function'); @@ -109,20 +104,25 @@ describe('defineFeature', () => { }); }, }), - getSnapshot: () => ({ loading: false }), - subscribe: () => {}, }); expect(feature.state).toBeTypeOf('function'); }); + + it('attach is optional', () => { + const feature = defineFeature()({ + state: () => ({ playing: false }), + }); + + expect(feature.state).toBeTypeOf('function'); + expect(feature.attach).toBeUndefined(); + }); }); describe('isFeature', () => { it('returns true for features created with defineFeature', () => { const feature = defineFeature()({ state: () => ({ playing: false }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); expect(isFeature(feature)).toBe(true); diff --git a/packages/store/src/core/tests/integration/store.test.ts b/packages/store/src/core/tests/integration/store.test.ts index b1c774d5..e5ab977e 100644 --- a/packages/store/src/core/tests/integration/store.test.ts +++ b/packages/store/src/core/tests/integration/store.test.ts @@ -21,15 +21,16 @@ describe('store lifecycle integration', () => { }); }, }), - getSnapshot: ({ target: t }) => ({ count: t.value }), - subscribe: ({ target: t, update, signal }) => { - events.push('subscribe'); - t.addEventListener('change', update, { signal }); + + attach({ target: t, signal, set }) { + events.push('attach-feature'); + set({ count: t.value }); + + t.addEventListener('change', () => set({ count: t.value }), { signal }); signal.addEventListener('abort', () => events.push('unsubscribe')); }, }); - // Cast to any for test access to dynamic properties const store = createStore({ features: [feature], onSetup: () => events.push('setup'), @@ -42,7 +43,7 @@ describe('store lifecycle integration', () => { targetInstance.value = 5; const detach = store.attach(targetInstance); - expect(events).toEqual(['setup', 'subscribe', 'attach']); + expect(events).toEqual(['setup', 'attach-feature', 'attach']); expect(store.state.count).toBe(5); await store.increment(); @@ -93,8 +94,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ loading: false }), - subscribe: () => {}, }); const store = createStore({ @@ -134,8 +133,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ fetching: false }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); @@ -173,8 +170,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ running: false }), - subscribe: () => {}, }); const store = createStore({ @@ -218,8 +213,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); @@ -251,8 +244,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); const store = createStore({ @@ -287,8 +278,6 @@ describe('task coordination', () => { }); }, }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); @@ -312,14 +301,18 @@ describe('state syncing', () => { it('multiple features merge state correctly', () => { const audioFeature = defineFeature<{ volume: number; rate: number }>()({ state: () => ({ volume: 1 }), - getSnapshot: ({ target }) => ({ volume: target.volume }), - subscribe: () => {}, + + attach({ target, set }) { + set({ volume: target.volume }); + }, }); const playbackFeature = defineFeature<{ volume: number; rate: number }>()({ state: () => ({ rate: 1 }), - getSnapshot: ({ target }) => ({ rate: target.rate }), - subscribe: () => {}, + + attach({ target, set }) { + set({ rate: target.rate }); + }, }); const store = createStore({ @@ -355,9 +348,11 @@ describe('immediate execution', () => { }); }, }), - getSnapshot: ({ target }) => ({ paused: target.paused }), - subscribe: ({ target, update, signal }) => { - target.addEventListener('play', update, { signal }); + + attach({ target, signal, set }) { + set({ paused: target.paused }); + + target.addEventListener('play', () => set({ paused: target.paused }), { signal }); }, }); @@ -390,8 +385,6 @@ describe('meta tracing', () => { }); }, }), - getSnapshot: () => ({ playing: false }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); @@ -420,8 +413,6 @@ describe('meta tracing', () => { }); }, }), - getSnapshot: () => ({ count: 0 }), - subscribe: () => {}, }); const store = createStore({ @@ -450,8 +441,6 @@ describe('meta tracing', () => { }); }, }), - getSnapshot: () => ({ loading: false }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); @@ -472,45 +461,47 @@ describe('meta tracing', () => { }); describe('sync actions', () => { - it('target() allows sync mutations without task', () => { + it('task handler allows sync mutations', async () => { class Target { volume = 1; } const feature = defineFeature()({ - state: ({ target }) => ({ + state: ({ task }) => ({ volume: 1, setVolume(value: number) { - target().volume = value; + return task(({ target }) => { + target.volume = value; + }); }, }), - getSnapshot: ({ target: t }) => ({ volume: t.volume }), - subscribe: () => {}, + + attach({ target: t, set }) { + set({ volume: t.volume }); + }, }); const store = createStore({ features: [feature] }); const targetInstance = new Target(); store.attach(targetInstance); - store.setVolume(0.5); + await store.setVolume(0.5); expect(targetInstance.volume).toBe(0.5); }); - it('target() throws when not attached', () => { + it('task throws when not attached', async () => { const feature = defineFeature()({ - state: ({ target }) => ({ + state: ({ task }) => ({ value: 0, doSomething() { - target(); + return task(() => {}); }, }), - getSnapshot: () => ({ value: 0 }), - subscribe: () => {}, }); const store = createStore({ features: [feature] }); - expect(() => store.doSomething()).toThrow('NO_TARGET'); + await expect(store.doSomething()).rejects.toThrow('NO_TARGET'); }); }); diff --git a/packages/store/src/core/tests/state.test.ts b/packages/store/src/core/tests/state.test.ts index 7d13b3d3..60652c6c 100644 --- a/packages/store/src/core/tests/state.test.ts +++ b/packages/store/src/core/tests/state.test.ts @@ -23,12 +23,6 @@ describe('createState', () => { expect(state.current.muted).toBe(false); }); - it('reflects changes after set', () => { - const state = createTestState(); - state.set('volume', 0.5); - expect(state.current.volume).toBe(0.5); - }); - it('reflects changes after patch', () => { const state = createTestState(); state.patch({ volume: 0.5, muted: true }); @@ -37,24 +31,6 @@ describe('createState', () => { }); }); - describe('set', () => { - it('updates a single key', () => { - const state = createTestState(); - state.set('volume', 0.5); - expect(state.current.volume).toBe(0.5); - }); - - it('does not notify if value is the same', () => { - const state = createTestState(); - const listener = vi.fn(); - state.subscribe(listener); - - state.set('volume', 1); // same as initial - flush(); - expect(listener).not.toHaveBeenCalled(); - }); - }); - describe('patch', () => { it('updates multiple keys', () => { const state = createTestState(); @@ -81,7 +57,7 @@ describe('createState', () => { const listener = vi.fn(); state.subscribe(listener); - state.set('volume', 0.5); + state.patch({ volume: 0.5 }); expect(listener).not.toHaveBeenCalled(); await Promise.resolve(); @@ -93,21 +69,21 @@ describe('createState', () => { const listener = vi.fn(); state.subscribe(listener); - state.set('volume', 0.5); + state.patch({ volume: 0.5 }); expect(listener).not.toHaveBeenCalled(); flush(); expect(listener).toHaveBeenCalledOnce(); }); - it('batches multiple mutations into one notification', () => { + it('batches multiple patches into one notification', () => { const state = createTestState(); const listener = vi.fn(); state.subscribe(listener); - state.set('volume', 0.5); - state.set('muted', true); - state.set('currentTime', 10); + state.patch({ volume: 0.5 }); + state.patch({ muted: true }); + state.patch({ currentTime: 10 }); flush(); expect(listener).toHaveBeenCalledOnce(); @@ -118,12 +94,12 @@ describe('createState', () => { const listener = vi.fn(); const unsub = state.subscribe(listener); - state.set('volume', 0.5); + state.patch({ volume: 0.5 }); flush(); expect(listener).toHaveBeenCalledOnce(); unsub(); - state.set('volume', 0.3); + state.patch({ volume: 0.3 }); flush(); expect(listener).toHaveBeenCalledOnce(); // still 1 }); diff --git a/packages/store/src/core/tests/store.test.ts b/packages/store/src/core/tests/store.test.ts index a5e27392..5401051e 100644 --- a/packages/store/src/core/tests/store.test.ts +++ b/packages/store/src/core/tests/store.test.ts @@ -31,14 +31,15 @@ describe('store', () => { }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - target.addEventListener('volumechange', update); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', update); + target.removeEventListener('volumechange', sync); }); }, }); @@ -65,8 +66,10 @@ describe('store', () => { }); }, }), - getSnapshot: ({ target }) => ({ paused: target.paused }), - subscribe: () => {}, + + attach({ target, set }) { + set({ paused: target.paused }); + }, }); describe('creation', () => { @@ -234,8 +237,6 @@ describe('store', () => { }); }, }), - getSnapshot: () => ({ value: 0 }), - subscribe: () => {}, }); const store = createStore({ @@ -272,8 +273,6 @@ describe('store', () => { }); }, }), - getSnapshot: () => ({ value: 0 }), - subscribe: () => {}, }); const store = createStore({ @@ -306,8 +305,6 @@ describe('store', () => { }); }, }), - getSnapshot: () => ({ value: 0 }), - subscribe: () => {}, }); const store = createStore({ @@ -404,8 +401,6 @@ describe('store', () => { }); }, }), - getSnapshot: () => ({ value: 0 }), - subscribe: () => {}, }); const store = createStore({ diff --git a/packages/store/src/lit/tests/create-store.test.ts b/packages/store/src/lit/tests/create-store.test.ts index 55233b84..dc8d176b 100644 --- a/packages/store/src/lit/tests/create-store.test.ts +++ b/packages/store/src/lit/tests/create-store.test.ts @@ -23,15 +23,15 @@ describe('createStore', () => { }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - const handler = () => update(); - target.addEventListener('volumechange', handler); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', handler); + target.removeEventListener('volumechange', sync); }); }, }); diff --git a/packages/store/src/lit/tests/test-utils.ts b/packages/store/src/lit/tests/test-utils.ts index 593f642d..54f7e461 100644 --- a/packages/store/src/lit/tests/test-utils.ts +++ b/packages/store/src/lit/tests/test-utils.ts @@ -54,15 +54,15 @@ export const audioFeature = defineFeature()({ }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - const handler = () => update(); - target.addEventListener('volumechange', handler); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', handler); + target.removeEventListener('volumechange', sync); }); }, }); @@ -97,15 +97,15 @@ export const customKeyFeature = defineFeature()({ }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - const handler = () => update(); - target.addEventListener('volumechange', handler); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', handler); + target.removeEventListener('volumechange', sync); }); }, }); diff --git a/packages/store/src/react/hooks/tests/test-utils.ts b/packages/store/src/react/hooks/tests/test-utils.ts index 453faf7c..473a0de9 100644 --- a/packages/store/src/react/hooks/tests/test-utils.ts +++ b/packages/store/src/react/hooks/tests/test-utils.ts @@ -29,14 +29,15 @@ export const audioFeature = defineFeature()({ }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - target.addEventListener('volumechange', update); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', update); + target.removeEventListener('volumechange', sync); }); }, }); @@ -87,15 +88,15 @@ export const asyncAudioFeature = defineFeature()({ }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - const handler = () => update(); - target.addEventListener('volumechange', handler); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', handler); + target.removeEventListener('volumechange', sync); }); }, }); @@ -142,15 +143,15 @@ export const customKeyFeature = defineFeature()({ }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - const handler = () => update(); - target.addEventListener('volumechange', handler); + + attach({ target, signal, set }) { + const sync = () => set({ volume: target.volume, muted: target.muted }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', handler); + target.removeEventListener('volumechange', sync); }); }, }); diff --git a/packages/store/src/react/tests/context.test.tsx b/packages/store/src/react/tests/context.test.tsx index aa606707..2b1de2a2 100644 --- a/packages/store/src/react/tests/context.test.tsx +++ b/packages/store/src/react/tests/context.test.tsx @@ -17,8 +17,9 @@ describe('context', () => { state: () => ({ volume: 1, }), - getSnapshot: ({ target }) => ({ volume: target.volume }), - subscribe: () => {}, + attach({ target, set }) { + set({ volume: target.volume }); + }, }); describe('useStoreContext', () => { diff --git a/packages/store/src/react/tests/create-store.test.tsx b/packages/store/src/react/tests/create-store.test.tsx index 54450673..f9675dbe 100644 --- a/packages/store/src/react/tests/create-store.test.tsx +++ b/packages/store/src/react/tests/create-store.test.tsx @@ -31,14 +31,18 @@ describe('createStore', () => { }); }, }), - getSnapshot: ({ target }) => ({ - volume: target.volume, - muted: target.muted, - }), - subscribe: ({ target, update, signal }) => { - target.addEventListener('volumechange', update); + attach({ target, signal, set }) { + const sync = () => + set({ + volume: target.volume, + muted: target.muted, + }); + + sync(); + + target.addEventListener('volumechange', sync); signal.addEventListener('abort', () => { - target.removeEventListener('volumechange', update); + target.removeEventListener('volumechange', sync); }); }, });