[csswg-drafts] [resize-observer-1] Proposal for more robust per element error handling (#6185)

hilts-vaughan has just created a new issue for https://github.com/w3c/csswg-drafts:

== [resize-observer-1] Proposal for more robust per element error handling  ==
# Proposal to augment ResizeObserver error handling

## The problem

Today, `ResizeObserver` will throw an error that is emitted on `window` if a potential loop is detected / notifications were emitted again that could cause a loop. The event has the following characteristics that
are problematic:

1. The `target`  and `currentTarget` (at least in Chrome) is defined to be `window` and thus has no context of what thew
2. The exception will trigger most default `window.onerror` handlers installed by various reporting systems today, such as [Sentry](https://sentry.io) for every single instance of this
3. The error cannot be handled on a per element / observation level
4. It assumes that more that a single iteration implies that the layout would not converge

**Sometimes, this error is acceptable or even expected**. A good example of this from things that change to be depending on their content size using negative margins / padding (the latter is solved in a later version of the spec to read padding boxes) This can change their width and thus, fire another event. However, there are other cases such as the one from MDN where the font-size of the item inside is being changed.

1. [Font-size changing example](https://jsfiddle.net/Lxzmh40u/)
2. [Open-source library that viewport changes](https://github.com/petyosi/react-virtuoso/commit/5475a10d873811b056d059c85fa419e764b70181)
3. Negative-margins for a "full bleed behaviour"

In all the above cases, it is expected that the layout will converge and stop delivering more notifications on the 2nd loop through since it has "converged" on a single layout choice (a new font-size, new width, etc). For example, in the case of the font-size change, there is a one:one mapping of size to font-size, and thus the callback is idempotent. Today, there is no way to actually acknowledge this in code and have a programmer annotate and let the browser know that the layout will indeed converge and that the later notifications are not needed.

We need a way for a programmer to programatically identify and handle these exceptions with enough information for the various use cases. Such cases would be:

1. I expect my layout to converge, and a 2nd iteration isn't neccessary
2. I didn't expect any additional notification and would like to know about them
3. Provide enough flexibility that users can handle other cases without more intervention from the standards team
4. The exception may be thrown from some 3rd party code on a global window that the user does not control

## How users are dealing with this today

Drawing inspiration from [Stackoverflow](https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded]):

1. [Ignore the error all together](https://stackoverflow.com/a/50387233) @ 241 votes
2. [Wrap the change in rAF](https://stackoverflow.com/a/58701523) @ 41 votes
3. [Swallow the exception](https://github.com/search?q=ResizeObserver+loop+limit+exceeded&type=code)

It is clear that #1 and #3 is bad.

It is less clear that #2 is bad, but it is subtly wrong since the rAF would not run until the _next frame_, causing jank, and actually completely removing the "loop until settled" layout logic and causing a potential judder since it would not run until the next frame (since rAF running in an RO does not run until the next frame, since it is too late to run it for the current frame -- causing jank)

## What we can do about it

Let's add an API to the `callback` on `observer` called `allowUndeliveredFor(element : SVGElement|HTMLElement, {selfOnly: true|false = true})`. That is,
the API would then look like this:

```
callback : (entries : ResizeObserverEntry [], observer : ResizeObserver, allowUndeliveredFor : (element : SVGElement|HTMLElement, {selfOnly: true|false = true}))
```

The semenatics of invoking the callback would be the following: when the developer invokes `allowUndeliveredFor` for a given `element`, that node would not be considered when gathering skipped notifications as "not skipped"

This would amend step 6 of the spec that reads:

> If Document has skipped observations then deliver resize loop error notification

to become:

> If Document has skipped observations that were not marked as "allowed" then deliver resize loop error notification

The definition of "allowed" is then any node marked `allowUndeliveredFor` (all it's descendants if `allowUndeliveredFor` has the flag `selfOnly` set to `false`) the node hasn't been revisited again by another observer since. This means that if you had two `ResizeObserver`s that were tracking the same element -- you could ignore the error from either of them, however a notification firing another observer elsewhere would undo the internal set flag set by `allowUndeliveredFor`.

This is important since it prevents developers from supressing an error that they didn't intend to -- from within their own handler they know it's safe, but perhaps not others. It is still possible that something other than the RO handler changes the resizing in another observer, the developer must manually handle this.

You can find use cases below.

### _I expect my layout to converge on a single call_

Consider a case where font-size is adjusted in the RO:

```
        const resizeObserver = new ResizeObserver((entries, observer, allowUndeliveredFor) => {
          for (let entry of entries) {
              h1Elem.style.fontSize = Math.max(1.5, entry.contentRect.width/200) + 'rem';
              pElem.style.fontSize = Math.max(1, entry.contentRect.width/600) + 'rem';
              allowUndeliveredFor(entry.target, {selfOnly: false})
            }
          }
        });

        resizeObserver.observe(container);
```

In this case, the user has specified that they expect that the target element will have changed in size and it's fine that it will finish with an undelivered notifcation.

It was also considered to add a new callback that would be invoked with a set of all the undelivered notifications right before the delivery step. It could be summarized as:

```
        const ignore = new Set();
        const resizeObserver = new ResizeObserver((entries, observer) => {
          set.clear();
          for (let entry of entries) {
              h1Elem.style.fontSize = Math.max(1.5, entry.contentRect.width/200) + 'rem';
              pElem.style.fontSize = Math.max(1, entry.contentRect.width/600) + 'rem';
              set.add(entry.target);
            }
          }
        }, (undeliveredNotifications : UndeliveredNotification[], allowUndeliveredFor...) => {
          // UndeliveredNotification {
          //   target : Element;
          // }
          for(const notification of undeliveredNotifications) {
            if(ignore.has(notification.target)) {
              allowUndeliveredFor(entry.target)
            }
          }
        });

        resizeObserver.observe(container);
```

However, it is more clunky than the above option. It would also have to be per observer. However, it does allow you to post-process more easily if the elements to ignore depends on the final set of undelivered notifications.
Thus, it may be more desirable to implement the latter or both, even if slightly more clunky and leave it up to library authors to provide a good abstraction over top of it.

### Users may just ignore everything

This is signficantly harder to justify in this case -- since it has a named semantic meaning and you have to do it for each element. Some users may still do this but even so, it is better than supressing the entire error.

Users will naturally ignore everything when they don't understand the issue and have no other escape hatch.

### I didn't expect an additional notification

This is trivial, by not invoking the callback then the default is to continue to deliver the notification.
You can then handle it like you do today.

### _Provide flexibility to handle without more invervention_

You can technically handle any level of errors; if you would be fine with ignoring all errors
then you could mark every node on the DOM tree down by labeling it with `selfOnly: false` very explicitly
indiciating that you are fine with ignoring everything down.

With this, users can make it very clear about what is an expected error and what is not at the granular node level which is then level at which an undelivered notification would happen.

### The exception may be thrown from some 3rd party code on a global window that the user does not control

This is harder to deal with -- since JS errors can be thrown all the time in extensions and other things that developers don't control. A simple proposal that would fix this would be to change `target` to be _an_ element that had an undelivered notification to assist in filtering it out but this change is seperate from allowing developers to avoid throwing their own errors in the first place.

However, since multiple can have undelivered notifications a new field may be needed, such as `undeliveredElements` and users could check against this if they _really_ wanted to know.

I've omitted a detailed design on this since it is less critical than allowing users to handle their own in my eyes, but I think we could handle it in another proposal. Doing so would give users more information as to what element was the one that was resized as well and make for easier debugging.


Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/6185 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Tuesday, 6 April 2021 21:03:54 UTC