Skip to content

refactor(watch)!: narrow Sync's latency input to Paced - #3061

Closed
kixelated wants to merge 1 commit into
devfrom
claude/narrow-sync-latency-3052
Closed

refactor(watch)!: narrow Sync's latency input to Paced#3061
kixelated wants to merge 1 commit into
devfrom
claude/narrow-sync-latency-3052

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Closes #3052. Follow-up to #3048, which added latency="instant" and deliberately left this out because it targeted main.

Why

Sync accepted the full Latency union, including "instant", but has no way to honor it. Not pacing video and disabling audio both happen in MoqWatch, so this compiled and did nothing of the sort:

const sync = new Sync({ latency: "instant" });
new Video.Decoder(source, sync, { enabled });

The clock still paced video, and an independently wired audio decoder stayed enabled. latencyBounds resolved "instant" to zero bounds, so it degraded to a zero jitter buffer rather than breaking, but that isn't the mode that was asked for. The restriction was documented in a comment; now it's in the type.

What changed

-	latency: Getter<Latency>;
+	latency: Getter<Paced>;

plus the matching constructor default cast, and the Latency doc comment restored to the accurate "Sync takes the narrower Paced and never sees it".

Paced was already exported by js/watch/src/sync.ts, so the type side is a three-line change.

Call sites

Narrowing an input rejects callers passing a Signal<Latency>. Both in-tree ones were over-wide in the same way:

  • js/moq-boy/src/game.ts — its public latency knob was typed Watch.Latency, so a caller could set "instant" and get nothing. Same bug, one layer out.
  • js/watch/src/sync.test.ts — same annotation.

Reviewer notes

  • Targets dev: narrowing an input is a semver break in a published package.
  • moq-boy's knob narrows too. It's an app rather than a library, and the alternative is leaving it advertising a mode it can't perform.
  • just fix, just check, and just test all pass.

(Written by claude-opus-5)

`Sync` accepted the full `Latency` union but had no way to honor `"instant"`: not
pacing video and disabling audio both happen in `MoqWatch`. A consumer wiring one up
directly got a clock that still paced video and audio that stayed enabled, with only
a doc comment saying otherwise.

Take `Paced` instead, so a value the component can't implement stops typechecking
rather than being ruled out in prose.

`moq-boy`'s latency knob had the same over-wide annotation, advertising a mode it
never implemented. Closes #3052.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Superseded by #3068, which takes the simpler route: Sync now honors "instant" itself rather than being kept away from it by the type system. That deletes Paced, DecoderInput.paced, and the element's mapping signal, so there is nothing left for this PR to narrow.

(written by claude-opus-5)

@kixelated kixelated closed this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant