Overlay causes useLayoutEffect warnings in SSR mode

[jclem]

Describe the bug

When using the <Overlay /> and components that build upon it in SSR mode, React emits warnings about calling useLayoutEffect during SSR.

Of course, there are legitimate uses of useLayoutEffect in Primer and cases where it may be unavoidable. However, in <Overlay />, it looks like this particular call is not doing any DOM measurement or direct manipulation, but is kicking off an animation. This seems like it's probably fine to defer until after a paint occurs, and I'm guessing that browser is probably going to do that anyway before this animation begins.

Given that, I wonder if we can move this to a useEffect call, instead or alternatively investigate using a useIsomorphicLayoutEffect, which @mattcosta7 reminded me about in Slack:

const useIsomorphicLayoutEffect = typeof document !== 'undefined' ? useLayoutEffect : useEffect

(That said, in this case, I think just using the simpler useEffect is fine.)

In order to avoid these warnings currently, I have to defer completely the rendering of overlays until we're on the client.

To Reproduce
Steps to reproduce the behavior:

1. Create a Next.js app
2. Render an <Overlay /> on some page
3. Watch logs

[pksjce]

I agree with @jclem here because there is no obvious reason for why we need useLayoutEffect.
I wonder though, Overlay component is rendered conditionally after click of a button, so it doesn't make sense that this is rendered during SSR.

Here's what I tried,

• Created a new nextjs app.
• Copied the Dialog Overlay story from Overlay stories
• Added a getServerSideProps function on my page

Apart from an error that made me add ThemeProvider(this was not clear at all), I didn't get any warnings.

When I removed the conditional from rendering logic for Overlay in my component, I saw this warning,
Warning: useLayoutEffect does nothing on the server, because its effect cannot be encoded into the server renderer's output format. This will lead to a mismatch between the initial, non-hydrated UI and the intended UI. To avoid this, useLayoutEffect should only be used in components that render exclusively on the client. See https://reactjs.org/link/uselayouteffect-ssr for common fixes.
Also the UI completely failed with
error - ReferenceError: document is not defined at Portal (/Users/pk/Github/nextjs-primer-react/node_modules/@primer/components/lib/Portal/Portal.js:66:23)

So, I'm curious about how your bug and nextjs app looks like. Ideally I'd expect nothing from Overlay to run on SSR.

[jclem]

I'll have to go back a bit and find a commit this was still happening. Sorry for the delay, but I should be able to get to it in the next couple of days.

[jclem]

I was wrong, it's not from Overlay, it's from AnchoredOverlay. It looks like for us, this was coming from AnchoredOverlay's useAnchoredPosition call here. Inside useAnchoredPosition, there is a call to useLayoutEffect here.

[pksjce]

Was able to reproduce this here
https://github.com/pksjce/nextjs-primer-react

Looking at ways to mitigate this!

[pksjce]

Linking to this exploration by @siddharthkp https://github.com/pksjce/nextjs-primer-react/pull/1/files
I will look at going in direction 2 today and if not fallback to useIsomorphicEffect and have a PR up