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

Parallel Routes cause redundant RSC fetches on navigation #65878

Open
steve-marmalade opened this issue May 17, 2024 · 8 comments · May be fixed by #65935
Open

Parallel Routes cause redundant RSC fetches on navigation #65878

steve-marmalade opened this issue May 17, 2024 · 8 comments · May be fixed by #65935
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Parallel & Intercepting Routes Related to Parallel and/or Intercepting routes.

Comments

@steve-marmalade
Copy link

steve-marmalade commented May 17, 2024

Link to the code that reproduces this issue

https://github.com/marmalade-labs/parallel-route-rerender

To Reproduce

  1. Create production build of the application (bun run build && bun start) or navigate to https://parallel-route-rerender.vercel.app/a
  2. Click on one of the cards in the bottom half of the page

Current vs. Expected behavior

Current Behavior
If you open the Network tab in DevTools, when you click a card you will see 21 fetches for the current route (with different ?_rsc= params), which I believe correspond to the 21 different parallel slots in the layout.

image

Maybe this is intended in the sense that each request might correspond to a specific slot. However, each request appears to be re-running the matched slots (which will always and only be /app/[letter]/page.tsx and /app/[letter]/@grid/page.tsx). I demonstrate by adding a 1 second sleep in those pages, but not in any others (which should just be rendering a no-op default.tsx), and yet we can see that each of the 21 requests takes > 1 second.

Expected Behavior
Whether or not we expect 21 different HTTP requests, I would only expect that each slot is rendered once. So we should either see 1 request that takes ~ 1 second, or 21 requests where 2 of them take 1 second and the rest are <100 ms.

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP PREEMPT_DYNAMIC Tue, 07 May 2024 21:35:54 +0000
  Available memory (MB): 31886
  Available CPU cores: 8
Binaries:
  Node: 18.20.2
  npm: 10.8.0
  Yarn: 1.22.22
  pnpm: 8.15.5
Relevant Packages:
  next: 14.3.0-canary.68 // Latest available version is detected (14.3.0-canary.68).
  eslint-config-next: 14.2.3
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Navigation, Parallel & Intercepting Routes

Which stage(s) are affected? (Select all that apply)

next start (local), Vercel (Deployed)

Additional context

No response

@steve-marmalade steve-marmalade added the bug Issue was opened via the bug report template. label May 17, 2024
@github-actions github-actions bot added Navigation Related to Next.js linking (e.g., <Link>) and navigation. Parallel & Intercepting Routes Related to Parallel and/or Intercepting routes. labels May 17, 2024
@steve-marmalade
Copy link
Author

steve-marmalade commented May 17, 2024

@ztanner , tagging you in the hopes that I've misconfigured something in a way that will be easy for you to spot 🙏. This one is kind of a big deal for us because I believe it's increasing our server costs significantly (since we're paying to do the same work many times when a user navigates to a page).

@jimapl
Copy link

jimapl commented May 18, 2024

Do you do any api fetch requests on the server side?
I've noticed this behaviour on parallel routes where I rely on session and other fetched data. Where It builds the endpoint in dev, comes back with the data and rebuilds the parallel page. over and over again.

@ztanner
Copy link
Member

ztanner commented May 18, 2024

Hi @steve-marmalade --

Thanks for the great repro and description, as always 😄

As a quick workaround for the issue you're describing here, there're a few options:

  • Add prefetch={false} to the links
  • Or remove app/[letter]/loading.tsx and implement loading behavior with a regular <Suspense> boundary

Now, for an explanation of why it's happening: prefetch requests will return RSC data up to the nearest loading boundary. This is so that a prefetch doesn't lead to overfetching data, especially if you're linking to a page that has an expensive subtree. When a route is prefetched, the idea is that clicking the link will immediately show the prefetched loading boundary, while the part that suspended is streamed in. In this case, since there's a loading.tsx in the [letter] segment, upon navigation the router thinks that all of the parallel slots are missing data (because the server sent null data for them), triggering data fetches for them all.

I think I see a way to optimize this, and will work on a PR this coming week. Thanks for the heads up!

@ztanner ztanner linked a pull request May 18, 2024 that will close this issue
@steve-marmalade
Copy link
Author

Thank you for the detailed description and the workaround (which does seem to alleviate the problem!)

Just want to clarify one thing for my own understanding:

the router thinks that all of the parallel slots are missing data (because the server sent null data for them), triggering data fetches for them all.

Even in this case where the router triggers a data fetch for each slot, I still wouldn't expect those requests to take ~1 second each in my demo, because most of the slots are just returning null from default.tsx. Put another way, why does triggering a fetch for /app/[letter]/@grid2/2/page.tsx entail running app/[letter]/page.tsx and app/[letter]/@grid/page.tsx?

@ztanner
Copy link
Member

ztanner commented May 20, 2024

Right now data is fetched per-page, rather than having the ability to more granularly fetch per-segment/slot. This is something we're planning to address soon so that the Next.js server can be smarter about only requesting the data it needs, rather than returning more data than necessary.

At the moment, when the router sees missing data for the @grid3/default segment, it triggers a fetch for the page it's currently on to retrieve that data (ie, fetch("/a")). The common layout won't be refetched, but everything in-between will. This means that it'll need to wait on any slower requests that also get matched in that request, since it's not currently possible to say "only fetch me the data for /app/[letter]/@grid3/default.tsx, but not /app/[letter]/page.tsx".

@steve-marmalade
Copy link
Author

Heya @ztanner , I know there's a ton going on -- any chance this fix makes it into Canary this week?

@ztanner
Copy link
Member

ztanner commented May 23, 2024

Hey @steve-marmalade -- I talked with some folks on the team and the concern is that, while many people use default.tsx to return null, you could just as easily have default.tsx return an expensive subtree, which would go against the reason we only return up to the loading segment for prefetches in the first place.

I'll need to think through if there's an alternate approach we could do here, which may take a little bit more time. 🙏

@steve-marmalade
Copy link
Author

steve-marmalade commented May 24, 2024

Got it, thanks for the update.

Relatedly: would you expect that scrolling to an id on the current page would also trigger the fetch requests for each slot (regardless of whether loading.tsx is present)? E.g. <Link href="#section-a">Section A</Link>. I guess I didn't expect a Link with only a hash ID to make any fetch requests to the server. For now, I can replace with javascript scrollIntoView, but figured I'd raise in case it's unexpected to you as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation. Parallel & Intercepting Routes Related to Parallel and/or Intercepting routes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants