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

Performance issue between page navigation #1

Closed
AdisonCavani opened this issue Dec 3, 2022 · 4 comments
Closed

Performance issue between page navigation #1

AdisonCavani opened this issue Dec 3, 2022 · 4 comments

Comments

@AdisonCavani
Copy link

Hi. I implemented this code in my app, and I saw a massive performance hit. Navigating between pages is super slow compared to "normal" approach. Is it possible to mitigate this issue?

Without: https://adison-ro86xqv0p-adisoncavani.vercel.app/
With: https://adison-hi0d8ocz5-adisoncavani.vercel.app/

@joulev
Copy link
Owner

joulev commented Dec 4, 2022

From what I see, what happened is that your dynamic routes (distro-grub-themes/[[...slug]]/page.tsx and blog/[slug]/page.tsx) are server-side rendered (SSR) (in the With version) instead of generated at build time (SSG) (in the Without version). When you navigate to a dynamic page once, then navigate away, then navigate back, the route change is instant (soft navigation), which is a typical SSR behaviour in app directory.

What commit corresponds to each version you mentioned above? Also could you paste the build log for both builds (without personal info) so I can have some idea whether it really does SSR or not?

@AdisonCavani
Copy link
Author

@joulev
Copy link
Owner

joulev commented Dec 4, 2022

The issue can be reproduced with a dynamic route and seems to be a bug on Vercel's runtime.

With commit 67414bd, while locally next build works fine, on Vercel the behaviour is similar to yours.

I will investigate further to see if I can do anything, and (hopefully) gather enough info to submit a bug report.

@joulev
Copy link
Owner

joulev commented Dec 4, 2022

Looks like the bug already has a PR open: vercel/next.js#43603. I recommend subscribing to that PR and upgrade to canary the moment it ships. Upgrade your app to at least next@13.0.7-canary.5.

For now, if search parameters are irrelevant to your application, you can remove useSearchParams from the RouterEventWrapper function.

- import { usePathname, useSearchParams } from "next/navigation";
+ import { usePathname } from "next/navigation";

  function RouterEventWrapper({
    onStart = () => {},
    onComplete = () => {},
    children,
  }: React.PropsWithChildren<{ onStart?: () => void; onComplete?: () => void }>) {
    const [isChanging, setIsChanging] = useState(false);
    const pathname = usePathname();
-   const searchParams = useSearchParams();
-   useEffect(() => setIsChanging(false), [pathname, searchParams]);
+   useEffect(() => setIsChanging(false), [pathname]);
  
    useEffect(() => {
      if (isChanging) onStart();
      else onComplete();
    }, [isChanging]);
  
    return (
      <StartRouterChangeContext.Provider value={() => setIsChanging(true)}>
        {children}
      </StartRouterChangeContext.Provider>
    );
  }

Then it would work fast on Vercel as well. (For some reasons this useSearchParams bug didn't reproduce on next build, or is my computer suddenly fast?)

@joulev joulev closed this as completed in b23ea8d Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants