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

Cannot read property 'getBoundingClientRect' of undefined #94

Closed
aaronjensen opened this issue Jun 15, 2016 · 35 comments
Closed

Cannot read property 'getBoundingClientRect' of undefined #94

aaronjensen opened this issue Jun 15, 2016 · 35 comments

Comments

@aaronjensen
Copy link
Contributor

aaronjensen commented Jun 15, 2016

I'm not sure how to reproduce it, but we just got a report for the above error on the line:

https://github.com/captivationsoftware/react-sticky/blob/master/src/sticky.js#L69

return this.refs.placeholder.getBoundingClientRect().top;
@dbarbalato
Copy link
Member

Are you doing any DOM manipulation outside of React?

@aaronjensen
Copy link
Contributor Author

We're not, but it's possible that the users who got that error has a browser plugin that does. We haven't seen the error in 3 weeks, but it did happen to 4 people, all on Chrome.

@rafalmaciejewski
Copy link

rafalmaciejewski commented Jul 7, 2016

My colleague encountered the same problem on his machine. Furthermore, it happens to him in every browser: Chrome, Safari, Firefox and Opera. There is a difference in Firefox which didn't show any console output (it should have) but the page was broken in an exact same way it was broken on other browsers.

When we console.logged this.refs.placeholder (from getDistanceFromTop method) he got undefined while I got a DOM node. However, actual placeholder element is available in the DOM in both cases.

Is there any possibility that getDistanceFromTop is called before initial render?

We're both on OS X El Capitan 10.11.4 (15E65) and building with Webpack 2.1.0-beta.15 on NodeJS v6.3.0 ¯_(ツ)_/¯
react 15.2.0 and react-sticky 5.0.3

@pvande
Copy link
Contributor

pvande commented Jul 7, 2016

@2oo Is this behavior still triggering for him if you build against an older version of React (e.g. v15.0.1)?

@rafalmaciejewski
Copy link

@pvande Yes, it occurred with React 15.0.1 as well.

Also, except for 'Cannot read property' error there is this one:
Error: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component'srendermethod, or you have multiple copies of React loaded

If anyone has an idea what else to check we'll be glad to help.

@rafalmaciejewski
Copy link

It appears that in some cases for some reason there are two copies of React running if you import npm version of react-sticky. Instead of importing npm version I used es6 source code and built the whole app with one webpack config. It worked!

@pvande
Copy link
Contributor

pvande commented Jul 13, 2016

That's a fascinating lead... Thanks for doing the legwork on that!

/cc @dbarbalato

@homeslicesolutions-zz
Copy link

I am also running into this issue and it has nothing to do with multiple versions of React. There is some "stickyness" because I am using Redux. Now with Redux, components can "self-refresh" a maybe refresh quickly right before a refresh call from the parent component which say was planning planning to unmount that child component. So because of that, maybe the sticky.js needs a fix to deal with those things?

@pvande
Copy link
Contributor

pvande commented Sep 8, 2016

@homeslicesolutions If that's an accurate description of the problem, then it sounds like the problem actually lies with React's lifecycle methods not being called in a useful order. Can you try logging the react-sticky lifecycle methods, and report back what you find?

@homeslicesolutions-zz
Copy link

homeslicesolutions-zz commented Sep 12, 2016

@pvande possibly yes. But I know that I'm not the only one that have issues with React lifecycle and integration with Flux. I believe React is finding a way to mitigate this with some intelligent "queueing". In the meantime, an easy fix can be included in the react-sticky codebase. Just a simple check for "null" when you're trying to pull the "placeholder" node in the sticky codebase. I'll send a PR soon.

@pvande
Copy link
Contributor

pvande commented Sep 12, 2016

@homeslicesolutions I would be very interested in seeing an example that fired the lifecycle methods out of order. This Codepen is a great place to start a bug report from...

@homeslicesolutions-zz
Copy link

homeslicesolutions-zz commented Sep 13, 2016

@pvande So to preface my issue. We are stuck at React v14 right now for various admin reasons and possibly an upgrade to v15 would fix it because I believe they fixed some of their queueing issues. So maybe this issue is a non-issue, but I do have a PR anyway if you like to consider the small change: #124. BTW, this change made my application not error out and may be a good idea if people need to stay in version 14 of React

