Add section on avoiding memory leaks to events.md

[chrispaterson]

One of the most common causes of detached HTML Elements remaining in memory after a developer expects them to be removed and cleaned up is attributed to them not realizing that the Lit reactive lifecycle methods can be called again after disconnectedCallback. I think it might be prudent to add a section to the documentation that calls out the issue and gives a warning. This is my first attempt, if edits are needed please advise.

[augustjk]

Thank you! This is a great thing to point out. I do wonder if we should provide an alternative for the presented bad code but I'm not sure what the best fix for that would be... Maybe to add the event listener in connectedCallback and add the condition inside the handler callback?

[augustjk]

This might be blasphemous but we do a single space after periods.

Suggested change

Avoid mixing and matching adding and removing event listeners between the [Lit reactive lifecycle methods](/docs/components/lifecycle/#reactive-update-cycle) and custom element [lifecycle callbacks](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks). These two lifecycles, in some cases, operate independently of one another. For example, just because `disconnectedCallback` has been called does not mean that `shouldUpdate`, `willUpdate`, `update`, `render`, and `updated` will not be called again if a [reactive property](/docs/components/properties/) is changed after the element is removed from the DOM. This can result in memory leaks when listeners attached to active objects contain back references to the component that set them.

Avoid mixing and matching adding and removing event listeners between the [Lit reactive lifecycle methods](/docs/components/lifecycle/#reactive-update-cycle) and custom element [lifecycle callbacks](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks). These two lifecycles, in some cases, operate independently of one another. For example, just because `disconnectedCallback` has been called does not mean that `shouldUpdate`, `willUpdate`, `update`, `render`, and `updated` will not be called again if a [reactive property](/docs/components/properties/) is changed after the element is removed from the DOM. This can result in memory leaks when listeners attached to active objects contain back references to the component that set them.

[augustjk]

Nit around code style – single quotes for strings and underscore for private method and unused param. Also inline comment to make it clear this shouldn't be done.

Suggested change

	connectedCallback () {
	super.connectedCallback();
	this.listeningForResize = false;
	}
	disconnectedCallback() {
	window.removeEventListener("resize", this.handleWindowResize);
	super.disconnectedCallback();
	}
	handleWindowResize = (event) => {
	this.style.width = "880px";
	}
	willUpdate(changedProperties) {
	if (!this.listeningForResize) {
	window.addEventListener("resize", this.handleWindowResize);
	this.listeningForResize = true;
	}
	}
	connectedCallback () {
	super.connectedCallback();
	this.listeningForResize = false;
	}
	disconnectedCallback() {
	window.removeEventListener('resize', this._handleWindowResize);
	super.disconnectedCallback();
	}
	_handleWindowResize = (_event) => {
	this.style.width = '880px';
	}
	willUpdate(_changedProperties) {
	if (!this.listeningForResize) {
	// This might never get removed and cause a memory leak!
	window.addEventListener('resize', this._handleWindowResize);
	this.listeningForResize = true;
	}
	}

[justinfagnani]

I would love to see a description of the problematic structure so that readers can develop a mental model of when event listeners will hold on to the element.

Something like:

Event listeners can sometimes cause memory leaks if they're not removed, depending on the objects or elements they're added to and the objects they reference in the body of the event handler.

Listener functions hold a JavaScript reference to variables closed over in the function body. This behaves as if you added a property to the event target that references the objects used in the handler.

When this can often cause a leak is when a listener is added to an object outside of the custom element or it's children, that references back to the element:

class LeakyElement extends LitElement {
  constructor() {
    super();
    // This will cause LeakyElement instances to never be GC'ed if not removed:
    window.addEventListener('mousemove', this.#onMouseMove);
  }
  #onMouseMove = (e) => { /* ... */ };
}

To prevent a memory leak, you should added these listeners in connectedCallback() and remove them in disconnectedCallback():

class LeakyElement extends LitElement {
  connectedCallback() {
    super.connectedCallback();
    window.addEventListener('mousemove', this.#onMouseMove);
  }
  disconnectedCallback() {
    super.disconnectedCallback();
    window.removeEventListener('mousemove', this.#onMouseMove);
  }
  #onMouseMove = (e) => { /* ... */ };
}

Note that the reference to the event listener needs to be the same in the addEventListener() and removeEventListener() calls. Using a class field initialized to an arrow function is a convenient way to do this.

Event listeners added by an element to itself or a child do not usually cause a leak because they create a cycle that GCs can collect:

class LeakyElement extends LitElement {
  constructor() {
    super();
    // This will *not* cause a leak:
    this.addEventListener('click', this.#onClick);
  }
  #onClick = (e) => { /* ... */ };
}