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

bug: Cannot generate static page #2006

Open
DawnMD opened this issue Oct 28, 2024 · 10 comments
Open

bug: Cannot generate static page #2006

DawnMD opened this issue Oct 28, 2024 · 10 comments
Labels
❓ question Further information is requested works as expected

Comments

@DawnMD
Copy link

DawnMD commented Oct 28, 2024

Provide environment information

  System:
    OS: macOS 15.0.1
    CPU: (8) arm64 Apple M2
    Memory: 89.05 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/Library/Caches/fnm_multishells/23543_1730102638399/bin/node
    npm: 10.2.5 - ~/Library/Caches/fnm_multishells/23543_1730102638399/bin/npm
    pnpm: 9.4.0 - ~/Library/Caches/fnm_multishells/23543_1730102638399/bin/pnpm

Describe the bug

Pages are not generated statically when prefetching in a page or directly in a layout. If we remove all prefetch (be it await or void) then pages are generated statically.

Its mostly an issue when fetching some data from a CMS, as prefetch all or each the data from CMS will be a good way to render page statically

Tried to use next 15's static page symbol after upgrading, there also no static page symbol is showing when prefetching at the page/layout

Reproduction repo

https://github.com/t3-oss/create-t3-app

To reproduce

  1. Scaffold the app with
    • App router
    • tRPC
    • Tailwind
  2. Update the build script with --debug
  3. You'll get Static generation failed due to dynamic usage on /, reason: headers

Additional information

With prefetch at page level
image

Without prefetch
image

@juliusmarminge
Copy link
Member

Isn't this correct? headers() rejects so Next knows to make the page dynamic? I think this is just an internal error that they catch, if there was an actual issue the error would show without --debug flag?

Pages are not generated statically when prefetching in a page or directly in a layout.

No, you'd need to export const dynamic = 'force-static' to force dynamic when doing dynamic stuff. If you do so the trpc procedures runs during build and the page is outputted as static:
CleanShot 2024-11-04 at 09 39 46

So I'm not sure what the issue is

@juliusmarminge juliusmarminge added ❓ question Further information is requested works as expected and removed 🐞❔ unconfirmed bug labels Nov 4, 2024
@markomitranic
Copy link

I can confirm that this is indeed a issue - a page should not opt out of normal rendering, simply because we used a prefetch. The prefetch should behave the same as any other fetch on the page, and have its result cached and tagged in the same way.

This system as it is now, limits us to either dynamically serving pages always, or losing the automatic caching at fetch level and ability to use dynamic functions. Both options suck.

I don't see a clear solution to this, apart from not using prefetching, which takes away a great feature of trpc. :(

@juliusmarminge
Copy link
Member

a page should not opt out of normal rendering, simply because we used a prefetch. The prefetch should behave the same as any other fetch on the page, and have its result cached and tagged in the same way.

I don't understand what you mean here. This is how Next 15 works unless you're using ppr and/or dynamicIO?

@markomitranic
Copy link

@juliusmarminge

From Next.js docs

Good to know: The new model in the app directory favors granular caching control at the fetch request level over the binary all-or-nothing model of getServerSideProps and getStaticProps at the page-level in the pages directory. The dynamic option is a way to opt back in to the previous model as a convenience and provides a simpler migration path.
'auto' (default): The default option to cache as much as possible without preventing any components from opting into dynamic behavior.

'force-static': Force static rendering and cache the data of a layout or page by forcing cookies(), headers() and useSearchParams() to return empty values.

Using void api.post.getLatest.prefetch(); opts the page out of the auto mode which is the default, and requires you to explicitly specify force-static. This not only means that it is a footgun that is hard to notice (I had a case where someone used prefetch in the footer, effectively making the whole website dynamic) but also will not adhere to granular, automatic, fetch level cache invalidation. It works "correctly" but it is not a good solution to put into T3 and recommend as it changes the implicit, default behaviour of Next.js in a way that is hard to notice.

I wish there was a better way to do this. I badly need it (fx i used it to resolve route URLs for links used on the client, on the server). But not at the cost of making the whole website dynamic.

@juliusmarminge
Copy link
Member

juliusmarminge commented Nov 6, 2024

I wish there was a better way to do this.

The solution is called ppr which you can opt into. I am sorry but I don't understand what's t3 specific here. See this demo for example: https://github.com/juliusmarminge/next-dynamic

It works the same was and also shows an error log in --debug mode. This is how Next works.

CleanShot 2024-11-06 at 09 55 19

If you want to have more granular static generation enable PPR in your next config and wrap dynamic components in Suspense: juliusmarminge/next-dynamic#2.

CleanShot 2024-11-06 at 10 03 03

@markomitranic
Copy link

@juliusmarminge ah, I am sorry for my mistake - this is not at all an issue related to prefetching. It is an issue related to using trpc on the server (which is very, very much a T3 issue).

A quick test shows that this page is not honouring the automatic setting.

import dayjs from "dayjs";
import { api } from "~/trpc/server";

export const revalidate = 30;

export default async function Home() {
  const hello = await api.post.hello({ text: "from tRPC" });
  
  return <div>{dayjs().format("HH:mm:ss")}</div>;
}

