Skip to content

runtime: queue Video.play() before loadedmetadata instead of throwing - #412

Merged
TooTallNate merged 4 commits into
mainfrom
fix/video-play-before-metadata
Jul 8, 2026
Merged

runtime: queue Video.play() before loadedmetadata instead of throwing#412
TooTallNate merged 4 commits into
mainfrom
fix/video-play-before-metadata

Conversation

@TooTallNate

Copy link
Copy Markdown
Owner

Fixes #402

Problem

Video.play() rejected with InvalidStateError when called before the video's metadata had loaded, diverging from HTMLMediaElement.play() which never rejects for "not loaded yet". Because play() returns a promise, the common v.play().catch(() => {}) pattern silently swallowed the error, leaving the video frozen on its first frame with no visible failure.

Fix

play() now follows the HTML spec algorithm:

  • Before metadata (or with no source at all): sets paused = false, fires play on the paused→false transition, and returns a promise that resolves once playback actually begins (started from load()'s metadata handler).
  • Never throws synchronously. With no source the promise stays pending until a source arrives — verified empirically against headless Chromium (the resource selection algorithm waits; it does not reject NotSupportedError as the issue suggested).
  • A pending play() is rejected with AbortError by pause() or a superseding load(), and with NotSupportedError on load failure.

Testing

Three new conformance fixtures in test/fixtures/video.ts, all verified passing in both nxjs-test and Chrome on natecube (video: nxjs-test vs Chrome ✓, 44 assertions):

  • eager play() immediately after setting src — resolves and playback advances
  • no-source play() — stays pending, paused flips to false
  • pause() before metadata — pending play() rejects with AbortError

The no-source fixture caught my initial (wrong) NotSupportedError implementation diverging from Chrome, which is exactly what the conformance harness is for.

Video.play() previously rejected with InvalidStateError when called
before the video's metadata had loaded, diverging from
HTMLMediaElement.play() which never rejects for "not loaded yet".
Because play() returns a promise, the common v.play().catch(() => {})
pattern silently swallowed the error, leaving the video frozen on its
first frame with no visible failure.

play() now follows the HTML spec algorithm:
- Before metadata: sets paused = false and returns a promise that
  resolves once playback actually begins (started from load()'s
  metadata handler).
- No source: rejects with NotSupportedError (never throws
  synchronously).
- A pending play() is rejected with AbortError by pause() or a
  superseding load, and with NotSupportedError on load failure.

Fixes #402
Verified empirically against headless Chromium: play() with no source
leaves the promise pending (per the resource selection algorithm's
wait-for-a-source step) and still flips paused to false — it does not
reject. The conformance fixture caught the divergence.
Copilot AI review requested due to automatic review settings July 8, 2026 08:01
@changeset-bot

changeset-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: edf9ee3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@nx.js/runtime Patch
@nx.js/nro Patch
@nx.js/nsp Patch
create-nxjs-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nx-js Ready Ready Preview, Comment Jul 8, 2026 8:40am

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

📝 Runtime Type Changes

This PR modifies the public TypeScript API surface (packages/runtime/dist/index.d.ts):

View type diff
--- base: index.d.ts
+++ pr: index.d.ts
@@ -2836,7 +2836,7 @@
  * const video = new Video();
  * video.loop = true;
  * video.src = 'romfs:/attract.webm';
- * video.addEventListener('canplay', () => video.play());
+ * video.play(); // queued until metadata loads, like in browsers
  *
  * function render() {
  *   ctx.drawImage(video, 0, 0, screen.width, screen.height);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Video runtime API to align Video.play() behavior more closely with browser HTMLMediaElement.play(), specifically by queueing playback (instead of rejecting) when play() is called before loadedmetadata.

Changes:

  • Implement queued play() requests via an internal pendingPlays list and settle them on metadata load, load failure, pause(), or superseding load().
  • Update Video.play() to return a promise without synchronous throws (including the “no metadata yet” case).
  • Add new conformance fixtures to cover eager play(), no-source pending play(), and aborting pending play via pause().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
packages/runtime/src/video.ts Implements pending play() queuing/settlement and adjusts load()/pause() interactions.
packages/runtime/test/fixtures/video.ts Adds conformance tests for play() queuing and abort behavior.
.changeset/video-play-before-metadata.md Adds a patch changeset describing the behavior change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/runtime/src/video.ts Outdated
Comment thread packages/runtime/src/video.ts
Comment thread packages/runtime/test/fixtures/video.ts
@TooTallNate TooTallNate changed the title runtime: queue Video.play() before loadedmetadata instead of throwing runtime: queue Video.play() before loadedmetadata instead of throwing Jul 8, 2026
…ent on transition

Verified against headless Chromium:
- play() with no source, then setting a valid src later: the pending
  promise RESOLVES and playback begins (paused stays false). load()
  now only aborts pending plays / resets paused on a SUPERSEDING load
  (loadSeq > 0), where Chrome does reject with AbortError.
- play() while already playing: no duplicate play event, no redundant
  native call; the promise still resolves.

Adds three conformance fixtures pinning these behaviors in both
engines: no-src play resolved by a later src, superseding load
aborting a pending play, and double-play event count.
@TooTallNate
TooTallNate merged commit 2b6b6cb into main Jul 8, 2026
7 checks passed
@TooTallNate
TooTallNate deleted the fix/video-play-before-metadata branch July 8, 2026 08:46
@github-actions github-actions Bot mentioned this pull request Jul 8, 2026
@waldhari1

Copy link
Copy Markdown

Steady Deployments @TooTallNate 🔥...keep up the good work.

TooTallNate pushed a commit that referenced this pull request Jul 8, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`main` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `main`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @nx.js/runtime@1.0.0-beta.6

### Patch Changes

- fix: cross-context canvas font size leakage — re-pin the shared
FT_Face char_size at the start of `fillText()`, `strokeText()`, and
`measureText()` ([#406](#406))

- feat: `Image`, `Audio`, and `Video` now resolve `globalThis.fetch` at
call time, so embedder-installed `fetch` wrappers (e.g. custom URL
schemes) are honored for `src` loads
([#404](#404))

- fix: `Video.play()` no longer rejects with `InvalidStateError` when
called before `loadedmetadata` — playback is queued and the returned
promise resolves once it actually begins, matching
`HTMLMediaElement.play()`. A pending `play()` is rejected with
`AbortError` by `pause()` or a superseding load, and with
`NotSupportedError` on load failure
([#412](#412))

- fix: install WebGL2 `GL_CONSTANTS` with a single bulk
`Object.defineProperties()` call per target instead of ~740 sequential
`Object.defineProperty()` calls at module scope
([#405](#405))
## create-nxjs-app@1.0.0-beta.6


## @nx.js/nro@1.0.0-beta.6


## @nx.js/nsp@1.0.0-beta.6

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Video.play() throws before loadedmetadata instead of queuing (diverges from HTMLMediaElement)

3 participants