fix(YouTube): destroy player on unmount to fix memory leak in v2

[jv8-alt]

Adds the missing componentWillUnmount so player.destroy() runs when a YouTube player unmounts.

The bug

YouTube has no unmount hook. Player.js calls stop() on unmount, but that maps to stopVideo:

stop () {
  if (!document.body.contains(this.callPlayer('getIframe'))) return
  this.callPlayer('stopVideo')
}

stopVideo halts playback without deregistering the player, so every mount/unmount cycle leaks one — a detached <iframe> retained by window.YT, along with the player's event handlers and, through this.props, the surrounding component tree.

The API keys every player by element id and clears it only in destroy():

this.g.id=ja(this.g);T[this.g.id]=this;   // constructor: register
this.g&&this.g.id&&(T[this.g.id]=null);   // destroy(): the only place it clears

this.g is the iframe (n.getIframe=function(){return this.g}). The docs describe stopVideo() as stopping playback while keeping the player interface intact, and destroy() as the call that "Removes the <iframe> containing the player."

Found via heap snapshots on a production app: Detached HTMLIFrameElement nodes accumulating across route changes.

Previously reported, closed without a fix: #531 ("The YouTube player is not attached to the DOM" on unmount; asks for a destroy hook) and #377 (players not cleaned up when loaded in quick succession).

Full destroy(), verbatim from www-widgetapi.js (build ea6f527e)

n.destroy=function(){
  this.g&&this.g.id&&(T[this.g.id]=null);
  var a=this.i;a&&typeof a.dispose=="function"&&a.dispose();
  if(this.j){a=this.j;var b=this.g,c=b.parentNode;c&&c.replaceChild(a,b)}
  else(a=this.g)&&a.parentNode&&a.parentNode.removeChild(a);
  Z&&(Z[this.id]=null);
  this.options=null;
  this.g&&this.v&&this.g.removeEventListener("load",this.v);
  this.j=this.g=null
};

Z is a second registry, used by a global message listener to route postMessages to players. It is cleared in the same method and nowhere else.

The fix

componentWillUnmount () {
  if (this.player && typeof this.player.destroy === 'function') {
    try {
      this.player.destroy()
    } catch (error) {
      // destroy() throws if the iframe has already been removed
    }
  }
  this.player = null
}

Both guards matter: the component can unmount before getSDK() resolves, so this.player may never exist; and destroy() throws when the iframe is already gone — the exact error in #531, so the catch fixes that console error too. this.player = null drops the closure chain holding this.props.

Testing

Three cases in test/players/YouTube.js using the existing zora/sinon/react-test-renderer harness: destroy() called once and reference nulled; unmount before the player exists doesn't throw; a throwing destroy() doesn't break unmount.

Reverting src/players/YouTube.js alone makes them fail, so they aren't passing vacuously:

not ok 28 - destroy() is called on unmount
not ok 29 - the player reference is released

Full suite green and npm run lint clean on Node 18 (matching CI).

Note on v3

v3 has the same gap — it delegates to youtube-video-element, which defines no disconnectedCallback. Opened separately as muxinc/media-elements#256.