[chore] separate Headers type for Request and Response (#2248)

sveltejs · Aug 25, 2021 · 02f7aba · 02f7aba
1 parent 2144afd
commit 02f7aba
Show file tree

Hide file tree

Showing 16 changed files with 63 additions and 26 deletions.
diff --git a/.changeset/empty-donuts-smell.md b/.changeset/empty-donuts-smell.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[chore] separate RequestHeaders and ResponseHeaders types
diff --git a/packages/kit/src/core/adapt/prerender.js b/packages/kit/src/core/adapt/prerender.js
@@ -4,6 +4,7 @@ import { pathToFileURL, resolve, URL } from 'url';
 import { mkdirp } from '../../utils/filesystem.js';
 import { __fetch_polyfill } from '../../install-fetch.js';
 import { SVELTE_KIT } from '../constants.js';
+import { get_single_valued_header } from '../../utils/http.js';
 
 /**
  * @typedef {import('types/config').PrerenderErrorHandler} PrerenderErrorHandler
@@ -182,10 +183,14 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
 			mkdirp(dirname(file));
 
 			if (response_type === REDIRECT) {
-				const { location } = headers;
+				const location = get_single_valued_header(headers, 'location');
 
-				log.warn(`${rendered.status} ${path} -> ${location}`);
-				writeFileSync(file, `<meta http-equiv="refresh" content="0;url=${encodeURI(location)}">`);
+				if (location) {
+					log.warn(`${rendered.status} ${path} -> ${location}`);
+					writeFileSync(file, `<meta http-equiv="refresh" content="0;url=${encodeURI(location)}">`);
+				} else {
+					log.warn(`location header missing on redirect received from ${path}`);
+				}
 
 				return;
 			}

diff --git a/packages/kit/src/core/dev/index.js b/packages/kit/src/core/dev/index.js
@@ -367,7 +367,7 @@ async function create_handler(vite, config, dir, cwd, get_manifest) {
 
 				const rendered = await respond(
 					{
-						headers: /** @type {import('types/helper').Headers} */ (req.headers),
+						headers: /** @type {import('types/helper').RequestHeaders} */ (req.headers),
 						method: req.method,
 						host,
 						path: parsed.pathname.replace(config.kit.paths.base, ''),

diff --git a/packages/kit/src/core/preview/index.js b/packages/kit/src/core/preview/index.js
@@ -91,7 +91,7 @@ export async function preview({
 					host: /** @type {string} */ (config.kit.host ||
 						req.headers[config.kit.hostHeader || 'host']),
 					method: req.method,
-					headers: /** @type {import('types/helper').Headers} */ (req.headers),
+					headers: /** @type {import('types/helper').RequestHeaders} */ (req.headers),
 					path: parsed.pathname.replace(config.kit.paths.base, ''),
 					query: parsed.searchParams,
 					rawBody: body

diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js
@@ -1,3 +1,4 @@
+import { get_single_valued_header } from '../../utils/http.js';
 import { lowercase_keys } from './utils.js';
 
 /** @param {string} body */
@@ -62,7 +63,7 @@ export async function render_endpoint(request, route, match) {
 	let { status = 200, body, headers = {} } = response;
 
 	headers = lowercase_keys(headers);
-	const type = headers['content-type'];
+	const type = get_single_valued_header(headers, 'content-type');
 
 	const is_type_textual = is_content_type_textual(type);
 

diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
@@ -6,6 +6,7 @@ import { parse_body } from './parse_body/index.js';
 import { lowercase_keys } from './utils.js';
 import { coalesce_to_error } from '../utils.js';
 import { hash } from '../hash.js';
+import { get_single_valued_header } from '../../utils/http.js';
 
 /** @type {import('@sveltejs/kit/ssr').Respond} */
 export async function respond(incoming, options, state = {}) {
@@ -66,7 +67,8 @@ export async function respond(incoming, options, state = {}) {
 					if (response) {
 						// inject ETags for 200 responses
 						if (response.status === 200) {
-							if (!/(no-store|immutable)/.test(response.headers['cache-control'])) {
+							const cache_control = get_single_valued_header(response.headers, 'cache-control');
+							if (!cache_control || !/(no-store|immutable)/.test(cache_control)) {
 								const etag = `"${hash(response.body || '')}"`;
 
 								if (request.headers['if-none-match'] === etag) {

diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js
@@ -120,7 +120,9 @@ export async function load_node({
 				} else if (resolved.startsWith('/') && !resolved.startsWith('//')) {
 					const relative = resolved;
 
-					const headers = /** @type {import('types/helper').Headers} */ ({ ...opts.headers });
+					const headers = /** @type {import('types/helper').RequestHeaders} */ ({
+						...opts.headers
+					});
 
 					// TODO: fix type https://github.com/node-fetch/node-fetch/issues/1113
 					if (opts.credentials !== 'omit') {
@@ -164,9 +166,12 @@ export async function load_node({
 							state.prerender.dependencies.set(relative, rendered);
 						}
 
+						// Set-Cookie not available to be set in `fetch` and that's the only header value that
+						// can be an array so we know we have only simple values
+						// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
 						response = new Response(rendered.body, {
 							status: rendered.status,
-							headers: rendered.headers
+							headers: /** @type {Record<string, string>} */ (rendered.headers)
 						});
 					}
 				} else {
@@ -211,7 +216,7 @@ export async function load_node({
 							async function text() {
 								const body = await response.text();
 
-								/** @type {import('types/helper').Headers} */
+								/** @type {import('types/helper').ResponseHeaders} */
 								const headers = {};
 								for (const [key, value] of response.headers) {
 									if (key !== 'etag' && key !== 'set-cookie') headers[key] = value;

diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js
@@ -175,7 +175,7 @@ export async function render_response({
 				.join('\n\n\t\t\t')}
 		`.replace(/^\t{2}/gm, '');
 
-	/** @type {import('types/helper').Headers} */
+	/** @type {import('types/helper').ResponseHeaders} */
 	const headers = {
 		'content-type': 'text/html'
 	};

diff --git a/packages/kit/src/runtime/server/parse_body/index.js b/packages/kit/src/runtime/server/parse_body/index.js
@@ -2,7 +2,7 @@ import { read_only_form_data } from './read_only_form_data.js';
 
 /**
  * @param {import('types/app').RawBody} raw
- * @param {import('types/helper').Headers} headers
+ * @param {import('types/helper').RequestHeaders} headers
  */
 export function parse_body(raw, headers) {
 	if (!raw) return raw;

diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js
@@ -1,6 +1,6 @@
-/** @param {Record<string, string>} obj */
+/** @param {Record<string, any>} obj */
 export function lowercase_keys(obj) {
-	/** @type {Record<string, string>} */
+	/** @type {Record<string, any>} */
 	const clone = {};
 
 	for (const key in obj) {

diff --git a/packages/kit/src/utils/http.js b/packages/kit/src/utils/http.js
@@ -0,0 +1,20 @@
+/**
+ * @param {Record<string, string | string[]>} headers
+ * @param {string} key
+ * @returns {string | undefined}
+ */
+export function get_single_valued_header(headers, key) {
+	const value = headers[key];
+	if (Array.isArray(value)) {
+		if (value.length === 0) {
+			return undefined;
+		}
+		if (value.length > 1) {
+			throw new Error(
+				`Multiple headers provided for ${key}. Multiple may be provided only for set-cookie`
+			);
+		}
+		return value[0];
+	}
+	return value;
+}
diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js
@@ -23,7 +23,7 @@ export const handle = sequence(
 			...response,
 			headers: {
 				...response.headers,
-				'Set-Cookie': 'name=SvelteKit; path=/; HttpOnly'
+				'set-cookie': 'name=SvelteKit; path=/; HttpOnly'
 			}
 		};
 	}

diff --git a/packages/kit/types/app.d.ts b/packages/kit/types/app.d.ts
@@ -1,4 +1,4 @@
-import { Headers, ReadOnlyFormData } from './helper';
+import { ReadOnlyFormData, RequestHeaders } from './helper';
 import { ServerResponse } from './hooks';
 
 export interface App {
@@ -32,6 +32,6 @@ export interface IncomingRequest {
 	host: string;
 	path: string;
 	query: URLSearchParams;
-	headers: Headers;
+	headers: RequestHeaders;
 	rawBody: RawBody;
 }
diff --git a/packages/kit/types/endpoint.d.ts b/packages/kit/types/endpoint.d.ts
@@ -1,5 +1,5 @@
 import { ServerRequest } from './hooks';
-import { Headers, MaybePromise } from './helper';
+import { MaybePromise, ResponseHeaders } from './helper';
 
 type ToJSON = { toJSON(...args: any[]): JSONValue };
 type JSONValue = Exclude<JSONResponse, ToJSON>;
@@ -16,7 +16,7 @@ type DefaultBody = JSONResponse | Uint8Array;
 
 export interface EndpointOutput<Body extends DefaultBody = DefaultBody> {
 	status?: number;
-	headers?: Headers;
+	headers?: ResponseHeaders;
 	body?: Body;
 }
 

diff --git a/packages/kit/types/helper.d.ts b/packages/kit/types/helper.d.ts
@@ -8,11 +8,10 @@ interface ReadOnlyFormData {
 	[Symbol.iterator](): Generator<[string, string], void>;
 }
 
-// TODO we want to differentiate between request headers, which
-// always follow this type, and response headers, in which
-// 'set-cookie' is a `string[]` (or at least `string | string[]`)
-// but this can't happen until TypeScript 4.3
-export type Headers = Record<string, string>;
+export type RequestHeaders = Record<string, string>;
+
+/** Only value that can be an array is set-cookie. For everything else we assume string value */
+export type ResponseHeaders = Record<string, string | string[]>;
 
 // Utility Types
 export type InferValue<T, Key extends keyof T, Default> = T extends Record<Key, infer Val>

diff --git a/packages/kit/types/hooks.d.ts b/packages/kit/types/hooks.d.ts
@@ -1,5 +1,5 @@
 import { IncomingRequest, ParameterizedBody } from './app';
-import { Headers, MaybePromise } from './helper';
+import { MaybePromise, ResponseHeaders } from './helper';
 
 export type StrictBody = string | Uint8Array;
 
@@ -12,7 +12,7 @@ export interface ServerRequest<Locals = Record<string, any>, Body = unknown>
 
 export interface ServerResponse {
 	status: number;
-	headers: Headers;
+	headers: ResponseHeaders;
 	body?: StrictBody;
 }