Skip to content

Commit

Permalink
cancelAnimationFrame for reset when detach is called (#282)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gaoruhao authored and marcj committed Nov 22, 2019
1 parent 4b06648 commit 1e40612
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions src/ResizeSensor.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@
* @constructor
*/
var ResizeSensor = function(element, callback) {
var lastAnimationFrame = 0;

/**
*
* @constructor
Expand Down Expand Up @@ -204,7 +206,7 @@
var lastWidth = 0;
var lastHeight = 0;
var initialHiddenCheck = true;
var lastAnimationFrame = 0;
lastAnimationFrame = 0;

var resetExpandShrink = function () {
var width = element.offsetWidth;
Expand Down Expand Up @@ -281,14 +283,19 @@
addEvent(shrink, 'scroll', onScroll);

// Fix for custom Elements
requestAnimationFrame(reset);
lastAnimationFrame = requestAnimationFrame(reset);
}

forEachElement(element, function(elem){
attachResizeEvent(elem, callback);
});

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

This comment has been minimized.

Copy link
@Acinho

Acinho Feb 26, 2020

Contributor

Shouldn't it be set to 1 here?

This comment has been minimized.

Copy link
@marcj

marcj Feb 26, 2020

Owner

Is already fixed in 72adc49, but no, not 1. Why do you think so?

This comment has been minimized.

Copy link
@Acinho

Acinho Feb 26, 2020

Contributor

This was my first thought without looking at the code in detail. I was wrong, but I think that the check is wrong, shouldn't the animation frame be cleared if it exists?

In my case reset is stuck in an infinite loop because an element is added and removed from DOM very quickly, but this latest commit doesn't fix it.

}
ResizeSensor.detach(element, ev);
};

Expand Down

0 comments on commit 1e40612

Please sign in to comment.