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

Can't set cookies in middleware #7280

Closed
1 task
xriter opened this issue Jun 2, 2023 · 6 comments · Fixed by #7294
Closed
1 task

Can't set cookies in middleware #7280

xriter opened this issue Jun 2, 2023 · 6 comments · Fixed by #7294
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@xriter
Copy link

xriter commented Jun 2, 2023

What version of astro are you using?

2.5.6

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

Reading cookies context.cookies.get(...) from middleware.ts works fine, but when trying to set a cookie context.cookies.set(...), it doesn't seem to work.

// middleware.ts
import type { AstroCookies } from "astro";

export async function onRequest({ cookies }, next) {
	(cookies as AstroCookies).set("cookieFromMiddleware", "doesntWork", {
		path: "/",
		expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 365),
	});

	return next();
}

Setting a cookie (using Astro.cookies) in a .astro file does seem to work fine.

Am I missing something, or is this a bug?

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-tgbzvq?file=src%2Fmiddleware.ts

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

matthewp commented Jun 4, 2023

I think this is a bug. Probably we are creating a new instance of AstroCookies when the page is rendered, wiping out your previous sets.

Btw, why the (cookies as AstroCookies)? Is that incorrectly typed?

@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Jun 4, 2023
@matthewp matthewp self-assigned this Jun 4, 2023
@xriter
Copy link
Author

xriter commented Jun 5, 2023

@matthewp It was set to AstroCookies because I don't know what type to use for the onRequest function. If I know that, then hopefully the cookies will get the correct type automatically?

@i7N3
Copy link

i7N3 commented Jun 7, 2023

@matthewp

I am faced with the fact that the cookie is being lost.

"astro": "2.6.1"
"@astrojs/node": "5.2.0"

How to reproduce?

const middleware = defineMiddleware(async (context, next) => {
        // ...
	if (isAuthSuccess) {
		context.cookies.set('jid', '<TOKEN>')
		return context.redirect(`/dashboard`)
	}
	return next()
})
export const onRequest = sequence(middleware)

When I log cookie value on /dashboard page with Astro.cookies.get(...).value it's empty.
If I remove the redirect statement, then it works.

@i7N3
Copy link

i7N3 commented Jun 8, 2023

Here is the reproduction: https://github.com/i7N3/astro-middleware-bug-reproduction
Node: 16.18.0 (nvm), Monterey m1, tested both: dev and preview

@i7N3
Copy link

i7N3 commented Jun 21, 2023

@matthewp Should I open a new Issue?

@matthewp
Copy link
Contributor

Yes, please do, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants