Refactor login cookie with cookieOptions function

[ekzyis]

Description

I think it's better to remove the cookie when we left the login page instead of when we visit /signup.

However, if we navigate away by entering a URL, the useEffect cleanup will not run.

We could fix this with a global useEffect that listens for path changes and sets/unsets the cookie if we're on /login or not.

TODO:

• should we use path: /login? no, see Refactor login cookie with cookieOptions function #2003 (review)
• use global useEffect to always set/unset signin cookie if we're on /login or not? no, that would be too much

Video

2025-03-20.17-28-24.mp4

Checklist

Are your changes backwards compatible? Please answer below:

yes, especially because #1976 was not deployed yet

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

5. Tested that the cookie is removed when I leave /login.

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

[Soxasora]

I don't personally think that there's a real problem with having the signin cookie just lying around since we always update its state when actually needed and it will get consumed eventually when the anon decides to login/signup.

Although... I admit it's not that clean and I would want to remove it too on leave. My only fear with going with the global route listener is that we might be adding complexity to a cookie that has a fairly innocent purpose.

tldr
It would be okay, just not sure if it deserves being checked at every route change

About path: /login, we can't put it because now we have /email that uses the cookie to compose the correct message

[ekzyis]

My only fear with going with the global route listener is that we might be adding complexity to a cookie that has a fairly innocent purpose.

I agree, I think just returning a cleanup function in useEffect is simple and effective enough

[huumn]

I think because this does not work in corner cases, while the other solution does, we should probably stick to the other solution. I agree that this seems better in theory - but because it's unreliable, it isn't.

[ekzyis]

Oh, you are right, the other solution was more reliable. 0dc9ffd now also expires the cookie on /signup, but I kept the useEffect cleanup function.

[huumn]

The redirect to /email doesn't cause this cookie to clear? The fact that we'd have to test this might be a sign that this isn't the right thing to do. We're tying cookie state to the component lifecycle rather than "did we visit login or signup last?" ... this feels less predictable.

[ekzyis]

As discussed in chat, I removed the cleanup function now.

All this PR does now is to use cookieOptions instead of manual joining of cookie options.

I considered to also set expires and maxAge to undefined so we'd have a session cookie but I don't want to over-engineer this / mess with this any further without a good reason.