[ResizeSensor] cancelAnimationFrame for reset when detach is called to prevent infinite loop and memory leak

[gaoruhao]

In ResizeSensor, we use requestAnimationFrame for reset. However, if an element is added to DOM and removed from DOM very quickly (even before the first reset happens), the reset itself will stuck in an infinite loop. It also causes memory leak because the reset function holds the element forever.

[marcj]

Good catch, thanks!

[gaoruhao]

@marcj Would you please do a minor version update on NPM? The tests of my project are very unstable due to the bug. I am replying on this fix to save the whole tests.

[marcj]

Sure, v1.2.2 is released.

[gaoruhao]

Sure, v1.2.2 is released.

Awesome, it saves my life. Thank you. @marcj

[mKlus]

@marcj & @gaoruhao Version 1.2.2 broke our website resizing.
I haven't had chance to debug the issue, but when I reviewed the changes between 1.2.1 and 1.2.2 the below code seems to be incorrect to me.

// clean up the unfinished animation frame to prevent a potential endless requestAnimationFrame of reset
if (!lastAnimationFrame) {
  window.cancelAnimationFrame(lastAnimationFrame);
  lastAnimationFrame = 0;
}

Shouldn't it instead be (notice the if (lastAnimationFrame))?

// clean up the unfinished animation frame to prevent a potential endless requestAnimationFrame of reset
if (lastAnimationFrame) {
  window.cancelAnimationFrame(lastAnimationFrame);
  lastAnimationFrame = 0;
}

[marcj]

Indeed, that's incorrect, good catch! But it shouldn't break anything since its additional code that due to the bug isn't executed at all. What errors do you get?

[mKlus]

@marcj We don't get script error but with this change, the ResizeSensor handler won't get called. Our html5 app is quite simple so I'd expect this problem to be for most of the people.

            this.handleResize(size);
        });

I had a quick look and narrowed the issue down to the below change:

The lastAnimationFrame variable is used in reset function so something must have gone wrong there?

[mKlus]

@marcj Should I raise new issue for the above?

[mathias999us]

I am also having the same issue as mKlus with v1.2.2.

[marcj]

@mathias999us @mKlus Fixed in v1.2.3

[Acinho]

I am still having this issue with reset getting into infinite loop.
This fixes the issue: #289