IE11 - Unable to get property 'getBoundingClientRect' of undefined when unmounting sticky with resize event
Opened this issue · 6 comments
I'm submitting a ...
- bug report
- feature request
- support request
If you're reporting a bug, please provide a minimal demonstration of the problem
Hey first of all thanks for writing this great library!
I have an event listener that will hide the component containing react sticky on a window resize event. In IE11, I hit this exception: "Unable to get property 'getBoundingClientRect' of undefined or null reference". This happens because the component unmounts, and this.node becomes null. After that, the resize handler triggers for Container.js and throws on this line https://github.com/captivationsoftware/react-sticky/blob/master/src/Container.js#L48. This doesn't happen in Chrome. The event listener is not triggered, so the exception is never hit. Not sure why order execution is different there.
Below I have a demo of my use case; however, that website does not seem to work in IE11.
What is the current behavior?
IE11, hit exception, "Unable to get property 'getBoundingClientRect' of undefined or null reference", when this.node is null in Container.js.
What is the expected or desired behavior?
I have a work around where I use set timeout to wrap my event handler that hides the sticky component. That allows ReactSticky to have its handler execute, and then my custom handler can execute to hide it. However, I think it'd be better for Container.js to null check this.node before calling getBoundingClientRect in the event listener so this won't happen.
Why do you want this? What use case do you have?
IE11 support. Better support for hiding sticky with resize event.
What is your environment?
- Version: 6.0.3
- Browser: Internet Explorer 11
Is there anything else I should know?
Hmm, this should be handled by the unmount logic, which cancels any outstanding animation frames. I don't have an IE11 environment handy, so any digging you're able to do on this would be great. I'm curious to see if the componentWillUnmount logic is running before the exception gets thrown.
I can confirm that componentWillUnmount runs before the exception is thrown. There may be some interaction happening with the dom4 library. In IE11, dom4 is used for window.removeEventListener. That library uses a polyfill for request animation frame as well, so there may be a conflict there. From their readme:
requestAnimationFrame and cancelAnimationFrame are polyfilled too but the least legacy fallback to setTimeout does not support accurate timing and doesn't slow down execution with that logic. Feel free to load upfront other polyfills if needed.
Edit: Some of those previous comments about polyfills were wrong. IE11 does support RAF natively and the bug occurs without dom4 as well.
Hmm interesting. It definitely seems possible that raf and dom4 might be interacting badly. Arguably it's beyond the scope of react-sticky to polyfill requestAnimationFrame, but changing that would be a breaking change in SemVer.
As this seems to be 2 polyfill libraries interacting badly in an old browser, I think I'm going to suggest this sit for a little while before making any code changes. We'd discussed making a null check previously, but I really prefer to avoid them in favor of keeping data predictable. If this happens in other situations we can get on it.
(just saw your edit): Let me know what else you find! I'd rather avoid a null check, since we have logic already that should be preventing this from occurring.
Little more info, this.rafHandle is null when componentWillUnmount is called, which is likely why #246 isn't addressing this issue. this.notifySubscribers is actually called after componentWillUnmount.