QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.83k stars 1.31k forks source link

[🐞] Set cookie is not working properly on Refresh #3194

Closed origranot closed 1 year ago

origranot commented 1 year ago

Which component is affected?

Qwik City (routing)

Describe the bug

I want to be able to set cookie on page refresh, I am using access and refresh token for my authentication. My goal is to check on page refresh if the user don't have access token (expired or deleted) and have a valid refresh token, if this is the case I want to generate a new tokens from backend and set them as cookies.

My cookies is not being set on page refresh, only when I click attribute or use a redirect on other endpoint.

For the example I created this endpoint on my layout.tsx: The test cookie is not set on page refresh, it's only being set after clicking on attribute.

export const onGet: RequestHandler = ({ cookie }) => {
  console.log('Request handler is working..');
  cookie.set('test', 123, {
    path: '/',
    httpOnly: true,
    sameSite: 'strict',
  });
};

Reproduction

https://stackblitz.com/edit/qwik-starter-ak72zh?file=src/routes/layout.tsx

Steps to reproduce

  1. Open the page, (remove the test cookie if preset).
  2. Refresh the page, check that the request handler is responding but the test cookie is not being set...
  3. Click the attribute and see the cookie is there again.

System Info

System:
    OS: macOS 11.6
    CPU: (8) arm64 Apple M1
    Memory: 823.20 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.1.2 - /usr/local/bin/npm
  Browsers:
    Chrome: 110.0.5481.177
    Safari: 14.1.2
  npmPackages:
    @builder.io/qwik: ^0.19.2 => 0.19.2 
    @builder.io/qwik-city: ^0.4.0 => 0.4.0 
    @builder.io/qwik-react: ^0.4.2 => 0.4.2 
    vite: ^4.0.3 => 4.0.3

Additional Information

No response

stackblitz[bot] commented 1 year ago

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

origranot commented 1 year ago

Also tested it with the new release (0.20.0), same results 😢

gabriiels commented 1 year ago

The same thing happens to me, the console.log reads and executes it, but it doesn't set the cookie without a request or an action.

ahnpnl commented 1 year ago

My guess is, onGet is executed in server while cookie is browser specific. To workaround this, you can set header set-cookie

image

I think the possible fix would adjust this method https://github.com/BuilderIO/qwik/blob/f4e06e58b6fd57adb3dacb45d12dac97432ac2cc/packages/qwik-city/middleware/request-handler/cookie.ts#L117 to set set-cookie in request header. However, I'm not sure, just my assumption

origranot commented 1 year ago

My guess is, onGet is executed in server while cookie is browser specific. To workaround this, you can set header set-cookie image

I think the possible fix would adjust this method

https://github.com/BuilderIO/qwik/blob/f4e06e58b6fd57adb3dacb45d12dac97432ac2cc/packages/qwik-city/middleware/request-handler/cookie.ts#L117

to set set-cookie in request header. However, I'm not sure, just my assumption

@ahnpnl At second look, it still has some weird issues. Some times it's not working well if I am trying to set multiple cookies.

iacore commented 1 year ago

Seems like cookie is ignored when onGet and components$ are used together. It doesn't work with routeLoader$ either.

Set this as any route (index.ts):

export const onGet = async (act) => {
  const { cookie, headers } = act
  // cookie.set("a", "c", { path: "/" }) // not working
  headers.set("Set-Cookie", "a=c") // working
}

export default component$(() => {
  return (
    <div>
...
origranot commented 1 year ago

Seems like cookie is ignored when onGet and components$ are used together. It doesn't work with routeLoader$ either.

Set this as any route (index.ts):

export const onGet = async (act) => {
  const { cookie, headers } = act
  // cookie.set("a", "c", { path: "/" }) // not working
  headers.set("Set-Cookie", "a=c") // working
}

export default component$(() => {
  return (
    <div>
...

It will work for the first cookie only. There is a bug on VITE dev server. if you will try to build and serve qwik you will see it will work with cookie.set

ref: https://github.com/BuilderIO/qwik/issues/3258