Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

ResizeObserver#observe() firing the callback immediately results in infinite loop #38

Closed
LeaVerou opened this issue Mar 29, 2017 · 27 comments

Comments

@LeaVerou
Copy link

I thought I'd post here first, in case this is intended behavior and not a Chrome bug. Couldn't quite tell from the spec.

Take a look at the following code (live here):

var div = document.querySelector("div");
var resizeObserver = new ResizeObserver(entries => {
	console.log("resizeobserver fired");
	resizeObserver.disconnect();
	
	// [Operation that would cause resize here]
	
	resizeObserver.observe(div);
});

resizeObserver.observe(div);

This is a very common pattern with any kind of observer: Often you want to monitor external changes to the DOM and as a response to them, you need to make changes of your own that should not trigger the same observer because that would cause a loop.

With Mutation observers, this pattern works fine. However, since ResizeObserver seems to be firing the callback immediately after observe() is called, this causes an infinite loop.

@cvrebert
Copy link

cvrebert commented Mar 30, 2017 via email

@LeaVerou
Copy link
Author

LeaVerou commented Mar 30, 2017

Don’t think so. It would be a duplicate if there was no disconnect and observe in the callback above. I'm not asking why doing resize operations in the callback causes a loop. I'm saying that if I'm doing these operations after the observer has been disconnected, and observing again after resizing, it should not trigger any loop. The reason it does currently is that the observer fires immediately after observe() is attached, whether any resizing happens or not. If you check out the JSBin I posted, the loop happens even if the only code in the callback is the disconnect and observe!

@atotic atotic added the bug label Mar 30, 2017
@atotic
Copy link
Collaborator

atotic commented Mar 30, 2017

This is a bug. The infinite loop should have been prevented by depth limit. Filed
https://bugs.chromium.org/p/chromium/issues/detail?id=706972

Briefly looking at the code, this should not have happened.

Glad you are looking at the API. We are planning on sending Intent To Ship soon, would love to hear any comments about the API you might have.

@LeaVerou
Copy link
Author

LeaVerou commented Mar 30, 2017

As I mentioned above, it should not even get close to the depth limit, because it shouldn't fire immediately on observe, but only once an actual resize has occurred! The bug is NOT that the depth limit is not applied, it's that the callback is firing immediately once observe() is called. Is that per spec or is it a Chrome bug?

If I want the same code to run both immediately when I attach the observer and in the future on resizes, I can just call it myself before attaching the observer and it would just be an extra line of code. However, if I don't want this behavior, there's no opt-out. Also, running the callback immediately once observe() is called is inconsistent with how Mutation Observers work. I've been using this pattern to with mutation observers for years and it works fine.

@atotic
Copy link
Collaborator

atotic commented Mar 30, 2017

callback is firing immediately once observe() is called. Is that per spec or is it a Chrome bug?

That is per spec. The reason is non-obvious (layout thrashing). See
#8 for details.

@que-etc
Copy link

que-etc commented Oct 6, 2017

@atotic

Hi Aleks,

I've recently found that ResizeObserver fires the loop limit exceeded error whenever resize is being triggered by the RO callback. This happens even when only one element is being observed and there are no subsequent calls after that, so this is probably not because of an infinite loop (example).

Could please confirm that this is an expected behavior because it definitely wasn't like that a few months ago.

@atotic
Copy link
Collaborator

atotic commented Oct 6, 2017

It is an expected behavior. If an element is resized in response to a notification, that generates a new expectation, which can't be delivered.

This should have been happening all along. I am surprised that this was not the case a few months ago.

There was a bug where garbage collection stopped all notifications, but this example would not have triggered it.

What do you think should happen?

@que-etc
Copy link

que-etc commented Oct 6, 2017

This should have been happening all along. I am surprised that this was not the case a few months ago.

Well, testing of the native implementation against tests for the polyfill wasn't failing before. At least not when I checked it last time. It's a shame that I've found it only now.

What do you think should happen?

As far as I'm concerned, if it doesn't lead to an infinite loop then no exceptions should be raised.

The most terrifying thing is that when RO gets enabled by default it will break quite a lot of existing implementations that rely on the polyfill. Unfortunately I don't know of any workaround to reproduce this exact behavior on the JS side. It doesn't seem to be possible to tell what triggered the resize of an element.

@eeeps
Copy link

eeeps commented Oct 6, 2017