@pvande
Copy link
Contributor

pvande commented Sep 13, 2016

@homeslicesolutions To be clear, if react-sticky isn't working with React 14, I still consider that a problem. My primary concern is in making sure that we're solving the right problem – I don't doubt that a null check makes the error go away, but knowing why/how we're getting a null where we don't believe we should may uncover deeper problems (either in our code or in React).

If we're using React incorrectly, we should fix that.
If our library is being used incorrectly, we should make it harder to misuse.
If React itself has a bug, we should make sure that they're aware of the problem, and we should work around it.

@homeslicesolutions-zz
Copy link

homeslicesolutions-zz commented Sep 13, 2016

@pvande I completely understand. I found out didn't have anything to do with React versions. So I took some time yesterday and today really looking into the react-sticky and in our code and I realized that the null check is unnecessary and the error shouldn't happen. But, I can send you a video of the error happening if you have an email I can send to.

I concluded the error happened because of a strange, rare race condition as many Redux actions are dispatched and reducers are being run with intertwining dependencies manipulating the state many times with React trying to catch up. I know what you're thinking... Redux? React? Race condition? Redux is functional and should solve Race condition issues! But I've experienced and read about times where things can a get rug pulled out from underneath and React erroring out unmounting ungracefully which made me think it was a React issue (probably why they use to have the "isMounted" method). I went in our code and fixed the flow and make sure I was clearing out states that should be clearing out and the react-sticky error went away.

Long story short, I'm sorry to waste your time. It was our problem all along. I will close the PR.

@pvande
Copy link
Contributor

pvande commented Sep 13, 2016

@homeslicesolutions No worries! I'm glad everything worked out!

@SpainTrain
Copy link

Any updates from anyone else who was previously experiencing this? We have seen this in the wild from our error reporting service, but cannot reproduce it at all.

TypeError: Cannot read property 'getBoundingClientRect' of undefined
1
File "webpack:///./~/react-sticky/lib/sticky.js" line 114 col 1 in t.value
return this.refs.placeholder.getBoundingClientRect().top;
2
File "webpack:///./~/react-sticky/lib/sticky.js" line 127 col 1 in t.value
var fromTop = this.getDistanceFromTop();
3
File "webpack:///./~/react-sticky/lib/sticky.js" line 49 col 1 in r.recomputeState
var isSticky = _this.isSticky();

I admittedly haven't dug into your lib's source yet, but a similar lib has users with the same issue: akiran/react-slick#555

@pvande
Copy link
Contributor

pvande commented Feb 15, 2017

@SpainTrain The source isn't too bad; I recommend taking a look!

Having said that, the places that call getBoundingClientRect() are all directly or indirectly called by Sticky.recomputeState. That method is called whenever the props are updated (via componentWillReceiveProps) and via event handlers on window (which are attached in componentDidMount and unattached in componentWillUnmount).

The things that would me most useful in diagnosing this error would be a) evidence that getBoundingClientRect is being called from outside one of those two source (which would be an easy fix!), or b) a scenario where recomputeState is being called after the component has unmounted.

@SpainTrain
Copy link

Thanks for pointing us in the right direction. I will add your comments to our internal tracking ticket. For some reason this has started occurring a lot, so hopefully we can track it down.

@SpainTrain
Copy link

I was able to git bisect and find where this issue was introduced. It turns out that for us it was caused by the New Relic Browser agent. Based on your explanation of what would cause this error to be thrown, I might guess that the NR Browser agents futzes with the DOM elements that the components hold a ref to or the events on window.

Removing NR Browser immediately cleared the issue, so anyone else experiencing this issue should try the same.

@chaintng
Copy link

chaintng commented Mar 2, 2017

I have this problem once install new relic browser also.

@199911
Copy link

199911 commented Apr 19, 2017

I got this problem too and I am using new relic browser also

@dbarbalato
Copy link
Member

No longer relevant with 6.x

@Vadorequest
Copy link

Vadorequest commented May 6, 2017

