wnr / element-resize-detector

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

Update scroll.js #93

Closed Shirley89 closed 6 years ago

Shirley89 commented 6 years ago

function 'onExpandScroll' and 'onShrinkScroll' throw an error, getState(element) return undefined, judge getState(element)

wnr commented 6 years ago

Hi, sorry for the late response. Could you provide some background regarding this issue? I'm a bit reluctant to add such fixes that only avoids throwing exceptions rather than fixing the real underlying issue. If the getState function returns null at this point, the whole system is in an undefined errornous state, and we need to ensure that does not happen rather than muting the exception. Can you reproduce this?

xxx commented 6 years ago

@wnr I'm not the creator of this issue, but my entire team is also seeing these warnings on certain page loads. This module is a dependency of react-sizeme, which is where our use comes from.

wnr commented 6 years ago

@xxx I understand. As I said, this PR will only mute the exceptions and not solve the real issue. Please open an issue at react-sizeme so that they can debug this issue. These symptoms are often caused due to invalid use of this library - that elements are being removed or having its children overwritten before calling uninstall. This library is powering very large apps and is used a lot, so I'm quite confident that the issue is not at this end. I'll close this PR for now.

Shirley89 commented 6 years ago

@wnr I am afraid what @xxx said is what I met, and maybe react-sizeme use invalid of this library, could we just fix to be compatible with unexpected situation?

wnr commented 6 years ago

@Shirley89 No sorry. I don't like the idea of sprinkling a lot of guards everywhere in the code just to mute errors due to invalid use. I'll gladly help you analyze your issue, but then you need to provide some more information and a minimal project reproducing the issue.

Shirley89 commented 6 years ago

@wnr OK, u r right