Pardon my ignorance, but why does the JSBin example trigger an error, while this does not (or am I just missing it? Expecting to see it in the console...)

In the example, the block still becomes 300px wide. My understanding is, RO sees that the observed element has triggered its own resize and quits queuing microtasks until the next frame. So the intended change happens, just a frame late. Is that correct? If so, why is that a big deal?

@joshjg
Copy link

joshjg commented Apr 18, 2018

Any update on this?

@matthewbordas
Copy link

?

@atotic
Copy link
Collaborator

atotic commented Aug 10, 2018

A note to everyone interested in this API.

We've gotten feedback from developers who'd like to observe border-box rect. We are thinking of tweaking the API to allow observation of border box (possibly scrollSize too). We are hoping to have a proposal by October TPAC.

@giniedp
Copy link

giniedp commented Sep 17, 2018

I am using the ag-grid library which uses the ResizeObserver under the hood. My issue is described here ag-grid/ag-grid#2588

My observation can be summarized in the following code snippet

const observedElement
const siblingOfObservedElement

const observer = new ResizeObserver(()=> {
    updateStyleOf(siblingOfObservedElement);
});

observer.observe(observedElement);

When running unit tests of our product with Chrome Headless (all versions since 64) inside docker (debian jessie) the test suite breaks with a ResizeObserver loop limit exceeded exception. However, the behavior is not consistent. The test suite breaks in different spots every time it is run. But the issue can not be reproduced in Chrome non-headless.

My questions:
Is there anything more that developers need to know about the loop limit and how to avoid it?
Are there any guidelines or best practice of how to handle the callback?
Can anyone confirm that this is expected behavior for my example code above

Our test suite will pass by changing the library source code to this:

const observer = new ResizeObserver(()=> {
    setTimeout(() => updateStyleOf(siblingOfObservedElement));
});

Interestingly by doing so, the observer does not trigger an additional callback.

@atotic
Copy link
Collaborator

atotic commented Sep 17, 2018

"loop_limit exceeded" notification happens when ResizeObserver cannot deliver observations.
Here is a simplified explanation:




Here, height of #container depends on height of #target. If you are observing #container, and change #target height inside RO callback, you'll hit the error. #target resize will change #container height, which will trigger RO observation, which cannot be delivered because RO has already delivered notifications at this depth.

This error is often benign. Seeing many of them would be worrysome, and might indicate an infinite resize loop.

For detailed explanation of what this error means, check out #7

It is interesting that it only happens in headless, and that it is not consistent. From your example, sibiling's size affects observed's size. But why only sometimes?

@giniedp
Copy link

giniedp commented Sep 18, 2018

@atotic thanks for the explanation. I hoped there would be more to it. But in the essence it is clear.

So i tried to reproduce the behavior https://github.com/giniedp/ag-grid-ro-issue
and found out that there is no issue with Chrome Headless. Its just that Chrome Headless has less screen space. The test case was not hitting that space boundaries in Chrome but only in Headless and presumably the the library had to perform different layout operations.

Now i still dont know why it only breaks sometimes. But that is something the library developers have to sort out.

@schontz
Copy link

schontz commented Apr 24, 2019

Is there any way to catch this error? It is happening during unit testing, which is halting our test suite. I've tried try...catch, window.onerror, and window.addEventListener('error', ...), all without success.

@atotic
Copy link
Collaborator

atotic commented Apr 24, 2019

Error gets reported to "window.onerror" like many other runtime errors. There is no way to "catch" it.
I assume your test framework is also using windows.onerror to detect this error. Is there a way to tell your test framework to ignore ResizeObserver global errors?

@schontz
Copy link

schontz commented Apr 24, 2019

I'm using Intern and there is now an issue for this case.

My question is this: if this is a benign error that we should ignore, why is it being reported? Why do we need to redo tooling and such for non-errors? Can it be delivered as a warning instead?

@nothingismagick
Copy link

So if this is happening to you with cypress, @jennifer-shehane suggests:


Cypress.on('uncaught:exception', (err) => {
  if (resizeObserverLoopErrRe.test(err.message)) {
    // returning false here prevents Cypress from
    // failing the test
    return false
  }
})

And to mute Sentry:

Sentry.init({
  ...
  ignoreErrors: ['ResizeObserver loop limit exceeded']
})

From this discussion: quasarframework/quasar#2233

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests