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

Reading request.cookies in a GET app route does not make the route dynamic #49006

Closed
1 task done
jmurty opened this issue Apr 30, 2023 · 5 comments
Closed
1 task done
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. locked

Comments

@jmurty
Copy link

jmurty commented Apr 30, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

    Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.4.0: Mon Mar  6 20:59:28 PST 2023; root:xnu-8796.101.5~3/RELEASE_ARM64_T6000
    Binaries:
      Node: 16.16.0
      npm: 8.11.0
      Yarn: 1.22.17
      pnpm: N/A
    Relevant packages:
      next: 13.4.2-canary.4
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.5

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true)

Link to the code that reproduces this issue

https://github.com/jmurty/next-issue-non-dynamic-cookies

To Reproduce

  • Clone example project
  • npm run build && npm run start
  • Note that route /api/non-dynamic-cookies builds as Static, not Server
  • Hit the route with e.g. curl http://localhost:3000/api/non-dynamic-cookies -H 'Cookie:a=b' and see the timestamp remains unchanged, and cookies provided are not returned
  • As a counter-example the /api/dynamic-cookies builds as Server, and if hit with e.g. curl http://localhost:3000/api/dynamic-cookies -H 'Cookie:a=b' returns a new timestamp each time and the provided cookie values
  • Update the non-dynamic-cookies.ts route file to comment-in any of the other attribute lookups on the request e.g. request.url. Upon rebuild and restart, the route becomes dynamic

Old steps

Reproduce bug when reading fromrequest.cookies:

  • Implement an app route endpoint with only a GET function that reads cookie data from request.cookies
  • Run project, either self-hosted (e.g. yarn build && yarn start) or deployed to Vercel
  • Sent a GET request with or without cookies to the app route – e.g. https://next-issue-non-dynamic-cookies.vercel.app/api/non-dynamic-cookies
  • App route does not run dynamically, it instead returns content cached at build time

Counter-example, no bug when reading cookies imported from "next/headers" :

  • Implement an app route endpoint with only a GET function that reads cookies imported from "next/headers"
  • Run project, either self-hosted (e.g. yarn build && yarn start) or deployed to Vercel
  • Sent a GET request with or without cookies to the app route – https://next-issue-non-dynamic-cookies.vercel.app/api/dynamic-cookies
  • App route does run dynamically

Describe the Bug

In 13.4.1 reading cookie data in a GET app route from the request.cookies attribute provided by NextRequest does not trigger Next.js to make the app route dynamic. Instead the app route is mis-categorised as a static route at build time, and will return the cached static content forever.

Workarounds for now are to either:

  • read cookie data from cookies via import { cookies } from "next/headers" instead of from request.cookies
  • force the app route to become dynamic in any other way, e.g. add export const revalidate = 0 or add a POST() function or…

Expected Behavior

Accessing the request.cookies attribute within a GET app route should mark it as dynamic, like what happens if you access other attributes of the request like url or headers, or when you access cookies via import { cookies } from "next/headers"

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

next start & Vercel

@jmurty jmurty added the bug Issue was opened via the bug report template. label Apr 30, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label Apr 30, 2023
@jmurty
Copy link
Author

jmurty commented Apr 30, 2023

Here is an example GET app route that remains static despite reading from request.cookies:
https://github.com/jmurty/next-issue-non-dynamic-cookies/blob/7368210fcbbb974055379cbf313a18e209190555/app/api/non-dynamic-cookies/route.js#L1-L24

Note that uncommenting use of other request attributes in this app route will make it dynamic: request.url, request.headers, request.json()

Here is a nearly identical GET app route that is correctly treated as dynamic because it reads cookie data via import { cookies } from "next/headers":
https://github.com/jmurty/next-issue-non-dynamic-cookies/blob/main/app/api/dynamic-cookies/route.js

@jmurty jmurty changed the title Reading request.cookies in an GET app route does not make the route dynamic Reading request.cookies in an app route with just GET does not make the route dynamic Apr 30, 2023
@jmurty jmurty changed the title Reading request.cookies in an app route with just GET does not make the route dynamic Reading request.cookies in a GET app route does not make the route dynamic Apr 30, 2023
@jmurty
Copy link
Author

jmurty commented Apr 30, 2023

All Next.js versions that support app routes in the app/ directory have the same behaviour. I tried versions 13.2.4 & 13.2.0

@jmurty
Copy link
Author

jmurty commented Apr 30, 2023

Give this subtle bug, and others that might be lurking, can I suggest that you rethink the current approach where GET function app routes are automatically determined to be dynamic or static based on vague criteria and a sprinkling of black magic?

It would be much simpler if app routes were always dynamic by default, but could be made static with the explicit and obvious addition of revalidate = false in the appropriate place(s).

Here is the current documentation about this:
CleanShot 2023-05-01 at 00 57 38@2x

Based on my experiments tracking down this bug, I think these seemingly simple guidelines need to have the following caveats spelled out clearly. (However I'm still not sure I'm right about my additions below, it's not at all clear exactly what is going on under the hood)

Route handlers are evaluated dynamically when:

  • Using the Request object with the GET method.

Provided you call one of a specially-blessed subset of attributes on the Request object. Just passing in the Request object to the function is not enough, you must also access the request appropriately within your code.

  • Using any of the other HTTP methods.

Here "using" doesn't mean making e.g. POST requests from a client, which is what I assumed at first. It really means adding code for any non-GET functions to the app route file. If you have a static GET function working as intended, you might be surprised when you later add a POST function to that app route and suddenly find the old GET function has become dynamic. Just adding the following triggers dynamic behaviour for the entire app route file: export async function POST() {}

"Using" here means calling a blessed attribute of the cookies or headers imports in the GET function code. This is fairly obvious, but worth noting that unlike adding the "use client" marker at the top of a React component file, adding import { cookies } from "next/headers" to a GET-only app route file doesn't change it's behaviour at all unless your code also accesses a blessed attribute.

@jmurty
Copy link
Author

jmurty commented Dec 9, 2023

This issue is fixed for me in version 14.0.4 most likely by the PR #58769. Thanks!

@jmurty jmurty closed this as completed Dec 9, 2023
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. locked
Projects
None yet
Development

No branches or pull requests

1 participant