Big performance regression in 0.7.5

[echenley]

It looks like _createPopper is now being run on every render, which is reinstantiating popper and causing a full browser repaint. In our fairly complex app, this is actually causing Chrome to freeze in certain scenarios, especially on lower end hardware. Pinning to 0.7.4 alleviated our issues.

@pzatrick

[patreeceeo]

Looking in to it.

[patreeceeo]

I'm getting close to a solution, but I've run in to what seems to be an issue with PopperJS. In this bin I've commented out the else branch inside the ref function and added a check to prevent re-creating the PopperJS instance in _createPopper. (I dislike it when methods themselves check whether they should actually do what their names imply, but did it in this case to correspond to _destroyPopper, and I'm not sure if there's a good reason for it.) That fixes @echenley's performance issue, though it causes a regression on #77.

I was trying to re-fix #77 when I added a call to scheduleUpdate inside componentDidMount. For some reason one call to scheduleUpdate was not enough, so I wrapped it in a setInterval. In the bin linked above you can see the popper gradually move with each call until it's in the correct position. @souporserious Any idea why this is happening? Figured I'd ask you before creating an issue in PopperJS.

[souporserious]

Ah sorry about that! I'll see what I can do to fix it. Please use 0.7.4 for now.

[souporserious]

Sorry, just saw your comment @pzatrick, I'm not entirely sure off the top of my head. @FezVrasta any ideas? I'm guessing this has to do with react-popper and not PopperJS.

[FezVrasta]

I think there's an open issue for the problem @pzatrick mentioned...

[souporserious]

Still haven't had a chance to work on this, but I just remembered this about refs https://reactjs.org/docs/refs-and-the-dom.html#caveats that's definitely a sure way to cause perf issues 💩, makes sense looking at it now. @echenley if you or someone else wants to submit a PR with a bound function that would be awesome, otherwise I should be able to get to this over the weekend or sometime next week.

[souporserious]

Fixed 🎉 d109fe5

[KurtPreston]

@souporserious Just tossing out there, it may be worth unpublishing the 0.7.5 release? In our app, we found that some computers worked okay with it, but some computers would literally skyrocket to 100% CPU usage with a memory leak the moment a tooltip was triggered, effectively destroying the app. (We were not able to figure out why there was such a marked behavioral difference from computer to computer.)

Since some other libraries are pinned to react-popper ^0.7.x, I think it might not be desirable to have the 0.7.5 release in the wild. Just my 2¢.

Thanks for the library!

[souporserious]

Thanks for the heads up @KurtPreston! I wasn't able to unpublish unfortunately, but I deprecated it so hopefully that helps.