I've also just run into the same issue, using the most simple Sticky example. I've created a dedicated branch for anyone who wants to dive in at https://github.com/Vadorequest/chatounerie-du-luberon/tree/sticky-issue

Run npm install
then npm start

Incriminated file is https://github.com/Vadorequest/chatounerie-du-luberon/blob/sticky-issue/src/components/Navbar.js#L26

image

image

Hope it helps.

@lukemarsh
Copy link

lukemarsh commented Jun 27, 2017

@dbarbalato any updates on this? The issue is still happening with the latest version

@dbarbalato
Copy link
Member

I'm not personally looking into it, no.

@witalobenicio
Copy link

witalobenicio commented Jun 29, 2017

This problem occurs probably because react-sticky is using ref as string, instead of setting to a variable.
How react-sticky do:

ref={'component'}

How it SHOULD do:

ref={(component) => this.stickyComponent = component}

Just an example

@gregoralbrecht
Copy link

gregoralbrecht commented Apr 13, 2018

I know this is an old issue but I was wondering if anyone actually got this to work? We're using react@16.2.0 and I just can't get it to work, even if I set everything up just like the examples. The <Stick /> just stays where it is and the TypeError: Cannot read property 'getBoundingClientRect' of undefined error pops up for every invocation.

Any help is much appreciated :)

Edit: I also get this:

Warning: Stateless function components cannot be given refs. Attempts to access this ref will fail.

Check the render method of `Sticky`.

@vcarl
Copy link
Contributor

vcarl commented Apr 13, 2018

Could you post your code @gregoralbrecht? At least the parts where you're using react-sticky :) Even better would be a Code Sandbox or something I can poke around!

@rafalmaciejewski
Copy link

@gregoralbrecht In my case issue was caused by two versions of react running in single runtime. React sticky was running on different version than components in my app that were used within Sticky. I use webpack and i handled it by defining an alias so webpack would use only fixed version of react for entire bundle. You can follow up on this solution here: https://webpack.js.org/configuration/resolve/

@gregoralbrecht
Copy link

@vcarl Here you go: https://codesandbox.io/s/ww8vq8rzww

@djandrzej Thanks for the response! Unfortunately, I can't touch the webpack config...

@vcarl
Copy link
Contributor

vcarl commented Apr 15, 2018

I see a few problems off the bat @gregoralbrecht, here's a working sandbox https://codesandbox.io/s/9op7mw8v4y

One of the problems is what the warning is telling you. Stateless function components cannot be given refs, so when Sticky tries to get a ref of the rendered component to measure, it ends up with undefined, ultimately giving you the error on this issue. You can fix that by using a class component.

Another problem is that you're not taking in the style argument from the Sticky callback, which is the mechanism used to actually make the component stay where it is. The callback needs to be ({ style }) => at the bare minimum to work, and your ActionBar component needs to pass the style prop to the actual DOM.

@gregoralbrecht
Copy link

@vcarl Thank you so much for looking into this so quickly! I suppose I should've figured out to use a stateful component (I actually started out with one), however it wasn't clear from the docs that you're required to pass to style prop.

The docs say:

style (object) - modifiable style attributes to optionally be passed to the element returned by this function

Do you want a PR to add the two requirements (style prop and "stateful React component") to the docs? I might be the only one stupid enough to not figure it out, but maybe it helps someone else?

Thanks again! 🚀

@vcarl
Copy link
Contributor

vcarl commented Apr 16, 2018

A doc PR would be great! I was also thinking of adding a dev-mode check for if the ref is undefined, logging a more helpful warning.

@harrygreen
Copy link

I didn't get the Stateless function components cannot be given refs warning, but it was the problem in my case. Thanks @vcarl.

@arshaan-abh
Copy link

I had the same issue. I'm not sure why, but it got solved when I wrapped inside of my Sticky element with a div tag.
I turned this:
<Sticky>{({style}) => <Image style={style} width="256" height="256" src="/assets/icon/windows.svg" alt="Windows"/> }</Sticky>
To this:
<Sticky>{({style}) => <div style={style}> <Image width="256" height="256" src="/assets/icon/windows.svg" alt="Windows"/> </div> }</Sticky>

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

No branches or pull requests