It opts into full dynamic, just by the virtue of using ~/trpc/server. This is something I thought we've solved almost 2 years ago on T3, you might remember all the drama/tickets surrounding this issue back when approuter T3 came out and we freaked out that we can't use TRPC in server components. I thought we solved this?

@juliusmarminge
Copy link
Member

juliusmarminge commented Nov 6, 2024

I think you have a misunderstanding of what the auto setting is. When you prefetch, you're initializing a data fetch which should be dynamic by default (Next 15 reverted their decision to cache by default in 15 cause of all the confused implicit cache behaviors)

The issue you're referring to was that every page got dynamic cause of the dynamic provider in the root layout. This is solved. Only pages that do dynamic data fetching will be dynamic in the current setup. If you want more granularity, well then you need PPR.

The reason that trpc opts you into dynamic is cause it's using headers() when we create the context. You can comment that out and call auth() only where you need to, in which case your pages which invokes only that procedure will be able to be statically generated. Note though that it will only run during build:

diff --git a/src/server/api/trpc.ts b/src/server/api/trpc.ts
index b38d2fa..d8c7076 100644
--- a/src/server/api/trpc.ts
+++ b/src/server/api/trpc.ts
@@ -22,7 +22,7 @@ import { ZodError } from "zod";
  *
  * @see https://trpc.io/docs/server/context
  */
-export const createTRPCContext = async (opts: { headers: Headers }) => {
+export const createTRPCContext = async (opts: { headers?: Headers }) => {
   return {
     ...opts,
   };
diff --git a/src/trpc/server.ts b/src/trpc/server.ts
index 91e557e..587dca4 100644
--- a/src/trpc/server.ts
+++ b/src/trpc/server.ts
@@ -1,7 +1,6 @@
 import "server-only";

 import { createHydrationHelpers } from "@trpc/react-query/rsc";
-import { headers } from "next/headers";
 import { cache } from "react";

 import { createCaller, type AppRouter } from "~/server/api/root";
@@ -12,12 +11,12 @@ import { createQueryClient } from "./query-client";
  * This wraps the `createTRPCContext` helper and provides the required context for the tRPC API when
  * handling a tRPC call from a React Server Component.
  */
-const createContext = cache(async () => {
-  const heads = new Headers(await headers());
-  heads.set("x-trpc-source", "rsc");
+const createContext = cache(() => {
+  // const heads = new Headers(await headers());
+  // heads.set("x-trpc-source", "rsc");

   return createTRPCContext({
-    headers: heads,
+
   });
 });

CleanShot 2024-11-06 at 11 00 09

@DawnMD
Copy link
Author

DawnMD commented Nov 6, 2024

Isn't this correct? headers() rejects so Next knows to make the page dynamic? I think this is just an internal error that they catch, if there was an actual issue the error would show without --debug flag?

Pages are not generated statically when prefetching in a page or directly in a layout.

No, you'd need to export const dynamic = 'force-static' to force dynamic when doing dynamic stuff. If you do so the trpc procedures runs during build and the page is outputted as static:

CleanShot 2024-11-04 at 09 39 46

So I'm not sure what the issue is

@juliusmarminge okay now after reading through the explanation it makes sense on why all the page was being dynamic in the first place. Yes, I agree it was a confusion rather than an issue as we need to add export const dynamic = 'force-static' to force the page to statically render.

From the other comments and nextjs documentation, then if we prefetch using searchParams it also it will opt into dynamic rendering as well? Correct me if I'm wrong?

@markomitranic
Copy link

markomitranic commented Nov 6, 2024

@juliusmarminge Thanks for your answer, it is really helpful.

Since the OP has gotten a response that satisfies their needs, i don't wish to spam this issue further. But for the sake of completeness, I'd like to ask one more question, if you'd be so kind to answer :)

I have tried the solution you proposed, and it indeed starts behaving normally again.

  • The build process is able to call the procedure and get the data without causing the page to go dynamic.
  • After a revalidation event happens, the procedure is successfully invoked again, on the server.

I would like to understand this better, could you please clarify what is the downside of removing headers? You mentioned running auth but server needs no auth anyway, and even with the client trpc, unless you are building a SaaS, auth is usually not needed.

I don't see what purpose using the x-trpc-source header serves in the first place? It seems to work both during build and at SSG runtime, and even when invoked through a Suspensed component. The sole reference I found to this is in some old code in the trpc repo discussions, where it says // x-trpc-source is not required, but can be useful for debugging :D

@DawnMD
Copy link
Author

DawnMD commented Nov 8, 2024

The solution is called ppr which you can opt into. I am sorry but I don't understand what's t3 specific here. See this demo for example: https://github.com/juliusmarminge/next-dynamic

It works the same was and also shows an error log in --debug mode. This is how Next works.
If you want to have more granular static generation enable PPR in your next config and wrap dynamic components in Suspense: juliusmarminge/next-dynamic#2.

@juliusmarminge correct me if I'm wrong here.
To my understanding, if we want more granular control over static and dynamic part we need to do PPR and wrap the dynamic calls via Suspense. So technically if we fetch (not prefetch) a trpc endpoint, then PPR should work making the whole page static and the suspense part as dynamic (as there is no prefetch and useSuspenseQuery is used inside the component)?

But while trying to build the page, its showing the page as dynamic!

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓ question Further information is requested works as expected
Projects
None yet
Development

No branches or pull requests

3 participants