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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[馃悶] Cookie not set when using $server #5951

Open
ziimakc opened this issue Mar 5, 2024 · 7 comments
Open

[馃悶] Cookie not set when using $server #5951

ziimakc opened this issue Mar 5, 2024 · 7 comments
Assignees
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working

Comments

@ziimakc
Copy link

ziimakc commented Mar 5, 2024

Describe the bug

Cookie is not set when using server$ with useTask$

Note: same example with useVisibleTask instead useTask works fine

Reproduction

https://github.com/ziimakc/qwik-cookie-not-set

const noCookies = server$(async function (
	this: RequestEventBase<QwikCityPlatform>,
): Promise<{}> {
	this.cookie.set("x", "x", {
		path: "/",
	});
	return {};
});

export default component$(() => {
	useTask$(async () => {
		const res = await noCookies(); // cookies not set in browser
	});

	return ( <h1>Hi 馃憢</h1> );
});

Steps to reproduce

  • Run npm dev
  • Open browser
  • Check cookies
@ziimakc ziimakc added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Mar 5, 2024
@PatrickJS
Copy link
Member

the problem is with setting headers and streaming. we need to set the headers right away which is why this is fine with useVisibleTask$ since the response and headers are set at the same time

@PatrickJS
Copy link
Member

ok I think I got a fix or at least something that will mostly work. so long as the cookie is set before the first write to stream

@PatrickJS
Copy link
Member

so the correct solution are plugin@<name>.ts https://qwik.dev/docs/advanced/plugins/

@PatrickJS
Copy link
Member

please use plugin@<name>.ts to set request cookies. if you have examples and a repro repo oh what you're trying to do then I can show you how to do it with plugin@<name>.ts/middleware

@PatrickJS PatrickJS closed this as not planned Won't fix, can't repro, duplicate, stale May 15, 2024
@ziimakc
Copy link
Author

ziimakc commented May 15, 2024

@PatrickJS and how would I set cookie in plugin that executes before $server, how do I know which server method was called?

If $server this.cookie.set is not supported, then should't it be removed to avoid confusion?

Example repo is provided and I wan't to do simple thing - set cookie when this method is called.

@PatrickJS PatrickJS reopened this May 15, 2024
@PatrickJS
Copy link
Member

here is a summary of my response on discord
Summary of Why server$ Can't Set Cookies in Certain Cases:

  1. Issue with Initial Load:

    • When using server$ to set cookies during the initial server-side render, it doesn't work because the response is streamed immediately.
    • HTTP requires headers (including cookies) to be set before the response is sent, but server$ during SSR streams the response right away, preventing cookies from being set properly.
  2. Client-Side Requests:

    • When server$ is called from the client, it works correctly because the response isn't streamed yet, allowing cookies to be set before sending the response.
  3. Use of Plugins:

    • To handle cookies (especially for authentication) on initial load, it's recommended to use Qwik plugins with an onRequest handler.
    • The plugin middleware can intercept requests and set cookies before the response is sent, ensuring proper handling.
  4. Alternative Approach:

    • If you need to set cookies after the initial load, consider using useVisibleTask$ instead of useTask$ to ensure the server$ function runs only on client requests, thus allowing cookies to be set correctly. (avoiding initial render issue)
  5. Specific Use Case:

    • For scenarios like updating a checkout session each time the cart content changes, a combination of plugin middleware (for initial load) and server$ (for client-side updates) may be needed.
  6. Documentation:

    • The documentation for Qwik plugins and handling such cases needs improvement, which is acknowledged in the conversation.

@ziimakc
Copy link
Author

ziimakc commented May 16, 2024

@PatrickJS from my point of view $server looks the same as $routeLoader and I can see it probably will be added also to $useResource #5323

In all of this cases we have requestEvent of the same type, but only for $server and probably $useResource it will not work on some specific cases when setting cookie on initial render.

Changing types or creating eslint rules seems not to be possible as it depends on where this method is used, so I agree that it seems just documenting this for now is the best option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants