Calling uninstall too soon after calling listenTo results in an error about element-resize-detector HOT 14 CLOSED

Hi. I might have a reason to reopen this issue again. I'm currently using react-sizeme which uses this library. I have created a responsive component that is designed to work in pretty much any page, however when rendered in particular pages I get the following error:

It seems to be that injectScrollElements is called, creating the necessary divs for the scroll approach, and then registering scroll events for the expand and shrink elements. These events rely on a method (onShrink and onExpand) which are added in the next step of the batch processor (in registerListenersAndPositionElements). The scroll events however are being called before these onShrink and onExpand have been registered on the element causing for an error seen above.

I'm a little lost as to why this is the case. Any help you could provide would be seriously appreciated!

from element-resize-detector.

Hi,

Thanks for the report. I'll have a look.

from element-resize-detector.

I have pushed a fix to master. Could you please verify it with your use case before I publish the new version (1.1.2)? Thanks

from element-resize-detector.

Great, it looks like it's working now. Thanks for the quick fix!

from element-resize-detector.

When do you think you will publish 1.1.2?

from element-resize-detector.

Hi, great! Sorry for the delay, I'll try to publish it later today. Cheers :)

from element-resize-detector.

I've pushed it now :)

from element-resize-detector.

Hi. I would like to reopen this issue. I'm sure the fix made the situation much better, but there is a scenario (particularly when integrated with React) where it still fails. I investigated and found the following assumption to be false: https://github.com/wnr/element-resize-detector/blob/master/src/detection-strategy/scroll.js#L599

Here is what happens in my application, which results in a call to install() while the component is still busy, but the listeners are already installed. This leads to the uninstall being canceled and subsequent errors like "scroll.js:380 Uncaught TypeError: Cannot read property 'onExpand' of undefined".
I use element-resize-detector through react-measure, both are the latest versions (element-resize-detector 1.1.8).

I have multiple elements in a container, each of which is measured. When the container detects that it gets too full (or equivalently if the container gets too small during resize), it just removes the elements that don't fit anymore (and puts them elsewhere, but that's not important here). For now, let's assume there are only two elements, and the second one won't fit. What happens (during page-load in particular):

• The two elements are "install()"-ed, leading to the 5 stages being scheduled in the batchProcessor.
• At some point, the injectScrollElements() stage happens for both elements. In particular, the second element now has its handlers attached.
• At a later point, the first element reaches its "ready()" stage. This causes a React update and my React code decides to remove the second element.
• This calls "uninstall()" for the second element. But because the second element is still busy, the handlers are not removed.
• Later the second element reaches its ready() stage and the main file (https://github.com/wnr/element-resize-detector/blob/master/src/element-resize-detector.js#L256) notices correctly that it was uninstalled before being detectable -- except that it's now in fact half-detectable..

The root cause seems to be the concurrency of the two install-processes, which is caused by the batch processing. If registering one element was an atomic (i.e. all stages complete before another element can start install()-ing), the problem wouldn't occur in my case. This could be achieved by not using distinct "level"-parameters for the batch processor.
Alternatively, the scroll.js:uninstall(element) method could not check for "state.busy", but instead just unregister the handlers, if they're there, and remove the child, if it's there.

Additionally, I get the same error for onRendered, and it seems like the "animationstart" handler is never removed? https://github.com/wnr/element-resize-detector/blob/master/src/detection-strategy/scroll.js#L274

from element-resize-detector.

Hi, and thanks for the very detailed bug report!

I'll have a closer look later today. Reopening.

from element-resize-detector.

I've now managed to reproduce the issue. It only occurs when the option callOnAdd is true. The assumption is that an element can never be uninstalled during the processing of the batch due to JS's execution model. This assumption is false when callOnAdd is true, because the callback called may uninstall other elements still in the batch.

I'll make a couple of unit tests for this and hopefully I'll come up with a solution later tonight.

This could be achieved by not using distinct "level"-parameters for the batch processor.
Alternatively, the scroll.js:uninstall(element) method could not check for "state.busy", but instead just unregister the handlers, if they're there, and remove the child, if it's there.

The levels are important for performance. I think the solution will be somewhere along your second suggestion.

Additionally, I get the same error for onRendered, and it seems like the "animationstart" handler is never removed? https://github.com/wnr/element-resize-detector/blob/master/src/detection-strategy/scroll.js#L274

Good catch, those should be removed as well.

from element-resize-detector.

I've pushed a WIP branch for release 1.1.9 called WIP-1.1.9 that contains a fix for the issues your describe. Could you please try it to see if it solves the problems for you?

from element-resize-detector.

Excellent! It works.

from element-resize-detector.

Nice, this has now been released as version 1.1.9.

from element-resize-detector.

Hmm that's an interesting case @Francesco-Lanciana. If you would open a separate issue I could have a look. Also, if you could manage to create a little test case reproducing the issue for me that would be great!

from element-resize-detector.