unbind onCLS listeners

[jantimon]

Usually it is possible to unbind listeners to avoid memory leaks.

Is there a way to do this for web-vitals listeners?

[tunetheweb]

No this is currently not possible, given that many of the metrics it's design to measure need to measured across the whole page lifecycle (CLS, INP). There has been some discussion on disabling them in #196 however this would likely break measurements as discussed in that PR.

[jantimon]

spa libraries and frameworks mound and unmount components

so for a click listener which is bound to the entire window we would have to clean it up once the component unmount

e..g. (react):

  useEffect(() => {
    const doSth = () => {
      alert("hello world");
    };

    // Bind the function
    window.addEventListener('click', doSth);

    // Clean up the event listener
    return () => {
      window.removeEventListener('click', doSth);
    };
  }, []);

e.g. (solid):

const doSth = () => {
    alert("hello world");
  };

  onCleanup(() => {
    // Clean up the event listener
    window.removeEventListener('click', doSth);
  });

  // Bind the function
  window.addEventListener('click', doSth);

e.g. angular

import { Component, OnInit, OnDestroy } from '@angular/core';

@Component({
  selector: 'app-my-component',
  template: '<div>My Component</div>'
})
export class MyComponentComponent implements OnInit, OnDestroy {
  private doSth = () => {
    alert("hello world");
  };

  constructor() { }

  ngOnInit(): void {
    // Bind the function
    window.addEventListener('click', this.doSth);
  }

  ngOnDestroy(): void {
    // Clean up the event listener
    window.removeEventListener('click', this.doSth);
  }
}

however with onCLS and similar listeners that's not possible

imho it would be possible by adjusting for example web-vitals/src/lib/observe.ts so it will always return a cleanup function

here is a very naiv example:

/**
 * Takes a performance entry type and a callback function, and creates a
 * `PerformanceObserver` instance that will observe the specified entry type
 * with buffering enabled and call the callback _for each entry_.
 *
 * This function also feature-detects entry support and wraps the logic in a
 * try/catch to avoid errors in unsupporting browsers.
 */
export const observe = <K extends keyof PerformanceEntryMap>(
  type: K,
  callback: (entries: PerformanceEntryMap[K]) => void,
  opts?: PerformanceObserverInit
): () => void => {
  try {
    if (PerformanceObserver.supportedEntryTypes.includes(type)) {
      const po = new PerformanceObserver((list) => {
        // Delay by a microtask to workaround a bug in Safari where the
        // callback is invoked immediately, rather than in a separate task.
        // See: https://github.com/GoogleChrome/web-vitals/issues/277
        Promise.resolve().then(() => {
          callback(list.getEntries() as PerformanceEntryMap[K]);
        });
      });
      po.observe(
        Object.assign(
          {
            type,
            buffered: true,
          },
          opts || {}
        ) as PerformanceObserverInit
      );
      return () => po.disconnect();
    }
  } catch (e) {
    // Do nothing.
  }
  return () => {};
};

same for web-vitals/src/lib/whenActivated.ts

export const whenActivated = (callback: () => void) => {
  if (document.prerendering) {
    addEventListener('prerenderingchange', callback, true);
    return () => removeEventListener('prerenderingchange', callback);
  } else {
    callback();
    return () => {};
  }
};

[philipwalton]

All of the metric functions exposed by this library are only ever intended to run once per page, including for SPAs, so there's never a valid reason to disconnect a PerformanceObserver that's actively observing entries for one of these metrics.

This is different from event listeners that do need to be removed if/when the component that registers them has been removed from the DOM.

Whether or not a page is an SPA doesn't change this. If an SPA were to invoke these functions and then disconnect the observers and re-invoke them after each route change, it wouldn't result in different values that are specific to just that route. The results are exactly the same regardless of when the functions are called, so removing and then re-invoking them is actually be more wasteful.

If your concern is how to properly measure the Core Web Vitals metrics in an SPA, checkout this post for how we plan to enable that, and note that the plan laid out in that post will still involves PerformanceObserver objects listening through the entire lifespan of the page.

[jantimon]

Maybe I need to explain my case a little bit more - I am talking about fullstack frameworks like Next.js or Nuxt with SSR and client side SPA technologies. While most user session are quite short we also saw user sessions lasting a week and longer.

Usually we send back the LCP information to our tracking once the user starts their first first client side navigation. After that we are no longer interested in new LCP events or FID events.

When building such apps we have to take care of the lifetime of our components, for example in scenarios such as using React's strict mode where side effects are executed twice.

The web-vitals library does not provide a way to clean up these performance observers. Consequently, strict mode would initialize two performance observers. This can lead to unnecessary duplication and potential performance overhead.

Additionally, when certain components in a SPA are unmounted, they may not be properly garbage collected because the web-vitals library still maintains references to them. Another issue is that those observers will update the state of the destroyed components

[philipwalton]

Usually we send back the LCP information to our tracking once the user starts their first first client side navigation. After that we are no longer interested in new LCP events or FID events.

Both the onLCP() and onFID() observers get disconnected (automatically) after the values of those metrics are finalized, so there should not be a problem here as long as your pages don't reinvoke the onLCP() or onFID() functions.

While most user session are quite short we also saw user sessions lasting a week and longer.

Understood. And just so it's clear, to properly calculate CLS and INP for those user session, you have to be observing all event and layout-shift entries for that entire time.

We've had some discussion internally about whether or not we should consider it a new user "visit" if (for example) the user puts their device to sleep and then returns to it hours or days later (as well as other such long sessions like those you describe). But even in those cases this library would still need to have an observer listening for the entirety of the user session, and it (itself) would internally handle resetting any local variables that hold references to event or layout-shift entries from the previous user "visit".

The web-vitals library does not provide a way to clean up these performance observers. Consequently, strict mode would initialize two performance observers. This can lead to unnecessary duplication and potential performance overhead.

My understanding is Strict Mode runs in development only, so the performance overhead shouldn't be an issue. If the double-reporting is a problem then I'd recommend calling the web-vitals metrics functions at the very top-level of your app and then putting the <StrictMode> container below that (which it seems like is a good idea anyway because as you said these functions will be called twice so you'd have the double-reporting either way; also the point of Strict Mode is to test the rendering and effects logic of a component, which is not applicable to the web-vitals functions as they are page-level.

Additionally, when certain components in a SPA are unmounted, they may not be properly garbage collected because the web-vitals library still maintains references to them.

The only references that will be kept are references to PerformancEntry objects, and those may contain references to DOM nodes so yes they will not be garbage collected. But these metrics were designed with this fact in mind, which is why none of these metrics require storing all past entries in memory. For example INP only ever stores the 10 largest event entries and CLS only stores the largest layout-shift entries in a 5-second that weren't within 500ms or a user interaction. So in both of these cases the maximum number of DOM nodes referenced will be capped to avoid memory leaks like you describe.

Another issue is that those observers will update the state of the destroyed components.

I don't think this is true. None of the web-vitals metric update any DOM elements or page components, and (as I mentioned before) if you're web-vitals function logic is itself run in a component, then that component should never be unmounted since it needs to be monitoring the page for its entire lifecycle. We also recommend you call these functions in a top-level component via a useEffect() hook to avoid issues with SSR (see previous discussion).

[tunetheweb]

Closing this as WAI.