-
Notifications
You must be signed in to change notification settings - Fork 13
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
Better SSR support #548
Better SSR support #548
Conversation
export { SSRProvider } from "@react-aria/ssr"; | ||
export type { SSRProviderProps } from "@react-aria/ssr"; |
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.
This is the same approach as react-spectrum. I don't think we need necessarily need this export, but in case someone composes their provider differently, I think it makes sense to expose it.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@gabro bento also needs a check to see if it is SSR so that it knows to skip any reference to document, which can be found anywhere a portal is created.
bento-design-system/packages/bento-design-system/src/SelectField/SelectField.tsx Line 101 in 8b2413a
I think this is just for the docs but for completeness
|
Thank you @bvkimball for spotting these! I'm not super familiar with SSR myself, but it seems most of them are inside useEffect, which should only execute client-side, so they should be ok, does it sound right to you? I'll take a look at the ones that are not and figure out a strategy. |
@gabro Yes that does sound right, I have only gotten the error when using "SelectField" because of the useMemo. I will confirm, I did not see it anywhere else. I was just looking for the use of document. |
@bvkimball ok, thanks for confirming, I've updated the PR to fix the outstanding cases. I'll take a second pass tomorrow and merge it |
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.
Just a minor doubt, but 👍
@@ -48,7 +50,7 @@ export function NestedMenu({ | |||
offset, | |||
}); | |||
|
|||
return state.isOpen | |||
return state.isOpen && !isSSR |
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.
is there a specific reason not to use useCreatePortal
here instead of useIsSSR
and manual check?
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.
this createPortal
is not our createPortal
utility but the standard one from react-dom
. Not sure exactly why it wasn't implemented like that, @federico-ercoles may know, but that's the reason why I'm using isSRR
here
This prevents accessing document during SSR, which wouldn't work.
To use Bento in an SSR environment, the consumer needs to wrap the app in
SSRProvider
from react-aria. However, react-aria is an implementation detail, and we don't document this anywhere.This PR:
SSRProvider
from react-ariassr
prop toBentoProvider
to that the provider can be applied automatically (without even knowing about react-aria)