-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
backport: reading middleware cookies during render #69224
base: 14-2-1
Are you sure you want to change the base?
Conversation
Hi there 👋 It looks like this PR introduces broken links to the docs, please take a moment to fix them before merging:
Thank you 🙏 |
Failing test suitesCommit: 6ff29d2
Expand output● app dir - with output export - dynamic api route prod › production mode › should work in prod with dynamicApiRoute 'force-static'
Read more about building and testing Next.js in contributing.md. |
beb7f7c
to
a73a501
Compare
c8b36dc
to
c82d6be
Compare
### What Cookies set/updated/removed in middleware won't be accessible during the render in which they were set ### Why Middleware will properly set a `set-cookie` header to inform the client of the cookie change, but this means the `AsyncLocalStorage` context containing the cookies value wouldn't be updated until the next time the request headers were parsed. In other words, on the first request the cookie would be sent but wouldn't be available in the `cookies()` context. And then the following request would properly have the cookie values. ### How This uses a proxy on the `ResponseCookies` used in middleware to add a middleware override header with the cookie value. When we instantiate the cached cookies, we merge in whatever headers would have been set by middleware, so that they're available in the same render that invoked middleware. ### Test Plan This changeset adds a test to confirm cookies set/deleted in middleware are available in a single pass. Verified with a deployment [here](https://vtest314-e2e-tests-ldx7olfl1-ztanner.vercel.app/rsc-cookies). Fixes #49442 Closes NEXT-1126
When we provide the `set-cookie` string in `x-middleware-set-cookie`, we need to ensure that multiple values are properly delimited. We also make sure the cookies that get passed into `RequestCookies` aren't in `ResponseCookie` form, to prevent something like `Path=/` from being part of `cookies()`. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change Closes NEXT- Fixes # -->
If middleware targets a server action handler and sets or updates a cookie, the newly updated cookie would not be reflected in the `cookies()` response of the action handler In #65008 we fixed a bug where cookies set in middleware were not reflected in the `cookies()` call in a server component from the same request. We did this by introducing a `x-middleware-set-cookie` header, that signaled to downstream handlers that middleware had run on the request & set a cookie. However this handling was only applied to the sealed/read-only cookies. Cookies accessed from a server action use `mutableCookies`, since those aren't frozen as a server action is allowed to modify cookies. This pulls the cookie merge handling into a function and applies the merge to `mutableCookies`. Fixes #67814 Closes NDX-95
This ensures that cookies merged via `x-middleware-set-cookie` preserve the options that are passed to them. Currently we're only selectively merging in `name` and `value` but really we can just copy the `ResponseCookie` in kind rather than enumerating on select properties.
c82d6be
to
6ff29d2
Compare
Backport changes related to reading middleware-set cookies in render/actions.
this required some minor surgery because RequestAsyncStorageWrapper became withRequestStore, but nothing that seems too bad