docs/spf fundamental concepts#1396
Merged
cjpillsbury merged 54 commits intomainfrom Apr 28, 2026
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. |
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (8)
Players (3)
Skins (29)
UI Components (25)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (7)
Skins (26)
UI Components (20)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (9)
🏷️ @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 |
cjpillsbury
force-pushed
the
docs/spf-fundamental-concepts
branch
from
b77539d to
b801add
Compare
createPlaybackEngine is now a protocol-agnostic engine that composes arbitrary features. HLS-specific state, owners, config, and feature wiring move to createHlsPlaybackEngine in a new hls-engine module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces an explainer document covering the engine composition model, reactive primitives (signals, effects), features, actors/reactors, and the task system. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Alternative to the existing explainer that teaches SPF concepts iteratively through progressive examples building on a counter app: State → Owners/Effects → Tasks. Each section introduces one concept with a copy-pasteable example. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refactor createPlaybackEngine to automatically infer the combined state, owners, and config types from the features passed to it. Features declare their requirements via parameter types; the engine computes the intersection — no explicit type annotations needed at the call site. Adds InferFeatureState/Owners/Config and ResolveFeatureState/Owners/Config utility types. Includes type tests (expectTypeOf) and type error tests (vitest typecheck with @ts-expect-error) covering inference, composition, conflict detection, and invalid option/write rejection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reject incompatible feature compositions at the type level: - State/config: detect when intersecting feature requirements produces `never` or `undefined` fields (e.g. one feature expects `count?: number`, another expects `count?: string`). - Owners: pairwise subtype check — shared owner keys must have types in an extends relationship. Catches class hierarchy mismatches like `HTMLCanvasElement` vs `HTMLVideoElement` while allowing valid subtype compositions like `HTMLElement` + `HTMLVideoElement`. Features that omit a channel (state, owners, or config) compose freely with features that declare it. Error messages surface as string literal types (e.g. 'Error: features have conflicting state types'). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the single-feature overload (features must be passed as an array) and the untyped implementation overload. The function is now a single generic signature with resolved types used directly in the implementation body. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Features declare their minimum contract (e.g. `{ count?: number }`)
without needing to anticipate whether external code will reset fields
to undefined. This is a composition-level concern — on source switches,
the engine or external code commonly resets state/owners fields to
undefined, and feature authors shouldn't have to annotate | undefined
for fields they never reset themselves.
Adds tests confirming optional state and owners fields can be reset
to undefined via update() and set().
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The composition model is general-purpose — not limited to playback engines. Rename to reflect that SPF compositions can be used for media processing pipelines, validation, auditing, server-side processing, etc. - createPlaybackEngine → createComposition - PlaybackEngine → Composition - PlaybackEngineOptions → CompositionOptions - Backward-compat re-exports for old names in index.ts - HLS-specific names (createHlsPlaybackEngine, etc.) unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… content Rename to fundamentals.md and focus purely on SPF's core architecture: compositions, features, state, owners, config, effects, tasks, reactors, and actors. Remove the HLS engine section, media-specific examples, and domain-specific open questions. The document now teaches SPF concepts without assuming any particular domain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce reactors through a pausable counter with DOM button features. Demonstrates monitor-driven state machine transitions, entry/cleanup lifecycle, listen() utility for effect-scoped event listeners, and the contrast between reactors (lifecycle) and effects (simple observation). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Behaviors are deliberately more granular than features — each handles a single concern rather than encapsulating a cohesive feature set. This distinction matters because SPF compositions break up what you'd call a "feature" into multiple decoupled behaviors for easier composition, replacement, and extension. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SPF behaviors are deliberately more granular than features — each handles a single concern. What you'd call a "feature" (e.g. track selection) is split into multiple decoupled behaviors for easier composition, replacement, and extension. - Feature → Behavior, FeatureCleanup → BehaviorCleanup, etc. - InferFeature* → InferBehavior*, ResolveFeature* → ResolveBehavior* - Backward-compat re-export: Behavior as Feature in index.ts - HLS engine comments updated; directory names unchanged Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… index Drop Feature, PlaybackEngine, createPlaybackEngine, and old HLS aliases. No downstream consumers need the old names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce actors by refactoring the persist behavior to use a message- driven save actor. Demonstrates: message-based interface (save, cancel), stateful task coordination via SerialRunner, observable snapshot for render to show save status, preemption on reset, and onSettled auto-transitions. Frames the distinction as "reactors decide when, actors handle how." Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the duplicated fetch task into a makeSaveTask() function, reducing noise in the actor state handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The narrative should be additive — each section builds on the previous composition rather than dropping pieces. The reactor section now carries forward persist alongside the new counter reactor and button behaviors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Break the State section into what/when bookends, a signals primer covering effect/computed, and per-concept subsections for createComposition, the behavior signature, initialState, config, reading/writing state, outside usage, and cross-behavior TypeScript guardrails. Fixes the prior example that passed a behavior function instead of an array to createComposition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promote the composition-level story (createComposition, behavior as type source, type derivation, outside usage) out of state and into a dedicated Compositions section. Introduce a minimal defineCount behavior there as a type source, and upgrade it to a real ticking counter in the slimmed State section, which now focuses on initialState, config, and reading/writing state inside a behavior. Cross-behavior type guardrails are deferred to the Owners section, where multiple behaviors first appear organically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace doc-structure narration ("this section", "the rest of this
section", "covered below", "the previous section", etc.) with direct
prose or deletions so the content carries its own shape. Also trim the
types-flow sentence to avoid namedropping initialState, initialOwners,
and config before those options have been introduced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Give Compositions the same show-then-drill-in shape as State: - Add "A composition in action" subsection with a full lead example (behavior + read + effect + computed + setInterval + cleanup) that the remaining subsections break down - Add what/when summary pair to the opener - Rework "Creating a composition" to use the typed composition from the lead example instead of a hypothetical empty one - Merge "Giving state a shape" and "How types flow" into a single slimmed subsection covering behavior contract, inherited types, boundary errors, empty-case contrast, and stability note - Replace the simple outside update() call with a setInterval that motivates why ongoing work belongs inside a behavior; fold every outside cleanup (effect, interval) into one cleanup block - Bridge into State with a paragraph that picks up the setInterval pain as the motivation for internalizing logic as a behavior Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop "Reading and writing state inside a behavior" — its content is about signal operations and the update() convenience rather than state specifically, so move a compressed update() intro alongside the TC39 Signals line in Compositions' "Creating a composition" where signal vocabulary is first introduced. Drop "When to reach for state" — the what/when pair at the top of State already makes the claim, Compositions has no equivalent closer, and the state-vs-owners distinction belongs in Owners' opener rather than as a trailing recap here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Give Owners the same show-then-drill-in shape as Compositions/State:
- Open with narrative that picks up the prior examples ("a ticking
counter and a console log aren't much of an application") and
introduces resources as the category plus owners as the channel
- Add a what/when summary pair
- Add a NOTE flagging that the "owners" name is provisional, with
"resources" named as a candidate under consideration
- Replace the single long example with three subsections: "A
DOM-renderer behavior" (typed counter + render lead example),
"initialOwners" (seed option with TS check), "Composing behaviors"
(cross-behavior guardrails covering both the state/config
intersection rule and the owners subtype rule deferred from the
Compositions section)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously, Tasks/Actors inserted new behaviors mid-list, and the two Advanced sections branched back to a pre-persist composition. Now every running example appends its new behaviors at the end, and Advanced picks up from where Actors left off — mount creates savingElement alongside the other descendants so renderSaving has its target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lity Add two discussions to the Creating owners within behaviors section: why each descendant is registered under its own key (vs. a MutationObserver over rootElement) and how the typed-owners + guards contract makes mount a swappable unit. Drop the media-engine analog paragraph; the swappability note already carries the generalization and the doc otherwise stays on the counter example. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The section opened with a generic observation about behaviors only reading from owners so far. Replace with a concrete call-back to the Actors composition's initialOwners block — four elements handed over by id, any rename forcing caller changes — so the motivation lands the way every prior section's opening does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After every behavior cleanup finishes, createComposition now resets the owners signal to an empty object. Behaviors no longer have to unregister the keys they wrote — the composition clears them as part of teardown. Update the mount example in the fundamentals doc to match: cleanup now only detaches the DOM nodes, and the prose explains that the composition itself handles owners clearing. Add engine.test.ts covering the new behavior: owners populated by a behavior, via initialOwners, or across multiple behaviors are all cleared; cleanups still see the pre-destroy owners; async cleanups are awaited before the clear. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… matters In the common real-world shape the composition shares a lifetime with a component that discards rootElement, so the descendants get GC'd without explicit .remove() calls. Drop the cleanup return and replace it with a short comment calling out where cleanup would go if it were needed — .remove() descendants, close a socket, disconnect an observer, close a MediaSource — so the principle stays visible without the defensive code. Update the destroy-path sentence to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Owners / Reactors / Tasks / Actors code blocks all note which behaviors are unchanged from previous sections. The two Advanced sections skipped that note — add them so the convention is consistent across every running example. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the single rootElement positional with a CounterOptions interface that merges state seeds (initialCount, paused) with renamed config (tickIntervalMs, placeholder, autoSaveEveryTicks) and the root element. The constructor spreads caller options over defaults into a readonly #options field, then splits that across createComposition. Getters, reset(), and the countchange event detail all read their fallback from #options too, so defaults live in exactly one place. Update the consumer-side example to pass rootElement, tickIntervalMs, and paused through the options bag; extend the closing prose to call out the translation layer — consumers don't see the state/config/owners split the composition uses internally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reset()'s 0 is the trigger cancelOnReset watches for, not a return-to-initial semantic. Pull reset() back off #options.initialCount and note in prose why the literal belongs to the composition's internal contract, not the caller-facing options. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The single-effect bridge carried a lastCount / lastPaused pair of local let bindings and diffed each field against them — imperative tracking inside a reactive callback. Extract count and paused into their own computed signals, give each its own effect dispatching the matching event, and collect the stop functions in a stops array. Add a code comment at the bridge site and a prose paragraph after the options discussion so the "one effect per derivation" pattern is explicitly the recommended shape and the single-effect-with-diffing alternative is called out as imperative code wearing a reactive costume. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the #stopBridge wrapper + local stops array with a single #teardowns: Array<() => void> | undefined class field, assigned directly to [effect(...), effect(...)] in the constructor. destroy() iterates, clears to undefined for re-destroy safety, then awaits the composition. Prose updated to name the new field in all three relevant paragraphs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both behaviors have non-idempotent effects — persist schedules a save task or sends a save message, cancelOnReset sends a cancel message — so reading count directly out of state re-fires them whenever any unrelated state field changes. Wrap count in a computed so each effect only re-runs when the count itself changes. Add a new "Narrowing what an effect re-runs on" sub-section to the Tasks section introducing the pattern, and a brief bridge sentence from the "Three things are new" paragraph. Call out that the DOM-update effects earlier in the doc (renderCount, pauseButton label) don't need the same treatment because their side effects are idempotent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wrapper's #teardowns array is the same manual-lifecycle shape the doc opened with — setInterval and stopLogging living outside the composition, the caller managing both. The doc's first pivot was moving them inside as behaviors to tie cleanup to composition.destroy(). The bridge could follow the same move: a forwardEvents behavior taking an EventTarget as an owner. Sketch the alternative, then call out why we kept the bridge on the wrapper — dispatching CustomEvent knows about the public API shape, which is adapter work, not composition work. The #teardowns bookkeeping is a small cost paid to keep composition concerns from leaking out. Placed the new sub-section before the closing "This is the Adapter shape" paragraph so the adapter-shape summary lands as a natural conclusion to the line we just drew. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Keep "This is the Adapter shape" as the pattern name — it's one of the few labels the doc hands out — but drop the SpfMedia / HLS example tail to match the discipline the rest of the doc holds around staying on the counter and off playback-engine specifics. Same precedent as the Advanced: Creating owners section, where we dropped the media-engine analog paragraph. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Close the fundamentals doc with a synthesis section covering the four themes the running examples accumulated: - Additive, not rewriting: new capabilities appended as behaviors, with honest exceptions for counter (Reactors) and persist (Actors) that still kept their contracts stable. - Use what you need: each primitive is opt-in, with a short list of when each one earns its keep. - Contracts, not couplings: no behavior imports another; the only interface is the shape of shared signals. - Conventions we leaned on: five recurring patterns indexed with pointers back to where they were introduced. Close with a bookend paragraph that reframes the doc as fundamentals — real compositions scale along these same primitives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructure the package to match the conceptual split established in
docs/fundamentals.md. Top-level src now has two clear homes:
- src/core/: SPF framework primitives
- composition/ (createComposition, Behavior/Composition types,
moved from src/dom/playback-engine/engine.ts)
- signals/ (unchanged)
- tasks/ (moved from core/task.ts)
- actors/ (actor.ts, machine-actor, transition-actor, moved from
sibling files under core/)
- reactors/ (moved from core/create-machine-reactor.ts)
- machine.ts (shared by actors and reactors) stays at core root
- utils/generate-id.ts stays (used by Task)
- src/media/: runtime-agnostic HAS/media-domain pieces (abr, buffer,
hls, features, types, utils/track-selection) — all moved from
src/core/
Expand src/index.ts to re-export the framework primitives so imports
from '@videojs/spf' actually resolve (previously only VERSION was
exported). Drop createComposition, Behavior, Composition, and effect
from the @videojs/spf/playback-engine subpath — they belong on the
main export now. The three public subpaths (., ./dom,
./playback-engine) stay the same; what each exports has shifted to
match the new structure.
Update every import across src/ (dom/features, dom/playback-engine,
dom/media, dom/network, dom/tests, all.ts, media internals, core
internals) to point at the new paths. Update the engine test to use
plain objects instead of DOM elements so it runs in the node 'core'
vitest project alongside the framework primitive it covers, not the
browser 'dom' project. Update fundamentals.md doc imports to point at
@videojs/spf throughout.
BREAKING CHANGE: createComposition, effect, Behavior, and Composition
no longer export from @videojs/spf/playback-engine. Import them from
@videojs/spf instead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two stale imports in the template that the previous spf restructure exposed once Turbo's cache was invalidated: - createPlaybackEngine / PlaybackEngineState → createHlsPlaybackEngine / HlsPlaybackEngineState (renamed in spf commit d9cac1b ages ago; template was never updated) - effect now lives on @videojs/spf (framework primitive front door), not @videojs/spf/playback-engine (HLS-specific subpath) apps/sandbox/src/ is gitignored — the template is source of truth and is what the workspace build actually compiles. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root CLAUDE.md: update the packages/spf row in the Package Layout
table to name both subpaths (/dom, /playback-engine), and expand the
Dependency Hierarchy block to list the three entry points with their
new semantics (framework primitives at root).
internal/design/spf/{architecture,primitives}.md: update file-path
references to match the new layout — core/task.ts → core/tasks/task.ts,
core/actor.ts → core/actors/actor.ts, core/create-machine-actor.ts →
core/actors/create-machine-actor.ts, core/create-machine-reactor.ts →
core/reactors/create-machine-reactor.ts, core/hls|abr|buffer →
media/hls|abr|buffer, dom/playback-engine/engine.ts →
core/composition/engine.ts. Refresh the layer-split paragraph in
architecture.md to reflect the three-way split (core framework
primitives / media domain / dom bindings) instead of the old
core-vs-dom model.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The engine.ts name was a holdover from when createComposition was still called createPlaybackEngine (renamed in d9cac1b). Now that the file lives under core/composition/ alongside the other framework primitive factories, match the established naming pattern: - create-machine-actor.ts → createMachineActor - create-machine-reactor.ts → createMachineReactor - create-transition-actor.ts → createTransitionActor - create-composition.ts → createComposition (was engine.ts) Rename the three test files alongside — create-composition.test.ts, create-composition-types.test.ts, create-composition-types.test-d.ts. Update every importer: src/index.ts, dom/playback-engine/hls-engine.ts, dom/playback-engine/adapter.ts, internal tests, and the path reference in internal/design/spf/architecture.md. No API changes — only the module path moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The src split refactor moved abr, buffer, hls, and media features to src/media/ but left vitest.config.ts with only core and dom projects, silently dropping 16 test files (~225 tests) from the run. Adds a media project mirroring the src layout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cjpillsbury
force-pushed
the
docs/spf-fundamental-concepts
branch
from
11e035c to
e4a865a
Compare
![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 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e4a865a. Configure here.
|
|
||
| // Module-level VTT parser cleanup | ||
| // TODO: this should be owned by loadTextTrackCues | ||
| () => destroyVttParser(), |
There was a problem hiding this comment.
VTT parser cleanup runs at creation, not destruction
High Severity
The behavior () => destroyVttParser() calls destroyVttParser() immediately when the composition is created (since createComposition invokes each behavior as f(deps)), and returns void — so no cleanup is registered. The old engine called destroyVttParser() inside its destroy() callback. The fix is to return a cleanup function: () => () => destroyVttParser(). As written, the singleton dummyVideo is nulled at creation time (a no-op if fresh, but destructive if reusing) and leaked on destroy.
Reviewed by Cursor Bugbot for commit e4a865a. Configure here.
PR #1400 dropped typescript from every package in favor of @typescript/native-preview (tsgo) at the workspace root. vitest's typecheck project still defaulted to the tsc binary, which is no longer installed, so `pnpm -F @videojs/spf test` fails with `Error: spawn tsc ENOENT`. This is the only failing job in CI on this branch and is unrelated to the docs work. vitest's typecheck.checker accepts any binary that understands --noEmit --pretty false --incremental --tsBuildInfoFile; tsgo does, and pnpm puts node_modules/.bin/tsgo on PATH for lifecycle scripts, so 'tsgo' resolves without any new devDependency. Verified against the existing create-composition-types.test-d.ts: clean runs report "Type Errors: no errors" and exit 0, and intentionally broken type tests surface as TypeCheckError with file/line/column parsed from tsgo's tsc-compatible diagnostic output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
decepulis
reviewed
Apr 24, 2026
Collaborator
decepulis
left a comment
decepulis
left a comment
There was a problem hiding this comment.
Pardon the lack of granular comments. I read this twice, though, I promise!
When we take another pass (maybe when we put these on videojs.org), we'll have to really get into jargon and sentence structure. This is a dense read, keeping my brain in high gear on every word; that's part of the reason this review took me a while. Reminds me of pulling through academic papers when writing my thesis. (The other part is that I don't know how to say no to distractions, but that's outside the scope of this review.)
That being said, the structure and content within that structure is golden. This doc has got some great bones and communicates the right things in the right order. This whole iterative counter example is great. And really cool to see the progress since we've last checked in. Imo, good to commit.
|
|
||
| **What it is** — a factory that wires independent behaviors to shared reactive channels and returns a handle for reading state and tearing everything down. | ||
|
|
||
| **When to use it** — when a problem has multiple concerns that share data and lifecycle. Each concern stays a standalone function; shared values flow through signals; cleanup happens together. |
|
|
||
| ### A composition in action | ||
|
|
||
| A composition with one stand-in behavior, driven entirely from outside: |
Collaborator
There was a problem hiding this comment.
Maybe just a quick preamble so people know what to expect. this is the whole code block, but we'll break it down bit by bit after this
| }); | ||
| ``` | ||
|
|
||
| Derived values use `computed`: a read-only signal whose value is a function of other signals. It recomputes lazily — only when something reads it after a dependency has changed. |
Collaborator
There was a problem hiding this comment.
I appreciate that this doesn't assume I know more about signals than I do
Comment on lines
+127
to
+141
| Writes from outside are uncommon — ongoing work almost always belongs in a behavior. Driving `count` on an interval from outside, for example, means you own the interval's lifecycle yourself: | ||
|
|
||
| ```ts | ||
| const id = setInterval(() => { | ||
| update(composition.state, { count: (composition.state.get().count ?? 0) + 1 }); | ||
| }, 250); | ||
| ``` | ||
|
|
||
| Destroying the composition runs each behavior's cleanup and awaits any async work — but anything you started out here is on you: | ||
|
|
||
| ```ts | ||
| await composition.destroy(); // behaviors are torn down | ||
| stopLogging(); // the effect is on you | ||
| clearInterval(id); // the interval is on you | ||
| ``` |
Collaborator
There was a problem hiding this comment.
This might be a place to tease that there are other tools that will help you write in a structured way?
Collaborator
There was a problem hiding this comment.
Ah I mean the first paragraph of the next section kinda addresses this very comment
| const count = computed(() => state.get().count ?? 0); | ||
| ``` | ||
|
|
||
| The reason is the shape of the effect that reads it. `save()` is a non-idempotent side effect: it schedules a network request. A state signal re-notifies on every write, so reading `state.get().count` directly would re-run the effect whenever `paused` toggled (or any unrelated field changed) and fire a save whenever the current count happened to be divisible by `saveEvery`. `computed` caches by value, so reading its `.get()` inside an effect only triggers a re-run when that value actually changed. |
| }, | ||
| }, | ||
| saving: { | ||
| onSettled: 'idle', |
|
|
||
| --- | ||
|
|
||
| ## Advanced: Wrapping a composition in a public API |
Collaborator
There was a problem hiding this comment.
nit: not suuuuure this is "advanced", though it probably does belong here near the end
|
|
||
| More events can be wired the same way — a `saving`/`saved` pair reading `saveActor.snapshot`, a `destroy` event dispatched before teardown, custom events derived from any signal worth surfacing. Each is another `computed` + `effect` pushed onto `#teardowns`. The pattern scales: every piece of composition state that matters to a consumer gets its own projection. | ||
|
|
||
| ### Bridge as a behavior? |
Collaborator
There was a problem hiding this comment.
Could use a 10% more descriptive title. I'm imagining scanning a TOC and not 100% grokking this header
Comment on lines
+1002
to
+1008
| ### Additive, not rewriting | ||
|
|
||
| Every section added new behaviors without touching the ones that came before. `counter` and `logCount` stayed the same from State onward; `renderCount` stayed the same from Owners onward; the buttons from Reactors onward. New capabilities landed as appended behaviors in the composition list, not as edits to existing ones. | ||
|
|
||
| The exceptions are informative. `counter` was redefined in Reactors — its internal `setInterval` became a reactor-managed effect so it could enter and leave a `running` state. `persist` was redefined in Actors — its runner and in-flight flag moved into a dedicated actor, and `persist` itself shrank to a message forwarder. Neither rewrite reached outside the behavior being replaced: `counter` still read and wrote `state.count`; `persist` still triggered on the same Nth-tick condition. The contract each behavior exposed stayed stable; the implementation swapped. | ||
|
|
||
| That's the normal way to extend a composition. Replace a behavior with a differently-shaped version of itself; add new behaviors for new capabilities; don't reach into existing behaviors to adjust them. |
Collaborator
There was a problem hiding this comment.
This feels like a review of your own work, rather than something necessarily useful to the reader.
Not saying that with a lot of conviction. Just. Give it a second thought and see where you land.
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
2 participants
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.
Summary
Adds
packages/spf/docs/fundamentals.md— a 1000-line tutorial-style explainer of SPF's core concepts — and lands the refactors that the doc is written against:createPlaybackEngine→createComposition. The playback engine is now a generic composition of behaviors; HLS is one concrete composition (createHlsPlaybackEngine) built on it.Featureis renamed toBehaviorin types and source to match the docs' vocabulary.src/split into framework primitives and media domain.core/now holds framework primitives (actors/,reactors/,tasks/,composition/); anything streaming-specific (abr/,buffer/,hls/) moved tomedia/.dom/playback-engine/is the DOM/HLS adapter layer.playback-engine/index, owners cleared ondestroy,composition/engine.tsrenamed tocreate-composition.ts, sandbox template updated to the current API, and path refs inCLAUDE.md+ design docs synced to the new layout.The doc is structured Compositions → State → Owners → Reactors → Tasks → Actors, followed by advanced sections (creating owners within behaviors, wrapping a composition in a public API) and a capstone "Bringing it all together." A separate top-level explainer was drafted and pulled back out of this branch to iterate on later (b77539d).
Test plan
pnpm typecheckpnpm -F @videojs/spf test(type tests + unit tests forcreate-composition)pnpm -F @videojs/spf buildpnpm build:sandboxand exercise thespf-segment-loadingtemplatepnpm check:workspacepackages/spf/docs/fundamentals.mdend-to-end; verify code samples compile against the current APINote
Medium Risk
Moderate risk because this is a broad SPF API and module-structure refactor (new
createComposition, new exports, and playback-engine rename) that can break downstream imports and subtly change engine teardown/typing behavior, though runtime logic is largely re-wired rather than rewritten.Overview
SPF is refactored from a monolithic
createPlaybackEngineto a generic, typed composition model. This introducescreateComposition(behaviors + inferredstate/owners/config, conflict detection, and async cleanup) and exports SPF primitives (signals,tasks,actors,reactors, composition types) from the root@videojs/spfentrypoint.The HLS engine is reimplemented as a concrete composition (
createHlsPlaybackEngine) and the oldengine.tsis removed.SpfMediaMixin, sandbox template, and tests are updated to the new HLS engine API/types, anddestroy()now relies on composition cleanup + owners clearing.Code is reorganized to separate framework primitives from media-domain logic. HLS parsing/ABR/buffer/types/features move from
core/tomedia/, actors/reactors/tasks get dedicated subfolders, imports are rewired accordingly, and documentation/design docs are updated; a new large tutorial docdocs/fundamentals.mdis added. Test tooling is adjusted to addmediaand typecheck projects, andtsconfigrelaxesexactOptionalPropertyTypes.Reviewed by Cursor Bugbot for commit f260285. Bugbot is set up for automated code reviews on this repo. Configure here.