useResizeObserver not preventing Resize Observer errors in some situations

[dgsousa]

🐛 Bug Report

In some situations, spectrum components that use useResizeObserver throw errors such as
ResizeObserver loop limit exceeded
to be thrown in Chrome and
ResizeObserver loop completed with undelivered notifications.
in Firefox.

Generally these errors are benign and can be swallowed. However, Safari's CORS policy throws anonymous script errors and swallowing all script errors in Safari is a less than ideal solution.

🤔 Expected Behavior

useResizeObserver should not be causing Resize Observers to throw errors.

😯 Current Behavior

💁 Possible Solution

Resize Observers generally throw these errors to prevent infinite loops as detailed here:
https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded

But these errors are preventable using the approach detailed in this article:
https://blog.elantha.com/resizeobserver-loop-limit-exceeded/

🔦 Context

💻 Code Sample

import React from "react";
import ReactDOM from "react-dom";
import App from "./App";

window.addEventListener('error', (err) => {
    console.log(err);
})

ReactDOM.render(<App />, document.getElementById("root"));

import React, {
  useEffect,
  useState,
} from 'react';
import {
  defaultTheme,
  Flex,
  View,
  Provider,
  Content,
  Text,
} from '@adobe/react-spectrum';
import {
  Tabs,
  Item,
} from '@react-spectrum/tabs';


export default function App() {
  const [
    widthUnder1100,
    setWidthUnder1100,
  ] = useState(window.innerWidth < 1100);


  useEffect(() => {
    const handleResize = () => {
      setWidthUnder1100(window.innerWidth < 1100);
    };
    window.addEventListener('resize', handleResize);
    return () => window.removeEventListener('resize', handleResize);
  }, []);

  const shrink = widthUnder1100;
  return (
    <Provider theme={ defaultTheme } colorScheme='light'>
      <Flex>
        <div
          data-id='collapse'
          style={ { width: shrink ? '200px' : 'auto' } }>
          <Tabs aria-label='Chat log collapse example'>
            <Item title='John Doe' key='item1'>
              <Content marginTop='size-250' marginStart='size-125'>
                <Text>1</Text>
              </Content>
            </Item>
            <Item title='Jane Doe' key='item2'>
              <Content marginTop='size-250' marginStart='size-125'>
                <Text>2</Text>
              </Content>
            </Item>
            <Item title='Joe Bloggs' key='item3'>
              <Content marginTop='size-250' marginStart='size-125'>
                <Text>3</Text>
              </Content>
            </Item>
          </Tabs>
        </div>
      </Flex>
    </Provider>
  );
}

🌍 Your Environment

Software	Version(s)
react-spectrum	16
Browser	Chrome / Firefox / Safari
Operating System	MacOS

🧢 Your Company/Team

Adobe / Marketo

Marketo

🕷 Tracking Issue (optional)

[LFDanLu]

Reproduced the error locally, will have to investigate to see if the solution in the blog you linked has any unforeseen side effects

EDIT: Another possible solution is to wrap https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/utils/src/useResizeObserver.ts#L29-L33 in a requestAnimationFrame as suggested in the same stackoverflow post

[dashed]

@LFDanLu is this still being investigated? We're seeing this in sentry.io after this PR was merged getsentry/sentry#32061

[LFDanLu]

@dashed thanks for the reminder, this must have fell off my radar. I'll see if I can get this into one of our upcoming sprints but would welcome a PR!

Just to note, my concerns were mainly centered around any odd flickering that may occur with adding a requestAnimationFrame. If any flickering was noticed, I was leaning towards trying to swallow this error/prevent it from causing noise since seems to be a benign error.

[snowystinger]

See #4204

[snowystinger]

I've taken more time to look into this. Everyone who is on a newer version of React Spectrum can no longer reproduce. I believe this message was specific to our TableView and was telling us about a mistake in our code which we corrected across these PRs
#3672
#3656
and the most likely culprit #3474

For future reference, if you're using our hook useResizeObserver, and you see this error. It is likely not a problem with the hook. It is telling you that you've updated the dom in response to the resize event before a new frame (or render) has occurred. This usually takes the form of elementRef.current.style.height = whatever which causes a new resize event immediately, possibly leading to an infinite loop. See #4204 for the story that reproduced the error. This is exactly what it was doing. The solution to it is to instead store the new height in state and apply it during the next render.

The mentioned solutions were all ways of hiding the loop. It is better not to hide this so we can all fix our code. This is what happened in 3474, though it may not look like we fixed a direct dom manipulation from the code changes because that code was hidden behind a few other layers of code.

I am going to close this now.

[snowystinger]

We've found someone able to reproduce again, reopening for more investigation

[snowystinger]

I have a minimal reproduction https://codesandbox.io/s/clever-hodgkin-ygvn7r?file=/src/App.js

Open in Safari
Make the window not super tall
Open the console
Open left and right combobox

it's due to this line

react-spectrum/packages/@react-aria/overlays/src/useOverlayPosition.ts

Line 149 in bd89c6a

Object.keys(position.position).forEach(key => (overlayRef.current as HTMLElement).style[key] = position.position[key] + 'px');

commenting it out fixes the error
and this makes sense, we're setting a maxHeight directly on an element instead of waiting for React's render

Note, this is actually a pretty new issue, we had and have fixed the issue with TableView showing this error.

Potential solution to try:
Revert the changes so position is only set via state setter rather than modifying the node styles directly (65f3a87 and 9fecc97 are related changes).
Additionally, instead of using a global to stop scrollIntoView from being called, try applying a data attribute to the parents that indicate it is being positioned (data-isRepositioning?). Use a mutationObserver that triggers the scrollIntoView once all the parents don't have that data attribute

UPDATE: I can no longer reproduce this issue in React 18 (Combobox specifically) in the latest version (3.32.2)

[LFDanLu]

Looks like this is still happening for the TableView in React 17/16 specifically. Try https://reactspectrum.blob.core.windows.net/reactspectrum/14553eb3181befc00e81883864743b323491ce3a/storybook-16/index.html?path=/story/tableview--resize-test&providerSwitcher-express=false and https://reactspectrum.blob.core.windows.net/reactspectrum/14553eb3181befc00e81883864743b323491ce3a/storybook-17/index.html?path=/story/tableview--resize-test&providerSwitcher-express=false and check the console upon resizing the browser window in such a way that the TableView shrinks.

[LFDanLu]

Some additional info: this only started happening in React 17/16 from this PR. Before this, we didn't receive the ResizeObserver loop completed with undelivered notifications. warning in React 18/17/16 when resizing the TableView's containing window at least since early Dec 2022.

Storybooks for testing, to reproduce simply open the javascript console and add a error listener via window.addEventListener('error', (e) => console.log('ERROR: ', e)); and try shrinking the story window to the point that the TableView's width grows smaller.

breaking case: https://reactspectrum.blob.core.windows.net/reactspectrum/11b5be3a08cfcd564fcb98f534cdabeb64ba0711/storybook-17/iframe.html?scrolling=false&providerSwitcher-locale=&providerSwitcher-theme=&providerSwitcher-scale=&providerSwitcher-express=false&args=&id=tableview--static&viewMode=story

working case from the commit before: https://reactspectrum.blob.core.windows.net/reactspectrum/91515d24149449ffea8099f5c06eb588bd6b6c15/storybook-17/iframe.html?scrolling=false&strict=false&args=&providerSwitcher-locale=&providerSwitcher-theme=&providerSwitcher-scale=&providerSwitcher-express=false&id=tableview--static&viewMode=story