Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cjpillsbury
changed the title
Contributor
📦 Bundle Size Report🎨 @videojs/html
Presets (7)
Media (8)
Players (3)
Skins (17)
UI Components (23)
Sizes are marginal over the root entry point. ⚛️ @videojs/react
Presets (7)
Media (7)
Skins (14)
UI Components (19)
Sizes are marginal over the root entry point. 🧩 @videojs/core
Entries (8)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf
Entries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
Replaces setupTextTracks, syncTextTrackModes, and syncSelectedTextTrackFromDom with a single syncTextTracks function implementing a 5-state FSM (preconditions-unmet → setting-up → set-up → destroying → destroyed). Each FSM state is managed by a dedicated effect; entry/exit actions map to the effect body and its returned cleanup function respectively. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Empirical testing confirmed cues added via addCue() survive a disabled → showing mode transition when no src is set on the <track> element. The textBufferState clearing on deselect in the old syncSelectedTextTrackFromDom was therefore unnecessary. Removes the textBufferState field and TextTrackBufferState import from sync-text-tracks.ts, and adds a browser test documenting the verified cue preservation behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
setupTextTracks, syncTextTrackModes, and syncSelectedTextTrackFromDom are fully replaced by syncTextTracks. Removes the six source and test files, and drops the now-incorrect src assertion from the playback-engine test (track elements no longer have src set). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….textTracks lookup loadTextTrackCues no longer requires a pre-built Map<id, HTMLTrackElement> in owners. It now holds a mediaElement reference and looks up the native TextTrack by id via Array.from(mediaElement.textTracks).find(t => t.id === trackId). syncTextTracks no longer builds or writes the map. PlaybackEngineOwners no longer carries the textTracks field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…/re-add change events on TextTrackList are queued as tasks (async per spec), so syncModes inside onChange cannot cause re-entrant calls. The syncTimeout guard is sufficient; removing and re-adding the listener on each settling- window change event was unnecessary complexity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…wns cue operations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve textBufferState from state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to Actor/Reactor pattern Implements TextTrackSegmentLoaderActor as a proper SignalActor using Task/SerialRunner primitives, and simplifies loadTextTrackCues to a thin Reactor that delegates all async work to the Actor layer. Key decisions: - SerialRunner chosen over ConcurrentRunner — VTT segments are small, serial execution keeps the generation-counter pattern simple - Empty object context — all loaded-segment bookkeeping lives in TextTracksActor, not the loader - untrack() wraps textTracksActor.snapshot.get() inside send() to prevent the Reactor's effect from subscribing to the TextTracksActor snapshot, which would cause spurious re-triggers on every cue add Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rceBufferActor
Introduces CueSegmentMeta (id + startTime + duration + trackId) on
AddCuesMessage, mirroring AppendSegmentMeta on AppendSegmentMessage.
TextTracksActorContext.segments now stores full segment timing instead of
bare { id } records.
This lets TextTrackSegmentLoaderActor pass context.segments[trackId]
directly to getSegmentsToLoad, eliminating the id→object reconstruction
and the redundant id-based double-filter that worked around the mismatch.
Also removes the distinct 'done' status (collapses back to 'idle') and
the redundant #destroyed flag (status signal is the source of truth).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…derActor Exposes the runner's current chain tail as a read-only `settled` getter. Capturing it after scheduling a batch and comparing identity in the resolution callback replaces the #loadGeneration counter, Promise.all collection, and .catch() in TextTrackSegmentLoaderActor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Narrows bufferedSegments to Pick<Segment, 'startTime' | 'duration'> — the only fields the function actually uses. Removes the assumption that callers must hold full Segment objects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces `createActor` — a declarative factory for building message-driven actors with finite state, per-state message handlers, an optional task runner, and automatic `onSettled` transitions. `SerialRunner.whenSettled(callback)` owns the generation-token logic internally, eliminating the need for a runner wrapper or scheduling-tracking flag in the actor. The raw runner is passed directly to handlers. Design documented in `internal/design/spf/actor-reactor-factories.md`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tory Replaces the class with a `createTextTrackSegmentLoaderActor` factory function using the new `createActor` primitive. The `runner.settled` promise + manual generation-token pattern is replaced by `onSettled: 'idle'` at the state level, delegating all generation-token logic to `SerialRunner.whenSettled`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the TextTracksActor class with a createTextTracksActor factory function using the createActor primitive. Context mutations via direct signal updates become setContext() calls; mediaElement is captured in the factory closure rather than held as a private field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…formance for ConcurrentRunner - Merge SettledRunnerLike into RunnerLike — whenSettled is now required on all runners - Remove hasWhenSettled type guard; simplify onSettled check in createActor - Add whenSettled, destroy, and settled-promise tracking to ConcurrentRunner - Change ConcurrentRunner.schedule() to return Promise<TValue>; return in-flight promise on dedupe - Add ConcurrentRunner tests: whenSettled, destroy, deduplication returns same promise Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces createReactor — the declarative counterpart to createActor for
signal-driven reactive logic. Each state holds an array of effect functions
(one independent effect() per element) with its own dependency tracking and
cleanup lifecycle. Snapshot shape matches actors: { status, context }.
'destroying' and 'destroyed' are implicit terminal states; destroy() transitions
through both synchronously, leaving room for async teardown in the future.
Migrates syncTextTracks to createReactor, replacing four named effect closures
with a declarative state definition. Moves selectedTextTrackId reset from the
mode-sync effect to the state-guard exit cleanup (correct cohesion). Return type
changes from () => void to Reactor for future status observability; engine
cleanups forEach updated to dispatch by shape.
Documents segment-actor migration assessment in .claude/plans/spf/.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tatus
States are now preconditions-unmet → idle → resolving → resolved, with
mutually-exclusive conditions derived by deriveStatus(). Each state has a
single condition monitor that reads derivedStatusSignal and transitions
directly to whatever state conditions currently dictate — including
mid-resolve condition changes. resolving additionally carries a fetch task
that returns its AbortController so createReactor aborts it on exit.
createReactor now supports { abort(): void } as an effect return type.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After update(state, { presentation: parsed }), derivedStatusSignal
becomes 'resolved' automatically, which triggers the resolving condition
monitor to transition. No need for the fetch task to call transition
directly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the manual signal() + update() plumbing with createMachineCore,
consistent with createActor and createReactor. getState() replaces
inline snapshotSignal.get().value reads; transition() replaces
update(snapshotSignal, { value }). No behaviour change.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add BatchMessage = { type: 'batch'; messages: IndividualSourceBufferMessage[] }
to the SourceBufferMessage union and handle it in send() alongside the
existing individual message types. Remove the separate batch() method
from the interface and implementation — send() is now the single entry
point for all SourceBuffer operations.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…via waitForIdle SourceBufferActor.send() now returns void. SegmentLoaderActor sequences SourceBuffer operations by watching snapshot state transitions via a waitForIdle() helper (effect-based promise) instead of awaiting the send() return value. Removes SourceBufferActorError — errors are logged and the actor self-heals to idle. Tests updated to use vi.waitFor() for actor settling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…into message SourceBufferActor is now built with createActor<'idle'|'updating', ...>. The 'updating' state uses onSettled:'idle' to auto-transition after tasks settle. Each SourceBufferMessage carries its own AbortSignal as a message field rather than a separate send() argument, making the public API send(message):void and consistent with the rest of the actor taxonomy. Call sites in SegmentLoaderActor and tests updated accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l comparison Documents the SPF vs XState behavioral divergence around where async work "belongs" (idle handler vs updating-on-entry invoke), consequences for atomicity/lifecycle/partial-updates, tradeoffs of adopting the XState model, and the state-scoped runner as a middle-ground open question. Updates the runner lifetime section to reflect that state-scoped runners are a revisit candidate rather than a firm rejection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in batch Adds getContext() — a live untracked read — to HandlerContext alongside the existing context snapshot. Tasks scheduled on the runner use getCtx: getContext so each task reads the context committed by the previous one, rather than the stale dispatch-time snapshot. This eliminates the workingCtx threading pattern in SourceBufferActor's batch handler: every task now calls setContext on completion, publishing its result immediately. Context accurately reflects what has actually been appended as it happens, consistent with the existing onPartialContext model for streaming. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Align task factory parameter names with HandlerContext.getContext for consistency across the actor API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Aligns MessageTaskOptions with HandlerContext naming — both now use setContext — removing the partial-specific name that implied a narrower contract than the function actually has. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the Open Questions entry with a documented decision: actor-lifetime runners stay for now. Explains why state-scoped runners require more than convention (onEnter + context side channel), and the condition under which we'd revisit. Also removes the Option C code example from the XState middle-ground section and cleans up stale getCtx/onPartialContext references. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cancellation is now an explicit actor message rather than an out-of-band signal threaded through every send(). SourceBufferActor gains a `cancel` handler in `updating` that calls runner.abortAll(); SegmentLoaderActor sends cancel only when preempting with a track switch or segment abort, preserving the same-track-seek init-commit optimisation without the appendSignal hack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the three identical append-init/append-segment/remove handler bodies with a shared onMessage closure, using HandlerContext to type the parameters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Runner.abortPending() - Add `abortPending()` to SerialRunner: aborts only queued tasks without touching the in-flight task, enabling the "continue" case in load handlers - Migrate createSegmentLoaderActor from CallbackActor closure to createActor with idle/loading states and SegmentLoaderActorContext tracking in-flight init and segment IDs - Replace executeLoadTask/runScheduled/abortController closure state with makeLoadTask factory + SerialRunner scheduling - loading.on.load: abortPending() for continue (in-flight still needed), abortAll() + optional cancel message for preempt (track switch / seek away) - On unexpected fetch errors, abort pending tasks so a failed init does not allow segment fetches to proceed with no init segment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r migration Replace stale "~50 lines of abort signal management" claim with the actual mechanism: explicit runner.abortAll/abortPending in handlers + cancel message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents message dispatch and double-destroy after the actor is torn down, matching the destroy pattern used by createMachineActor and createTransitionActor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r/createMachineReactor Makes the FSM nature of these factories explicit, distinguishing them from createTransitionActor (reducer-style, no state machine) and manual CallbackActor implementations. Renames files, exports, all consumers, tests, and design doc references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace stale actor examples in actor-reactor-factories.md with SourceBufferActor and SegmentLoaderActor (the actual consumers) - Add actor/reactor type taxonomy table documenting all four patterns - Fix HandlerContext shape (was missing getContext) - Update primitives.md: SourceBufferActor migrated, factory not class, monitor/entry/reactions terminology, entry/reactions now decided - Update text-track-architecture.md: fix actor descriptions, code examples to use monitor/entry/reactions, mark implemented futures - Update decisions.md and signals.md: always → monitor terminology - Update actor-migration-assessment.md: mark completed migrations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `effects` name directly maps to the underlying signal effect() primitive, making it immediately clear how tracked re-runs work. The `entry` / `effects` pairing is natural: entry is the special untracked one-time thing, effects are normal reactive signal effects. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cjpillsbury
force-pushed
the
feat/spf-architecture-reactors-texttracks
branch
from
abdf20b to
98c7b24
Compare
Use "reactive" for signals-based snapshots/context/state (the SPF primitive) and reserve "observable"/"Observable" for the RxJS/TC39 Observable pattern discussed as an alternative. Adds glossary and structure note to spf/index.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nitiated test Use function form of `update()` in `createMachineCore` transition to satisfy generic `Partial<Snapshot>` constraint. Omit `url` property instead of setting it to `undefined` to satisfy `exactOptionalPropertyTypes`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c2d19b4. Configure here.
| .catch((error) => { | ||
| if (error instanceof Error && error.name === 'AbortError') return; | ||
| throw error; | ||
| }); |
There was a problem hiding this comment.
Reactor stuck in 'resolving' after fetch error
Medium Severity
The resolving entry effect has no recovery path after a non-abort fetch error. The old effect-based code recovered because .finally() reset a resolving flag, allowing the effect to re-fire on the next state change (since state was tracked inside the effect body). The new reactor stays permanently stuck in 'resolving' because deriveState returns 'resolving' for any unresolved URL with shouldResolve met, and entry effects only run on state entry — not re-entry to the same state. A subsequent URL change (replacing one unresolved presentation with another) produces the same derived state, so no transition fires and no new fetch starts.
Reviewed by Cursor Bugbot for commit c2d19b4. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1158
Summary
Uses SPF's text track implementation as the proving ground for the Actor/Reactor architecture described in
internal/design/spf/primitives.md. The previous text track code bypassed the Task/TaskRunner/Actor/Reactor primitives — this branch rebuilds it from the ground up, producing reference implementations that inform the eventual media (video/audio) revisit.What changed
New primitives and factories:
createMachineActor— declarative FSM-based actor factory with per-state message handlers, optional runner, and reactivesnapshotcreateTransitionActor— reducer-style actor for reactive context without FSM statescreateMachineReactor— signal-driven reactor factory withmonitor,entry/effectssplit, and reactive snapshotcreateMachineCore— shared FSM kernel (snapshot signal, transition, state reader) consumed by both factoriesMachinetype hierarchy (MachineSnapshot,ActorSnapshot,Machine,SignalActor)SerialRunner.whenSettled()/SerialRunner.abortPending()for coordinated task lifecycleText track Actor/Reactor implementations:
TextTracksActor— manages the nativeTextTrackList(modes, cues, loaded/segments context) viacreateTransitionActorTextTrackSegmentLoaderActor— plans and executes VTT segment fetches viaCallbackActorinterfacesyncTextTracks— FSM reactor consolidating the three previous text track sync functions into a single reactor with bidirectional syncloadTextTrackCues— reactor coordinating segment loading via Actor messagesMigrations to new factories:
SourceBufferActor→createMachineActorSegmentLoaderActor→createMachineActorresolvePresentation→createMachineReactor(4-state FSM)trackPlaybackInitiated→createMachineReactorState cleanup:
textBufferStatefromPlaybackEngineState(now owned byTextTrackSegmentLoaderActor)owners.textTracksmap in favor ofmediaElement.textTrackslookupsetupTextTracks,syncTextTrackModes,syncSelectedTextTrackFromDomDesign docs:
actor-reactor-factories.md— decided factory API, XState comparison, open questionssignals.md— decision rationale, tradeoffs, friction pointstext-track-architecture.md— reference implementation assessmentprimitives.mdandindex.md(glossary, terminology disambiguation)Bug fixes:
TextTrackSegmentLoaderActormachine.tsgeneric constraint,exactOptionalPropertyTypesin test)Test plan
pnpm -F @videojs/spf test— all SPF tests passpnpm typecheck— clean across repopnpm lint— no new violationsdestroy()tears down all actors and aborts in-flight work🤖 Generated with Claude Code
Note
Medium Risk
Introduces new core state-machine factories and changes task runner scheduling/settlement semantics, which can subtly affect async ordering and teardown across playback features.
Overview
Adds declarative state-machine primitives to SPF:
createMachineActor,createTransitionActor, andcreateMachineReactor, backed by a sharedcreateMachineCore/Machinesnapshot type, and updatesSignalActor/CallbackActortyping aroundsnapshot.value+context.Extends
SerialRunner/ConcurrentRunnerwithwhenSettled()and improves cancellation controls (SerialRunner.abortPending()), enabling actors to auto-return via per-stateonSettledand to differentiate continue vs preempt behaviors.Migrates
resolvePresentationfrom ad-hoc effects to acreateMachineReactorFSM with abortable fetch on state exit, updates dependent tests, and aligns call sites likeend-of-streamto the newsnapshot.valuefield. Also includes substantial SPF design documentation additions/updates describing the factories, signals decision, and migration notes.Reviewed by Cursor Bugbot for commit c2d19b4. Bugbot is set up for automated code reviews on this repo. Configure here.