-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fixed SSR error in Menu
#16559
Fixed SSR error in Menu
#16559
Conversation
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
One small question, why do we change the target to element
from HTMLElement
? is it because it won't be recognised as DOM element on server?
No, actually we could kept the
|
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.
🥳
6cbcba7
Hey there! v11.60.0 was just released that references this issue/PR. |
Closes #15531
Closes #16004
Closes #16251
We are adding a checker to handle the
document.body
to avoid error when using SSR if the document is not rendered.Also we have to guard if the
target
is undefined, becausecreatePortal
does not accept an undefined elementFor some reason the eslint still yelling about the SSR, I tried multiple ways of guarding that. The first commit uses the
useIsomorphicEffect
but it was broking the tests.But I would like to check with the usesr having this problem, to see if this would fix it. Or I can try to create a quick demo, if there is time, to see if that works before releasing.
Testing / Reviewing
Menu
are working as expected.