Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IE11 - Unable to get property 'getBoundingClientRect' of undefined when unmounting sticky with resize event #279

Open
1 of 3 tasks
JohnReagan opened this issue Feb 26, 2019 · 6 comments

Comments

@JohnReagan
Copy link

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.

Bug Demo

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?

@vcarl
Copy link
Contributor

vcarl commented Feb 26, 2019

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.

@JohnReagan
Copy link
Author

JohnReagan commented Feb 26, 2019

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.

@vcarl
Copy link
Contributor

vcarl commented Feb 26, 2019

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.

@vcarl
Copy link
Contributor

vcarl commented Feb 26, 2019

(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.

@JohnReagan
Copy link
Author

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.

@Anenth
Copy link

Anenth commented Apr 30, 2019

I have a similar issue, but it happens in Mobile safari mainly and IE11
#266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants