chore: Slight improvements before merge

sveltejs · Jan 29, 2025 · 24e4438 · 24e4438
1 parent 4f7c4b9
commit 24e4438
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 46 deletions.
diff --git a/packages/kit/src/runtime/server/validate-headers.js b/packages/kit/src/runtime/server/validate-headers.js
@@ -23,24 +23,26 @@ const CONTENT_TYPE_PATTERN =
 /** @type {Record<string, (value: string) => void>} */
 const HEADER_VALIDATORS = {
 	'cache-control': (value) => {
-		const parts = value.split(',');
-		if (parts.some((part) => !part.trim())) {
-			throw new Error('cache-control header contains empty directives');
+		const error_suffix = `(While parsing "${value}".)`;
+		const parts = value.split(',').map((part) => part.trim());
+		if (parts.some((part) => !part)) {
+			throw new Error(`\`cache-control\` header contains empty directives. ${error_suffix}`);
 		}
 
-		const directives = parts.map((part) => part.trim().split('=')[0].toLowerCase());
+		const directives = parts.map((part) => part.split('=')[0].toLowerCase());
 		const invalid = directives.find((directive) => !VALID_CACHE_CONTROL_DIRECTIVES.has(directive));
 		if (invalid) {
 			throw new Error(
-				`Invalid cache-control directive "${invalid}". Did you mean one of: ${[...VALID_CACHE_CONTROL_DIRECTIVES].join(', ')}`
+				`Invalid cache-control directive "${invalid}". Did you mean one of: ${[...VALID_CACHE_CONTROL_DIRECTIVES].join(', ')}? ${error_suffix}`
 			);
 		}
 	},
 
 	'content-type': (value) => {
 		const type = value.split(';')[0].trim();
+		const error_suffix = `(While parsing "${value}".)`;
 		if (!CONTENT_TYPE_PATTERN.test(type)) {
-			throw new Error(`Invalid content-type value "${type}"`);
+			throw new Error(`Invalid content-type value "${type}". ${error_suffix}`);
 		}
 	}
 };

diff --git a/packages/kit/src/runtime/server/validate-headers.spec.js b/packages/kit/src/runtime/server/validate-headers.spec.js
@@ -1,109 +1,91 @@
-import { describe, test, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
+import { describe, test, expect, beforeEach, vi } from 'vitest';
+import {validateHeaders} from './validate-headers.js';
 
 describe('validateHeaders', () => {
-	/** @type {typeof console.warn} */
-	let console_warn;
+	const console_warn_spy = vi.spyOn(console, 'warn');
 
-	/** @type {(headers: Record<string, string>) => void} */
-	let validateHeaders;
-
-	beforeAll(() => {
-		console_warn = console.warn;
-		// @ts-expect-error
-		globalThis.__SVELTEKIT_DEV__ = false;
-	});
-
-	afterAll(() => {
-		console.warn = console_warn;
-		vi.unstubAllGlobals();
-	});
-
-	beforeEach(async () => {
-		console.warn = vi.fn();
-		// @ts-expect-error
-		globalThis.__SVELTEKIT_DEV__ = true;
-		const module = await import('./validate-headers.js');
-		validateHeaders = module.validateHeaders;
+	beforeEach(() => {
+		vi.resetAllMocks();
 	});
 
 	describe('cache-control header', () => {
 		test('accepts valid directives', () => {
 			validateHeaders({ 'cache-control': 'public, max-age=3600' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 
 		test('rejects invalid directives', () => {
 			validateHeaders({ 'cache-control': 'public, maxage=3600' });
-			expect(console.warn).toHaveBeenCalledWith(
+			expect(console_warn_spy).toHaveBeenCalledWith(
 				expect.stringContaining('Invalid cache-control directive "maxage"')
 			);
 		});
 
 		test('rejects empty directives', () => {
 			validateHeaders({ 'cache-control': 'public,, max-age=3600' });
-			expect(console.warn).toHaveBeenCalledWith(
-				expect.stringContaining('cache-control header contains empty directives')
+			expect(console_warn_spy).toHaveBeenCalledWith(
+				expect.stringContaining('`cache-control` header contains empty directives')
 			);
 
 			validateHeaders({ 'cache-control': 'public, , max-age=3600' });
-			expect(console.warn).toHaveBeenCalledWith(
-				expect.stringContaining('cache-control header contains empty directives')
+			expect(console_warn_spy).toHaveBeenCalledWith(
+				expect.stringContaining('`cache-control` header contains empty directives')
 			);
 		});
 
 		test('accepts multiple cache-control values', () => {
 			validateHeaders({ 'cache-control': 'max-age=3600, s-maxage=7200' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 	});
 
 	describe('content-type header', () => {
 		test('accepts standard content types', () => {
 			validateHeaders({ 'content-type': 'application/json' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 
 		test('accepts content types with parameters', () => {
 			validateHeaders({ 'content-type': 'text/html; charset=utf-8' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 
 			validateHeaders({ 'content-type': 'application/javascript; charset=utf-8' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 
 		test('accepts vendor-specific content types', () => {
 			validateHeaders({ 'content-type': 'x-custom/whatever' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 
 		test('rejects malformed content types', () => {
 			validateHeaders({ 'content-type': 'invalid-content-type' });
-			expect(console.warn).toHaveBeenCalledWith(
+			expect(console_warn_spy).toHaveBeenCalledWith(
 				expect.stringContaining('Invalid content-type value "invalid-content-type"')
 			);
 		});
 
 		test('rejects invalid content type categories', () => {
 			validateHeaders({ 'content-type': 'invalid/type; invalid=param' });
-			expect(console.warn).toHaveBeenCalledWith(
+			expect(console_warn_spy).toHaveBeenCalledWith(
 				expect.stringContaining('Invalid content-type value "invalid/type"')
 			);
 
 			validateHeaders({ 'content-type': 'bad/type; charset=utf-8' });
-			expect(console.warn).toHaveBeenCalledWith(
+			expect(console_warn_spy).toHaveBeenCalledWith(
 				expect.stringContaining('Invalid content-type value "bad/type"')
 			);
 		});
 
 		test('handles case-insensitive content-types', () => {
 			validateHeaders({ 'content-type': 'TEXT/HTML; charset=utf-8' });
-			expect(console.warn).not.toHaveBeenCalled();
+			expect(console_warn_spy).not.toHaveBeenCalled();
 		});
 	});
 
 	test('allows unknown headers', () => {
 		validateHeaders({ 'x-custom-header': 'some-value' });
-		expect(console.warn).not.toHaveBeenCalled();
+		expect(console_warn_spy).not.toHaveBeenCalled();
 	});
 
 	test('handles multiple headers simultaneously', () => {
@@ -112,6 +94,6 @@ describe('validateHeaders', () => {
 			'content-type': 'text/html',
 			'x-custom': 'value'
 		});
-		expect(console.warn).not.toHaveBeenCalled();
+		expect(console_warn_spy).not.toHaveBeenCalled();
 	});
 });