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

Server-rendered Navbar prevents static page prerendering #336

Open
GabeDahl opened this issue Apr 30, 2024 · 8 comments
Open

Server-rendered Navbar prevents static page prerendering #336

GabeDahl opened this issue Apr 30, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@GabeDahl
Copy link

The Navbar component is server-rendered. Since it is included in the root layout, this forces every single page to be server-rendered on demand (dynamic).

Because of this, the time to first byte often exceeds 1 second.

If the maintainers are open to it, I can rework the Navbar and its auth-awareness to avoid this hiccup.

@thorwebdev
Copy link
Collaborator

Thanks @GabeDahl for digging into this and reporting!

@dalkommatt and @chriscarrollsmith I remember y'all doing this as part of the awesome Next.js 14 rework. Do you happen to have any guidance on this?

@thorwebdev thorwebdev added the enhancement New feature or request label May 1, 2024
@chriscarrollsmith
Copy link
Contributor

@thorwebdev @GabeDahl I have an open PR with a full rework of the navbar:

#313

I can't speak to the time to first byte or the rendering sequence, because I haven't done any benchmarking on it. But I think it may resolve the problem? Give it a look.

@GabeDahl
Copy link
Author

GabeDahl commented May 1, 2024

@chriscarrollsmith I cloned your PR and removed all the async stuff from the home page (pricing page). So the root page.tsx is just:

export default function PricingPage() {
  return (
    <div>hello world</div>
  );
}

This should result in a static, pre-rendered page during the build process.
However:
image

You can see that all routes are still dynamic, and thus rendered on-demand at request time. This means that a website visitor will have to wait for server to cold-start & render the route instead of just serving the static content. (hence, a long time to first byte)

If you remove the navbar, static generation and pre-rendering work as expected.
So it appears that including a server component in the root layout prevents static rendering.

@GabeDahl
Copy link
Author

GabeDahl commented May 1, 2024

I ended up replacing the server-rendered component with a client component (I get the session, but that can be easily swapped for user):

'use client'
import { createClient } from '../../../utils/supabase/client';

...

export default function Header() {
  const path = usePathname();
  const [session, setSession] = useState<Session | null>();
  useEffect(() => {
    supabase.auth.getSession().then(({ data: { session } }) => {
      setSession(session);
    });

    const {
      data: { subscription }
    } = supabase.auth.onAuthStateChange((_event, session) => {
      setSession(session);
    });

    return () => subscription.unsubscribe();
  }, [path]);
  return (

...

It's not very elegant, but that's how I worked around some wonky issues with Next's root layout and direct child components (in other words, child components that are not part of any page, just the root layout)

@dalkommatt
Copy link
Contributor

dalkommatt commented May 1, 2024

Hi @GabeDahl, I'm interested to hear how you would rework it. The whole point of a navbar is that it's present on every page. I'm really hoping partial pre-rendering goes stable in Next.js 15, as it should solve this issue automatically if the <Navbar> component is wrapped in <Suspense>. Until then the only other alternative I'm aware of would be to create a sign in/sign out client component wrapped in <Suspense> and fetch the user client side, but you may run into issues there as well because supabase/ssr's createBrowserClient is not able to recognize a server-side sign in without the page being refocused or having a hard refresh.

@GabeDahl
Copy link
Author

GabeDahl commented May 1, 2024

@dalkommatt You're right. SSR's browser client doesn't recognize server-side auth state changes. That's why I hacked together that ugly useEffect that fires whenever the pathname changes or whenever supabase.auth.onAuthStateChange fires (to catch client-side auth updates).

My "fix" assumes that any server-side auth updates are accompanied with navigation/redirects. This repo already does that with the handleRequest and SignOut from /utils/auth-helpers/..., but that's a dangerous assumption to apply generally.

Partial pre-rendering is definitely the ideal solution. So I see three options:

  1. Keep the current dynamic Navbar and endure a really slow TTFB.
  2. Use my workaround which [dangerously] assumes devs will navigate following any server-side auth updates.
  3. Use the experimental partial pre-rendering

It's honestly a tough call. I'll personally opt for 2 or 3 in my projects, but that might not be ideal for a template/repo like this.

@chriscarrollsmith
Copy link
Contributor

Interesting, @GabeDahl. I appreciate your work on this. I agree that #2 or #3 is the way to go. Thanks also for the sample code; I will have to play around with that a bit. In our work on this repo, we were trying to keep auth management fully server-side for security purposes. But may have wedded ourselves a little too rigidly to that paradigm, especially considering that full SSR support in Next.js and Supabase was (and still is) still relatively new.

@Raspber
Copy link

Raspber commented May 21, 2024

This might be very pointless and not a good solution. I do not have a navbar in my layout since I am using the index page as a landing page and I have a Dashboard route that holds a layout and in there I have my main navbar that renders if the user is logged in, if not , users are redirected to the landing page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants