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
avoid warning if using usePreventScroll on server #1315
Conversation
I made this change using the GitHub UI after testing it locally. Feel free to just close this PR and bring this fix home separately as the lint step is failing atm. Thanks for making |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! thanks for the contribution, one quick question
import {chain, getScrollParent} from '@react-aria/utils'; | ||
import {useLayoutEffect} from 'react'; | ||
import {useEffect, useLayoutEffect} from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you try with import {chain, getScrollParent, useLayoutEffect} from '@react-aria/utils';
? It should do a roughly similar thing https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/utils/src/useLayoutEffect.ts
Maybe it needs updating to this? I don't know quite enough about SSR personally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works as well. Done 👍
The problem
When a component using
usePreventScroll
gets rendered on the server, it would emit a warning likeThis happened as
useLayoutEffect
is being run on the server.This leads to questions like https://stackoverflow.com/questions/64622494/usepreventscroll-causes-uselayouteffect-warning-in-nextjs/64623138.
The solution
To avoid the warning on the server, we can fall back to
useEffect
in server environments. This gets rid of the error and should have no effects on the behaviour.I skipped the PR template as this is quite a small change where I applied a quite common solution. So it didn't seem worth the overhead. Hope that's okay!
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
usePreventScroll
hook in a component that gets rendered on the server (e.g. in Next.js)Before this fix, you'd see a warning. After this fix, the warning is gone. ☮️