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

Expose add_cookies_to_headers via cookies.write #11712

Open
pzuraq opened this issue Jan 22, 2024 · 0 comments
Open

Expose add_cookies_to_headers via cookies.write #11712

pzuraq opened this issue Jan 22, 2024 · 0 comments
Labels
feature / enhancement New feature or request

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Jan 22, 2024

Describe the problem

This has been discussed before in #8409 and #7611, but I think we have a bit of a different use case here.

We're working on a cookie manager library that layers on some additional niceties to the cookies API in SvelteKit (e.g. defining and validating the shape of cookies, handling serializing to/from JSON and base64, ability to manage and write cookies both server side and client side, etc). The manager allows libraries to define their own cookies and then export them, so you can have shared cookies between different libraries more easily:

export const cookieManager = new SvelteKitCookieManager({
  deviceId: {
    key: 'device_id',
    validator: (z) => z.string(),
  }
});

// Usage
cookieManager.getWriter('server', event).get('deviceId');
cookieManager.getWriter('server', event).set('deviceId', 'foo');

This is all typed and really nice to work with. The tricky part is that we can't really use it in both hooks and +page.server.ts contexts, because in hooks we can't use the cookies API, but in +page.server.ts we must use the cookies API, there's no way for us to add set-cookies headers otherwise. Currently it seems like the only option is to have a new type of writer or some other option for changing the behavior in hooks vs pages.

cookieManager.getWriter('server', event.cookies).get('deviceId');
cookieManager.getWriter('server-hooks', event.headers.get('cookie')).set('deviceId', 'foo');

Describe the proposed solution

I feel like this could be remedied by exposing the add_cookies_to_headers function on the cookies API. This would only be callable once per request, it would throw if called a second time. The purpose would be so that hooks that return custom responses can optionally choose to write out the added cookies manually at some point.

Alternatives considered

We could allow users to add the set-cookie header via the headers API. Our library currently outputs the set-cookie value as a string, so it would be easy to add it that way, but it does seem a bit hacky.

We could also always serialize cookies to all responses, but this would be tricky. I think there are a couple ways to approach this:

  1. Serialize cookies at the end of hooks. As noted in cookies are not being set when returning custom Response objects in handle #7611 this would result in a gap between cookies being added and hooks resolving, so hooks would no longer see any added cookies. This would also be a breaking API change so probably not worth it.
  2. Add cookies at the end of handle IFF they have not been added by resolve (e.g. resolve has not been called). Throw an error otherwise. This would work well, but would mean that users can't call resolve, ignore the response, and then set cookies, which would be a weird edge case.
  3. Add cookies twice, once at the end of resolve and once at the end of handle. Set cookies would be cleared in between so they don't set twice, just new ones would be added the second time. This would allow the API to be used in all cases and allow resolve cookies to be ignored by hooks, but would be more complex and require some additional bookkeeping to flush the cookies between sets.

Importance

nice to have

Additional Information

No response

@eltigerchino eltigerchino added the feature / enhancement New feature or request label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants