ZeeCoder / use-resize-observer

A React hook that allows you to use a ResizeObserver to measure an element's size.
MIT License
644 stars 42 forks source link

ResizeObserver loop limit exceeded #38

Closed makenosound closed 1 month ago

makenosound commented 4 years ago

👋 Hello. We have a component that’s using this hook with in it’s default form (no custom callback) and have been getting a lot of ResizeObserver loop limit exceeded errors thrown during usage.

From my brief search around the internet it seems as though this is harmless but it does add a lot of noise in common usage and that that noise could be avoided by wrapping the ResizeObserver callback in a requestAnimationFrame.

I could adjust our usage to do this using a custom callback, but that would mean reimplementing the core observer logic which seems rather pointless. Does it make sense as a core adjustment?

ZeeCoder commented 4 years ago

This is usually a sign that you have an infinite loop in your code that was caught. I'd try to debug how it's getting together so much.

not sure what you're proposing as a solution?

makenosound commented 4 years ago

Sorry, @ZeeCoder — I’m not that familiar with the ResizeObserver API or its usage in practice and so my reading of https://stackoverflow.com/a/50387233 was that that exception is usually caused not necessarily by an infinite loop, but by the observer triggering more events than can be handled in a frame.

What I was musing on as a solution was wrapping the observe callback in a requestAnimationFrame to ensure that it only passes events at a rate that’s actually usable in a consuming app. I imagine that could cause issues with an initial evaluation of their values though so it probably doesn’t make sense after all (and that suggestion was predicated on that SO comment being accurate anyway).

ZeeCoder commented 4 years ago

Yeah having a rAF call in there would mean that you no longer get notification of resize events as they happen, but rather with a 16ms delay or so. (Depending where you are in the current tick.)

If you need rAF, then it should be really easy to have a custom local hook, that does something like:

function useResizeObserverWithRAF(opts) {
const [size, setSize] = useState({width: undefined, height: undefined});
useResizeObserver({ ...opts, onResize: (size) => requestAnimationFrame(() => setSize(size)) });

return size;
}

In terms of such optimisations (like throttle and debounce) it's not as easy to know exactly what people might need, and as you'd pile more feature on top, the harder it would be to maintain this lib, which is meant to be as low-level as it can. 🤷‍♂️

It is a good candidate for a readme entry or a codesandbox demo. (Like the ones I've made for throttle and debounce.)

ZeeCoder commented 3 years ago

@makenosound any chance you could give me a minimal example code on codesandbox or somewhere, where the issue is reproduced? I thought it would be easy but can't seem to create a simple app that would show this error.

dmtrKovalenko commented 1 year ago

This is possible to reproduce very rarely when browser does not have enough resources to handle resize observer so probably there would never be any reproduction.

The easiest way is to try out cypress in CI multiple times, but to be honest the main problem with this issue is that it periodically spams our sentry

ZeeCoder commented 1 year ago

I'd look into skipping these logs in the sentry agent so that they're not collected at all, if you can't pinpoint the source of the loop and address that.