search

wnr / element-resize-detector

Optimized cross-browser resize listener for elements.
MIT License
1.32k stars 118 forks source link

Clear object strategy timeout in uninstall function to fix potential … #117

Closed valduris closed 4 years ago

valduris commented 4 years ago

…runtime error

wnr commented 4 years ago

Hi and thank you for the PR.

This seems like a good thing to fix. Have you considered if your fix works for multiple .listenTo calls? It seems to me that the variable you introduced is shared across all object instances, and could therefore stop working when having multiple objects. I would consider binding that variable to each object instance instead like so: getState(element).checkForObjectDocumentTimeoutId = ....

I would also be very happy if you would provide a test case for this, to avoid future regression.

Cheers

valduris commented 4 years ago

@wnr , I've updated the PR as You requested, but it seems all unit tests for "object" strategy are disabled, so I haven't added any.

wnr commented 4 years ago

@valf Thank you. Looks good to me now. Yes, I forgot those tests was disabled for the CI env.