Improve the ResizeObserver loop errors situation (tests and prod)

[hsablonniere]

The context

We use a ResizeObserver for some components so they can adapt their style and/or template, depending on their size (and not the size of the viewport).
We mostly adapt on the width, not the height.

We wrote a "mixin" called withResizeObserver to hide the boilerplate and provide 2 higher level APIs:

• a declarative CSS API: you provide a list of breakpoints with JS and the mixin adds attributes on the element so the CSS can be applied automatically
• an imperative JS API: you set a _onResize() method on the component and the mixin calls it on each resize

NOTE: we recently modified the mixin to prevent some layout shifts 07cc728

The "problem"

When you use a ResizeObserver and do something that triggers a resize inside the resize callback, you can potentially create an infinite loop.

Thankfully, the spec has a section about this:

ResizeObserver extends step 12 with resize notifications. It tries to deliver all pending notifications by looping until no pending notifications are available. This can cause an infinite loop.
Infinite loop is prevented by shrinking the set of nodes that can notify at every iteration. In each iteration, only nodes deeper than the shallowest node in previous iteration can notify.
An error is generated if notification loop completes, and there are undelivered notifications. Elements with undelivered notifications will be considered for delivery in the next loop.

TL;DR: browsers must prevent infinite loops and generate an error if it happens.

• With Chrome:
  • The error is ResizeObserver loop limit exceeded
  • The error is NOT visible in the devtools
• With Firefox:
  • The error is ResizeObserver loop completed with undelivered notifications
  • The error is NOT visible in the devtools
• With Safari:
  • The error is ResizeObserver loop completed with undelivered notifications
  • The error is visible in the devtools

Even when the error is NOT visible in the devtools, it is generated and thrown:

• It can be caught by listening to error events on window.
• This is what test runners and error tracking software do,
• and this is why we see these errors in our CI and error reports.

Background research

I read many issues, articles and StackOverflow questions:

• https://blog.elantha.com/resizeobserver-loop-limit-exceeded/
• https://webkit.org/blog/9997/resizeobserver-in-webkit/
• https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded
• https://stackoverflow.com/questions/74759528/error-resizeobserver-loop-limit-exceeded
• https://stackoverflow.com/questions/63653605/resizeobserver-loop-limit-exceeded-api-is-never-used
• https://stackoverflow.com/questions/65732703/resizeobserver-loop-limit-exceeded-stop-exception
• ResizeObserver#observe() firing the callback immediately results in infinite loop WICG/resize-observer#38
• ResizeObserver loop limit exceeded in Chromium browsers webpack/webpack-dev-server#4804
• Error: ResizeObserver loop limit exceeded cypress-io/cypress#19591
• "ResizeObserver loop limit exceeded" since Chrome update 64 souporserious/react-measure#104
• ResizeObserver loop limit exceeded ag-grid/ag-grid#2588
• https://github.com/juggle/resize-observer/blob/master/README.md#resize-loop-detection

The conclusion is: this is not really an error you need to fix but more of a warning you can ignore (most of the time).

The solutions

If our goal is to stop seeing these errors in our CI and error reports, I see 2 solutions.

Solution 1: Prevent the error from happening in the first place

This article by Jon Elantha explains everything about the whole problem (in a react context).
He proposes a technique for solution 1.

🙂 Pros:

• I tried it and it works
• Not so much code to add

🤬 Cons:

• The added code is directly inside the resize callback
• It adds call to getBoundingClientRect() which you should try minimize for performance reasons
• It also requires to unobserve + observe which I have no idea if it's a performance reasons
• It will only work for our usage of ResizeObserver
  • If one of our dependencies triggers these "errors", they will appear in our CI and error reports
  • As we can see in the various GitHub issues, this is a classic and rarely handled/solved in projects

Solution 2: Prevent the error message from being passed to our tooling

The @lit-labs/virtualizer repo uses a ResizeObserver.
They don't try to prevent the error from happening but they have a way to silence the error messages so CI and error reports don't see them.

Their documentation have a section about ResizeObserver loop errors:

https://github.com/lit/lit/blob/main/packages/labs/virtualizer/README.md#resizeobserver-loop-limit-exceeded-errors

The error itself is benign and only means that the ResizeObserver was not able to deliver all observations within a single animation frame.
It may be safely ignored, but it may be necessary to instruct your framework to ignore it as well to prevent unnecessary side-effects and error-handling.

They provide a helper function to ignore these errors in test environments.

🙂 Pros:

• The code seems simpler with less impact on performances (just a guess)
• We already use (still in a WIP branch) the @lit-labs/virtualizer for our <cc-logs> component
• If we use this in our tests, it would work for all ResizeObserver:
  • The ones we use directly
  • The ones in our dependencies like @lit-labs/virtualizer

🤬 Cons:

• You should always be careful when patching things like window.onerror

Open questions:

A. Should we use this helper function in production?

• Yes, it means "a bit more code" to load but less traffic with the error tracking software
• No, it means "a bit less code" to load but more traffic with the error tracking software
  • Users will need to find how to ignore these errors directly in their error tracking software

B. If we use this helper function in production:

• Should components run the helper function automatically for the user?
  • We only need this for the ones which may trigger the error
  • We need to make sure we only run it once
• Should we ask the user to explicitly run the helper function in their app?
  • If so, the project could wrap and expose the helper function from @lit-labs/virtualizer

The proposition

For our tests

• I think we should go for solution 2

For production:

• A. I think we should use @lit-labs/virtualizer helper function
  • It doesn't look like much code to add
  • Less traffic to the error tracking software is good for everyone
• B. I think we should wrap and expose it so users can run it in their app
  • If we think it's too much code, we can write a smaller version
  • Users control if they really want to patch window.onerror
  • Users control when they apply it
  • Users may have other parts of their code which needs this
  • Users may already have some kind of filter to handle this

WDYT?

The plan

1. We wait for our cc-logs branch to be merged (it brings the @lit-labs/virtualizer dependency)
2. We add @lit-labs/virtualizer helper function in our tests so we don't see these errors anymore
3. We wrap and expose the helper function in a lib

• It will be easy for npm users
• It will be tricky for CDN users but I don't think we go so far

4. We document how to use it
5. We apply this in the console

WDYT?

[pdesoyres-cc]

As said by Elantha

Most of the time the situation isn't really an error, just a notification. That's probably why Chrome and Firefox have decided not to report the errors in the console.

Because of that, I would not try to implement the solution 1.

I find the lit-labs implementation very smart so I would be ok with solution 2.

[roberttran-cc]

I agree, I like solution 2 better.

Thank you for the analysis, it helps so much with decision making! 🙏

[florian-sanders-cc]

Thanks a lot for all the context and explanation! 🙌
Here's my take based on that:

• For tests: solution 2 👍
• For production: What if we only add these errors to a list of errors to be ignored by Glitchtip? Unless it actually causes an issue for our users but from what I understand it doesn't 🤔

About the solution 1 within the tests: we went for a quick & dirty solution when we first implemented a11y tests. We also catch other errors (a ChartJS / Canvas issue if I remember correctly). Like the resize observer issue, we chose not to "silence" it at the time because we were unsure of its impact. It would be better to silence it and open a proper issue about this?