vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.43k stars 26.55k forks source link

cookies break caching/memoization #58288

Open IonelLupu opened 9 months ago

IonelLupu commented 9 months ago

Link to the code that reproduces this issue

https://github.com/IonelLupu/nextjs-cookies-break-caching

To Reproduce

  1. use const cookiesStore = cookies() from import { cookies } from 'next/headers' in a server rendered page in the app directory
  2. make a request to a server and log something when that endpoint is reached
  3. go to the page that uses the cookies and watch how the logs are printed on the server every time your refresh the page due to caching being broken

If you comment this one const cookiesStore = cookies(), the caching works correctly (almost)

One other note: when I have the same fetch request both in the page and in generateMetadata, nextjs doesn't deduplicate the request. I can see two logs in my console instead of one.

Current vs. Expected behavior

Current: the fetch API responses are not cached when using const cookiesStore = cookies()

Expected: the fetch function to cache the API response

Verify canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.2.0: Fri Oct 13 09:27:54 PDT 2023; root:xnu-10002.60.54~14/RELEASE_ARM64_T6000
Binaries:
  Node: 18.12.1
  npm: 8.19.2
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 14.0.3-canary.0
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

(node:32856) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

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

App Router

Additional context

No response

mareksuscak commented 9 months ago

This is expected in specific scenarios but I think it's buggy or the documentation is inaccurate. The documentation claims that fetch requests are NOT cached if they come after the usage of headers() of cookies(). It's not clear if that opts out all fetch requests from using the data cache or just the ones that came after. Also, from our experience, even this is not true as moving cookies() after fetch() in the page component did not lead to fetch using the data cache. This either needs to be fixed or documentation updated to reflect the current implementation.

EDIT: Tried moving cookies to a child server component and that didn't help either. Even if wrapped in a Suspense boundary.

EDIT2: One of our devs claims that the data cache works as expected when running a production build locally but not on Vercel.

externl commented 9 months ago

I'm also experiencing this issue.

Not cached between different requests sure, but fetch requests should still be memoized during a component tree render? The documentation is not very clear.

IonelLupu commented 9 months ago

This is expected in specific scenarios

The problem is: we are using cookies() in the RootLayout of the app, making every single page in the project dynamic. We have some user specific hooks used across the app that need the values from the cookies in order to work.

For example, we are using Growthbook, an A/B testing SDK. In order to have the A/B tests properly set for the users, we need to wrap the entire children from RootLayour in the <GrowthbookProvider userId={userIdFromCookies}> component.

The solution for this problem is to use the <GrowthbookProvider> only in the components we need. But this will make use wrap GrowthbookProvider hundreds of times. But as @mareksuscak pointed out here:

EDIT: Tried moving cookies to a child server component and that didn't help either. Even if wrapped in a Suspense boundary.

... it seems it doesn't matter....

arjendevos commented 2 months ago

It seems that without cookies it also won't work. When i make a simple request: await fetch('http://localhost:8080/health'); my api get's called 2 times.

Edit: tried multiple next version and only worked in next@15 with react@19.