From 02f7aba5ea88f8f423d626b4c430be4760bcc561 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 25 Aug 2021 04:57:11 -0700 Subject: [PATCH] [chore] separate Headers type for Request and Response (#2248) --- .changeset/empty-donuts-smell.md | 5 +++++ packages/kit/src/core/adapt/prerender.js | 11 +++++++--- packages/kit/src/core/dev/index.js | 2 +- packages/kit/src/core/preview/index.js | 2 +- packages/kit/src/runtime/server/endpoint.js | 3 ++- packages/kit/src/runtime/server/index.js | 4 +++- .../kit/src/runtime/server/page/load_node.js | 11 +++++++--- .../kit/src/runtime/server/page/render.js | 2 +- .../src/runtime/server/parse_body/index.js | 2 +- packages/kit/src/runtime/server/utils.js | 4 ++-- packages/kit/src/utils/http.js | 20 +++++++++++++++++++ packages/kit/test/apps/basics/src/hooks.js | 2 +- packages/kit/types/app.d.ts | 4 ++-- packages/kit/types/endpoint.d.ts | 4 ++-- packages/kit/types/helper.d.ts | 9 ++++----- packages/kit/types/hooks.d.ts | 4 ++-- 16 files changed, 63 insertions(+), 26 deletions(-) create mode 100644 .changeset/empty-donuts-smell.md create mode 100644 packages/kit/src/utils/http.js diff --git a/.changeset/empty-donuts-smell.md b/.changeset/empty-donuts-smell.md new file mode 100644 index 000000000000..ee3607010dc1 --- /dev/null +++ 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 index 86903461abfb..803f8cd910ce 100644 --- 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, ``); + if (location) { + log.warn(`${rendered.status} ${path} -> ${location}`); + writeFileSync(file, ``); + } 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 index 68dfc7037eac..9ca153a9f650 100644 --- 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 index 2ec1778a4865..a859ba209700 100644 --- 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 index f27f4ec4ca4a..9e4664eec30d 100644 --- 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 index 4bfe7c4e0f32..8d6d4be7050f 100644 --- 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 index e4b4bcc19486..41ac5f0007a7 100644 --- 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} */ (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 index 6a1f7a8a01bb..90e03659ecce 100644 --- 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 index 588a8a936127..0d8f31582242 100644 --- 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 index 1580db21c4c7..960642c817c8 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -1,6 +1,6 @@ -/** @param {Record} obj */ +/** @param {Record} obj */ export function lowercase_keys(obj) { - /** @type {Record} */ + /** @type {Record} */ const clone = {}; for (const key in obj) { diff --git a/packages/kit/src/utils/http.js b/packages/kit/src/utils/http.js new file mode 100644 index 000000000000..1e1199b851b6 --- /dev/null +++ b/packages/kit/src/utils/http.js @@ -0,0 +1,20 @@ +/** + * @param {Record} 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 index 54bccc164a74..319e992cb250 100644 --- 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 index 720c49b4b819..4446d227cf57 100644 --- 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 index a9bce3727f7d..f23df7735d97 100644 --- 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; @@ -16,7 +16,7 @@ type DefaultBody = JSONResponse | Uint8Array; export interface EndpointOutput { status?: number; - headers?: Headers; + headers?: ResponseHeaders; body?: Body; } diff --git a/packages/kit/types/helper.d.ts b/packages/kit/types/helper.d.ts index 1d7c2abe9980..d6a7fcaf0cd1 100644 --- 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; +export type RequestHeaders = Record; + +/** Only value that can be an array is set-cookie. For everything else we assume string value */ +export type ResponseHeaders = Record; // Utility Types export type InferValue = T extends Record diff --git a/packages/kit/types/hooks.d.ts b/packages/kit/types/hooks.d.ts index 6320df30a0fe..60d8a8263ad4 100644 --- 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, Body = unknown> export interface ServerResponse { status: number; - headers: Headers; + headers: ResponseHeaders; body?: StrictBody; }