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

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

Merged
merged 1 commit into from May 8, 2024

Conversation

PetrusAsikainen
Copy link
Contributor

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
Copy link

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

@tehhowch
Copy link

tehhowch commented May 8, 2024

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

@FezVrasta FezVrasta merged commit 2bbdd4a into floating-ui:master May 8, 2024
@tehhowch
Copy link

tehhowch commented May 9, 2024

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

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

Successfully merging this pull request may close these issues.

None yet

4 participants