Skip to content

Commit d50f376

Browse files
staypuft3claude
authored andcommitted
fix(youtube-video-element): destroy the player on disconnect
The YouTube iframe API keeps every player it creates in module-scoped registries, so removing the element is not enough to release the <iframe>: it stays detached but reachable from `window.YT`, along with everything the player's event handlers close over. `api.destroy()` is currently only reachable from `load()` when `src` is cleared, and the element defines no `disconnectedCallback`, so every mount/unmount cycle leaks a player. In React this shows up as `Detached HTMLIFrameElement` nodes accumulating across route changes with `window.YT` as the retainer. Add `connectedCallback`/`disconnectedCallback` following the pattern already used in vimeo-video-element: destroy the player and reset the load state on disconnect, and reload on reconnect so a DOM move still works (`load()` is otherwise only triggered by attributeChangedCallback, which does not fire on a move). Guard the load itself. `load()` awaits the API script and then `loadComplete`, so an element disconnected while either is in flight would still attach a player, or start pollers for one, afterwards. A #loadId generation counter, bumped on disconnect and on each load, makes the superseded attempt bail out at both points. Track and clear the two polling intervals started at the end of load(). Neither timer id was stored, so the 50ms seek poller was never cancelled and the 100ms progress poller only cancelled itself once fully buffered. Both closures capture `this`, which kept the element (and through it the player and its iframe) alive independently of the registry, and with reload-on-reconnect each remount would stack another pair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent b5221ee commit d50f376

2 files changed

Lines changed: 97 additions & 2 deletions

File tree

packages/youtube-video-element/test/test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,48 @@ test('t parameter - case insensitive', async function (t) {
250250
t.equal(startParam, '171', 'start parameter is set from uppercase T parameter');
251251
});
252252

253+
test('destroys the player when disconnected', async function (t) {
254+
const video = await createVideoElement();
255+
await video.loadComplete;
256+
257+
t.ok(video.api, 'has a player once loaded');
258+
t.ok(video.shadowRoot.querySelector('iframe'), 'has an iframe once loaded');
259+
260+
video.remove();
261+
262+
t.equal(video.api, null, 'the player reference is released on disconnect');
263+
t.equal(video.isLoaded, false, 'the element is no longer marked loaded');
264+
t.equal(
265+
video.shadowRoot.querySelector('iframe'),
266+
null,
267+
'destroy() removed the iframe from the shadow root'
268+
);
269+
});
270+
271+
test('creates a new player when reconnected', async function (t) {
272+
const video = await createVideoElement();
273+
await video.loadComplete;
274+
275+
const firstApi = video.api;
276+
video.remove();
277+
document.body.append(video);
278+
279+
await video.loadComplete;
280+
281+
t.ok(video.api, 'has a player again after reconnecting');
282+
t.ok(video.api !== firstApi, 'a new player was created, not the destroyed one');
283+
t.ok(video.shadowRoot.querySelector('iframe'), 'the iframe was rebuilt');
284+
});
285+
286+
test('disconnecting before the player is ready does not throw', async function (t) {
287+
const video = await createVideoElement();
288+
// Do not await loadComplete: the API may still be loading.
289+
video.remove();
290+
291+
t.equal(video.api, null, 'no player is left behind');
292+
t.ok(true, 'disconnecting mid-load did not throw');
293+
});
294+
253295
function delay(ms) {
254296
return new Promise((resolve) => setTimeout(resolve, ms));
255297
}

packages/youtube-video-element/youtube-video-element.js

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
147147
loadComplete = new PublicPromise();
148148
#loadRequested;
149149
#hasLoaded;
150+
#wasDisconnected = false;
151+
#loadId = 0;
152+
#timers = [];
150153
#readyState = 0;
151154
#seeking = false;
152155
#seekComplete;
@@ -172,6 +175,12 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
172175
async load() {
173176
if (this.#loadRequested) return;
174177

178+
// Identifies this load attempt. `load()` awaits the API script, so the
179+
// element can be disconnected (or asked to load again) while this one is
180+
// still in flight; those bump #loadId and this attempt bails out below
181+
// instead of attaching a player nothing will ever destroy.
182+
const loadId = ++this.#loadId;
183+
175184
if (!this.shadowRoot) {
176185
this.attachShadow({ mode: 'open' });
177186
}
@@ -223,6 +232,10 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
223232
}
224233

225234
const YT = await loadScript(API_URL, API_GLOBAL, API_GLOBAL_READY);
235+
236+
// Superseded by a disconnect or a newer load() while awaiting the API.
237+
if (loadId !== this.#loadId) return;
238+
226239
this.api = new YT.Player(iframe, {
227240
events: {
228241
onReady: () => {
@@ -328,8 +341,14 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
328341

329342
await this.loadComplete;
330343

344+
// Superseded while awaiting loadComplete; don't start pollers for a player
345+
// that is already gone.
346+
if (loadId !== this.#loadId) return;
347+
348+
this.#clearTimers();
349+
331350
let lastCurrentTime = 0;
332-
setInterval(() => {
351+
this.#timers.push(setInterval(() => {
333352
const diff = Math.abs(this.currentTime - lastCurrentTime);
334353
const bufferedEnd = this.buffered.end(this.buffered.length - 1);
335354
if (this.seeking && bufferedEnd > 0.1) {
@@ -341,7 +360,7 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
341360
this.dispatchEvent(new Event('seeking'));
342361
}
343362
lastCurrentTime = this.currentTime;
344-
}, 50);
363+
}, 50));
345364

346365
let lastBufferedEnd;
347366
const progressInterval = setInterval(() => {
@@ -355,6 +374,40 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
355374
this.dispatchEvent(new Event('progress'));
356375
}
357376
}, 100);
377+
this.#timers.push(progressInterval);
378+
}
379+
380+
#clearTimers() {
381+
for (const id of this.#timers) clearInterval(id);
382+
this.#timers = [];
383+
}
384+
385+
connectedCallback() {
386+
// `load()` is only triggered by attributeChangedCallback, so an element that
387+
// is moved in the DOM (disconnected and reconnected without its attributes
388+
// changing) has to ask for a new player itself.
389+
if (this.#wasDisconnected) {
390+
this.#wasDisconnected = false;
391+
this.load();
392+
}
393+
super.connectedCallback?.();
394+
}
395+
396+
disconnectedCallback() {
397+
this.#wasDisconnected = true;
398+
this.#loadId++;
399+
this.#clearTimers();
400+
this.#loadRequested = null;
401+
this.#hasLoaded = null;
402+
this.isLoaded = false;
403+
this.loadComplete = new PublicPromise();
404+
// The YouTube iframe API holds a reference to every player it creates, so
405+
// dropping the element is not enough to release the <iframe>: it stays
406+
// detached but reachable from window.YT. destroy() removes the iframe and
407+
// deregisters the player.
408+
this.api?.destroy?.();
409+
this.api = null;
410+
super.disconnectedCallback?.();
358411
}
359412

360413
async attributeChangedCallback(attrName, oldValue, newValue) {

0 commit comments

Comments
 (0)