Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add section on avoiding memory leaks to events.md #1240

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add section on avoiding memory leaks to events.md #1240

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions packages/lit-dev-content/site/docs/v3/components/events.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,32 @@ disconnectedCallback() {

See the MDN documentation on using custom elements [lifecycle callbacks](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks) for more information on `connectedCallback` and `disconnectedCallback`.

### Avoiding Memory Leaks

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.
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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) => { /* ... */ };
}


```js
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;
}
}
Comment on lines +106 to +122
Copy link
Member

Choose a reason for hiding this comment

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

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;
}
}

```

The above example creates a memory leak if the reactive update cycle triggers again and holds the element (and its subtree) in memory as a Detached HTMLElement because `window` now has a reference to this element class through the event handler.

### Optimizing for performance

Adding event listeners is extremely fast and typically not a performance concern. However, for components that are used in high frequency and need a lot of event listeners, you can optimize first render performance by reducing the number of listeners used via [event delegation](#event-delegation) and adding listeners [asynchronously](#async-events) after rendering.
Expand Down
Loading