search

floating-ui / react-popper

🍿⚛Official React library to use Popper, the positioning library
https://popper.js.org/react-popper/
MIT License
2.5k stars 226 forks source link

Fix Reference & Manager compatibility with React 18's StrictMode #459

Closed PetrusAsikainen closed 4 months ago

PetrusAsikainen commented 1 year ago

Reference, at least in how BlueprintJS is using it, seems to be broken in React 18's StrictMode due to effects running twice. (This causes popovers to no longer close when clicking outside them and was very fun to track down.)

Fixing Reference

https://github.com/floating-ui/react-popper/blob/master/src/Reference.js#L17-L27

In Reference, the refHandler passed to children as ref is only called by React once on initial mount. However, React 18 calls the setRef(innerRef, null) cleanup function after the ref has mounted, and innerRef ends up being cleared and left to null.

Since React claims to always unmount ref callbacks, I couldn't see a reason to manually set innerRef to null in a useEffect - so I'd think to just remove it.

Fixing Manager

Additionally, Manager uses a "one-way" effect to keep track of mounting/unmounting - this is no longer compatible with React 18, so I added a line to reset the ref state when the effect is "remounted". Didn't bother tracking down a concrete bug with this, but at least this change shouldn't break anything.

adidahiya commented 11 months ago

hello @FezVrasta can you please take a look at this PR?

tehhowch commented 4 months ago

landing this fix would really help those who want to use Strict Mode & react 18 but don't yet have the bandwidth to migrate away from react-popper, too

tehhowch commented 3 months ago

This may have helped somewhat, but i'm still seeing an issue in react 18's strictmode with a null element ref. Will need to fork and investigate more, as it's a "dep of a dep" scenario