Skip to content

Commit 1c30859

Browse files
staypuft3claude
authored andcommitted
fix(youtube-video-element): destroy the player on disconnect
The YouTube iframe API keeps an internal reference to every player it creates, 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). Also guard the load itself. `load()` awaits the API script, so an element disconnected while that is in flight would still attach a player afterwards, which nothing would ever destroy. A #loadId generation counter, bumped on disconnect and on each load, makes the superseded attempt bail out before constructing the player. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent b5221ee commit 1c30859

2 files changed

Lines changed: 81 additions & 0 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: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
147147
loadComplete = new PublicPromise();
148148
#loadRequested;
149149
#hasLoaded;
150+
#wasDisconnected = false;
151+
#loadId = 0;
150152
#readyState = 0;
151153
#seeking = false;
152154
#seekComplete;
@@ -172,6 +174,12 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
172174
async load() {
173175
if (this.#loadRequested) return;
174176

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

225233
const YT = await loadScript(API_URL, API_GLOBAL, API_GLOBAL_READY);
234+
235+
// Superseded by a disconnect or a newer load() while awaiting the API.
236+
if (loadId !== this.#loadId) return;
237+
226238
this.api = new YT.Player(iframe, {
227239
events: {
228240
onReady: () => {
@@ -357,6 +369,33 @@ class YoutubeVideoElement extends MediaPlayedRangesMixin(globalThis.HTMLElement
357369
}, 100);
358370
}
359371

372+
connectedCallback() {
373+
// `load()` is only triggered by attributeChangedCallback, so an element that
374+
// is moved in the DOM (disconnected and reconnected without its attributes
375+
// changing) has to ask for a new player itself.
376+
if (this.#wasDisconnected) {
377+
this.#wasDisconnected = false;
378+
this.load();
379+
}
380+
super.connectedCallback?.();
381+
}
382+
383+
disconnectedCallback() {
384+
this.#wasDisconnected = true;
385+
this.#loadId++;
386+
this.#loadRequested = null;
387+
this.#hasLoaded = null;
388+
this.isLoaded = false;
389+
this.loadComplete = new PublicPromise();
390+
// The YouTube iframe API holds a reference to every player it creates, so
391+
// dropping the element is not enough to release the <iframe>: it stays
392+
// detached but reachable from window.YT. destroy() removes the iframe and
393+
// deregisters the player.
394+
this.api?.destroy?.();
395+
this.api = null;
396+
super.disconnectedCallback?.();
397+
}
398+
360399
async attributeChangedCallback(attrName, oldValue, newValue) {
361400
if (oldValue === newValue) return;
362401

0 commit comments

Comments
 (0)