Repository: sveltejs/kit
Pull request #6818 (2022-09-15 00:02:18)

fix missing `opts` type for `cookies` api
Currently, the `cookies` api is missing the forwarded types `CookieSerializeOptions` and `CookieParseOptions` from the `@types/cookie` package, so we don't get any typescript autocompletion.

Example:
In a new SvelteKit project, the argument `opts` is missing its type:
<img width="520" alt="CleanShot 2022-09-15 at 07 57 48@2x" src="https://user-images.githubusercontent.com/54401897/190282978-728a6d4c-8d5d-4673-9583-53dc07a58ab0.png">

However, after manually installing `@types/cookie` in a new project, the type appears:
<img width="506" alt="CleanShot 2022-09-15 at 07 59 10@2x" src="https://user-images.githubusercontent.com/54401897/190283094-848d6848-50ca-41c9-9345-74709dd69575.png">

~I was hoping that moving the types package to a dependency will ensure it gets installed automatically, but I have a feeling this might be the wrong way to fix the issue...~
Added it to peerDependencies as per @mquandalle 's advice.

### Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it solves.
- [ ] Ideally, include a test that fails without this PR but passes with it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check`

### Changesets
- [ ] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. All changesets should be `patch` until SvelteKit 1.0


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: f8ebcf6889a4ec99662c55343c0195db8d813673

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/s3812497/kit/new/missing-cookie-type?filename=.changeset/silent-flies-approve.md&value=---%0A%22%40sveltejs%2Fkit%22%3A%20patch%0A---%0A%0Afix%20missing%20opt%20type%20for%20cookies%20api%0A)


dummdidumm: Feels strange to me to have `cookies` in dependencies but the types for it in peerDependencies. I think moving both to dependencies makes more sense.

Patch:
diff --git a/.changeset/afraid-kings-mix.md b/.changeset/afraid-kings-mix.md
new file mode 100644
index 000000000..57d8d5566
--- /dev/null
+++ b/.changeset/afraid-kings-mix.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] forward cookies from fetch on redirect response
diff --git a/.changeset/popular-plums-beam.md b/.changeset/popular-plums-beam.md
new file mode 100644
index 000000000..28c7f6f92
--- /dev/null
+++ b/.changeset/popular-plums-beam.md
@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+adding missing `@types/cookie` dependency
diff --git a/packages/kit/package.json b/packages/kit/package.json
index 938f4e9a2..7e217159d 100644
--- a/packages/kit/package.json
+++ b/packages/kit/package.json
@@ -11,6 +11,7 @@
 	"type": "module",
 	"dependencies": {
 		"@sveltejs/vite-plugin-svelte": "^1.0.5",
+		"@types/cookie": "^0.5.1",
 		"cookie": "^0.5.0",
 		"devalue": "^3.1.2",
 		"kleur": "^4.1.4",
@@ -26,7 +27,6 @@
 	"devDependencies": {
 		"@playwright/test": "^1.25.0",
 		"@types/connect": "^3.4.35",
-		"@types/cookie": "^0.5.1",
 		"@types/marked": "^4.0.3",
 		"@types/mime": "^3.0.0",
 		"@types/node": "^16.11.36",
diff --git a/packages/kit/src/runtime/server/cookie.js b/packages/kit/src/runtime/server/cookie.js
index 6192542ef..559dcce3e 100644
--- a/packages/kit/src/runtime/server/cookie.js
+++ b/packages/kit/src/runtime/server/cookie.js
@@ -1,4 +1,4 @@
-import { parse } from 'cookie';
+import { parse, serialize } from 'cookie';
 
 /** @type {import('cookie').CookieSerializeOptions} */
 const DEFAULT_SERIALIZE_OPTIONS = {
@@ -12,7 +12,7 @@ const DEFAULT_SERIALIZE_OPTIONS = {
  * @param {URL} url
  */
 export function get_cookies(request, url) {
-	/** @type {Map<string, {name: string; value: string; options: import('cookie').CookieSerializeOptions;}>} */
+	/** @type {Map<string, import('./page/types').Cookie>} */
 	const new_cookies = new Map();
 
 	/** @type {import('types').Cookies} */
@@ -102,3 +102,14 @@ export function path_matches(path, constraint) {
 	if (path === normalized) return true;
 	return path.startsWith(normalized + '/');
 }
+
+/**
+ * @param {Headers} headers
+ * @param {import('./page/types').Cookie[]} cookies
+ */
+export function add_cookies_to_headers(headers, cookies) {
+	for (const new_cookie of cookies) {
+		const { name, value, options } = new_cookie;
+		headers.append('set-cookie', serialize(name, value, options));
+	}
+}
diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
index 751596c4c..cff90f3c0 100644
--- a/packages/kit/src/runtime/server/index.js
+++ b/packages/kit/src/runtime/server/index.js
@@ -1,4 +1,3 @@
-import * as cookie from 'cookie';
 import { render_endpoint } from './endpoint.js';
 import { render_page } from './page/index.js';
 import { render_response } from './page/render.js';
@@ -9,7 +8,7 @@ import { decode_params, disable_search, normalize_path } from '../../utils/url.j
 import { exec } from '../../utils/routing.js';
 import { render_data } from './data/index.js';
 import { DATA_SUFFIX } from '../../constants.js';
-import { get_cookies } from './cookie.js';
+import { add_cookies_to_headers, get_cookies } from './cookie.js';
 import { HttpError } from '../control.js';
 
 /* global __SVELTEKIT_ADAPTER_NAME__ */
@@ -246,12 +245,7 @@ export async function respond(request, options, state) {
 					}
 				}
 
-				for (const new_cookie of Array.from(new_cookies.values())) {
-					response.headers.append(
-						'set-cookie',
-						cookie.serialize(new_cookie.name, new_cookie.value, new_cookie.options)
-					);
-				}
+				add_cookies_to_headers(response.headers, Array.from(new_cookies.values()));
 
 				return response;
 			}
diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js
index 87c034da9..ec728254a 100644
--- a/packages/kit/src/runtime/server/page/fetch.js
+++ b/packages/kit/src/runtime/server/page/fetch.js
@@ -19,7 +19,7 @@ export function create_fetch({ event, options, state, route, prerender_default,
 
 	const initial_cookies = cookie.parse(event.request.headers.get('cookie') || '');
 
-	/** @type {import('set-cookie-parser').Cookie[]} */
+	/** @type {import('./types').Cookie[]} */
 	const set_cookies = [];
 
 	/**
@@ -31,8 +31,8 @@ export function create_fetch({ event, options, state, route, prerender_default,
 		const new_cookies = {};
 
 		for (const cookie of set_cookies) {
-			if (!domain_matches(url.hostname, cookie.domain)) continue;
-			if (!path_matches(url.pathname, cookie.path)) continue;
+			if (!domain_matches(url.hostname, cookie.options.domain)) continue;
+			if (!path_matches(url.pathname, cookie.options.path)) continue;
 
 			new_cookies[cookie.name] = cookie.value;
 		}
@@ -179,9 +179,11 @@ export function create_fetch({ event, options, state, route, prerender_default,
 		const set_cookie = response.headers.get('set-cookie');
 		if (set_cookie) {
 			set_cookies.push(
-				...set_cookie_parser
-					.splitCookiesString(set_cookie)
-					.map((str) => set_cookie_parser.parseString(str))
+				...set_cookie_parser.splitCookiesString(set_cookie).map((str) => {
+					const { name, value, ...options } = set_cookie_parser.parseString(str);
+					// options.sameSite is string, something more specific is required - type cast is safe
+					return /** @type{import('./types').Cookie} */ ({ name, value, options });
+				})
 			);
 		}
 
diff --git a/packages/kit/src/runtime/server/page/index.js b/packages/kit/src/runtime/server/page/index.js
index 64e617ee0..c0682193d 100644
--- a/packages/kit/src/runtime/server/page/index.js
+++ b/packages/kit/src/runtime/server/page/index.js
@@ -217,7 +217,7 @@ export async function render_page(event, route, page, options, state, resolve_op
 							});
 						}
 
-						return redirect_response(err.status, err.location);
+						return redirect_response(err.status, err.location, cookies);
 					}
 
 					const status = err instanceof HttpError ? err.status : 500;
diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
index 5480db4ee..308d02e0c 100644
--- a/packages/kit/src/runtime/server/page/render.js
+++ b/packages/kit/src/runtime/server/page/render.js
@@ -1,10 +1,10 @@
 import { devalue } from 'devalue';
 import { readable, writable } from 'svelte/store';
-import * as cookie from 'cookie';
 import { hash } from '../../hash.js';
 import { serialize_data } from './serialize_data.js';
 import { s } from '../../../utils/misc.js';
 import { Csp } from './csp.js';
+import { add_cookies_to_headers } from '../cookie.js';
 
 // TODO rename this function/module
 
@@ -18,7 +18,7 @@ const updated = {
  * @param {{
  *   branch: Array<import('./types').Loaded>;
  *   fetched: Array<import('./types').Fetched>;
- *   cookies: import('set-cookie-parser').Cookie[];
+ *   cookies: import('./types').Cookie[];
  *   options: import('types').SSROptions;
  *   state: import('types').SSRState;
  *   page_config: { ssr: boolean; csr: boolean };
@@ -328,11 +328,7 @@ export async function render_response({
 			headers.set('content-security-policy-report-only', report_only_header);
 		}
 
-		for (const new_cookie of cookies) {
-			const { name, value, ...options } = new_cookie;
-			// @ts-expect-error
-			headers.append('set-cookie', cookie.serialize(name, value, options));
-		}
+		add_cookies_to_headers(headers, cookies);
 
 		if (link_header_preloads.size) {
 			headers.set('link', Array.from(link_header_preloads).join(', '));
diff --git a/packages/kit/src/runtime/server/page/types.d.ts b/packages/kit/src/runtime/server/page/types.d.ts
index 7416f82ae..0e350772f 100644
--- a/packages/kit/src/runtime/server/page/types.d.ts
+++ b/packages/kit/src/runtime/server/page/types.d.ts
@@ -1,5 +1,5 @@
+import { CookieSerializeOptions } from 'cookie';
 import { SSRNode, CspDirectives } from 'types';
-import { HttpError } from '../../control.js';
 
 export interface Fetched {
 	url: string;
@@ -33,3 +33,9 @@ export interface CspOpts {
 	dev: boolean;
 	prerender: boolean;
 }
+
+export interface Cookie {
+	name: string;
+	value: string;
+	options: CookieSerializeOptions;
+}
diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js
index 025823262..333491328 100644
--- a/packages/kit/src/runtime/server/utils.js
+++ b/packages/kit/src/runtime/server/utils.js
@@ -2,6 +2,7 @@ import { devalue } from 'devalue';
 import { DATA_SUFFIX } from '../../constants.js';
 import { negotiate } from '../../utils/http.js';
 import { HttpError } from '../control.js';
+import { add_cookies_to_headers } from './cookie.js';
 
 /** @param {any} body */
 export function is_pojo(body) {
@@ -169,10 +170,13 @@ export function handle_error_and_jsonify(event, options, error) {
 /**
  * @param {number} status
  * @param {string} location
+ * @param {import('./page/types.js').Cookie[]} [cookies]
  */
-export function redirect_response(status, location) {
-	return new Response(undefined, {
+export function redirect_response(status, location, cookies = []) {
+	const response = new Response(undefined, {
 		status,
 		headers: { location }
 	});
+	add_cookies_to_headers(response.headers, cookies);
+	return response;
 }
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 3a20afe85..e7bbfd58e 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -295,6 +295,7 @@ importers:
       uvu: ^0.5.3
       vite: ^3.1.1
     dependencies:
+      '@types/cookie': 0.5.1
       '@sveltejs/vite-plugin-svelte': 1.0.5_svelte@3.48.0+vite@3.1.1
       cookie: 0.5.0
       devalue: 3.1.2
@@ -310,7 +311,6 @@ importers:
     devDependencies:
       '@playwright/test': 1.25.0
       '@types/connect': 3.4.35
-      '@types/cookie': 0.5.1
       '@types/marked': 4.0.3
       '@types/mime': 3.0.0
       '@types/node': 16.11.42
@@ -1179,7 +1179,7 @@ packages:
 
   /@types/cookie/0.5.1:
     resolution: {integrity: sha512-COUnqfB2+ckwXXSFInsFdOAWQzCCx+a5hq2ruyj+Vjund94RJQd4LG2u9hnvJrTgunKAaax7ancBYlDrNYxA0g==}
-    dev: true
+    dev: false
 
   /@types/estree/0.0.39:
     resolution: {integrity: sha512-EYNwp3bU+98cpU4lAWYYL7Zz+2gryWH1qbdDTidVd6hkiR6weksdbMadyXKXNPEkQFhXM+hVO9ZygomHXp+AIw==}


Test patch:
diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/+page.svelte
index 27884f041..648d1ea62 100644
--- a/packages/kit/test/apps/basics/src/routes/shadowed/+page.svelte
+++ b/packages/kit/test/apps/basics/src/routes/shadowed/+page.svelte
@@ -1,6 +1,7 @@
 <a href="/shadowed/simple">simple</a>
 <a href="/shadowed/redirect-get">redirect-get</a>
 <a href="/shadowed/redirect-get-with-cookie">redirect-get-with-cookie</a>
+<a href="/shadowed/redirect-get-with-cookie-from-fetch">redirect-get-with-cookie-from-fetch</a>
 <a href="/shadowed/error-get">error-get</a>
 <a href="/shadowed/no-get">no-get</a>
 <a href="/shadowed/dynamic/foo">dynamic/foo</a>
diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.js
new file mode 100644
index 000000000..dfb3dfaa8
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.js
@@ -0,0 +1,7 @@
+import { redirect } from '@sveltejs/kit';
+
+/** @type {import('./$types').PageLoad} */
+export async function load({ fetch }) {
+	await fetch('/shadowed/redirect-get-with-cookie-from-fetch/endpoint');
+	throw redirect(302, '/shadowed/redirected');
+}
diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.svelte b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.svelte
new file mode 100644
index 000000000..3c3d6df7a
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/+page.svelte
@@ -0,0 +1,1 @@
+<h1>This should not be visible</h1>
diff --git a/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/endpoint/+server.js b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/endpoint/+server.js
new file mode 100644
index 000000000..242d015c4
--- /dev/null
+++ b/packages/kit/test/apps/basics/src/routes/shadowed/redirect-get-with-cookie-from-fetch/endpoint/+server.js
@@ -0,0 +1,8 @@
+/** @type {import('./$types').RequestHandler} */
+export function GET({ cookies }) {
+	cookies.set('shadow-redirect-fetch', 'happy', {
+		path: '/',
+		secure: false // safari
+	});
+	return new Response('ok');
+}
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
index f6278b07f..15c03f300 100644
--- a/packages/kit/test/apps/basics/test/test.js
+++ b/packages/kit/test/apps/basics/test/test.js
@@ -171,6 +171,23 @@ test.describe('Shadowed pages', () => {
 		);
 	});
 
+	test('Handles GET redirects with cookies from fetch response', async ({
+		page,
+		context,
+		clicknav
+	}) => {
+		await page.goto('/shadowed');
+		await clicknav('[href="/shadowed/redirect-get-with-cookie-from-fetch"]');
+		expect(await page.textContent('h1')).toBe('Redirection was successful');
+
+		const cookies = await context.cookies();
+		expect(cookies).toEqual(
+			expect.arrayContaining([
+				expect.objectContaining({ name: 'shadow-redirect-fetch', value: 'happy' })
+			])
+		);
+	});
+
 	test('Handles POST redirects', async ({ page }) => {
 		await page.goto('/shadowed');
 		await Promise.all([page.waitForNavigation(), page.click('#redirect-post')]);