chore: pass lint/typechecks for kv-asset-handler

Followup from #6090, this enables typechecking and linting in kv-asset-handler, and fixes any failures.

.changeset/healthy-snails-heal.md

@@ -0,0 +1,7 @@
+---
+"@cloudflare/kv-asset-handler": patch
+---
+
+chore: pass lint/typechecks for kv-asset-handler
+
+Followup from https://github.com/cloudflare/workers-sdk/pull/6090, this enables typechecking and linting in kv-asset-handler, and fixes any failures.

packages/kv-asset-handler/package.json

@@ -9,7 +9,9 @@
 		"build": "tsc -d",
 		"pretest": "npm run build",
 		"test": "ava dist/test/*.js --verbose",
-		"test:ci": "npm run build && ava dist/test/*.js --verbose"
+		"test:ci": "npm run build && ava dist/test/*.js --verbose",
+		"check:lint": "eslint .",
+		"check:type": "tsc"
 	},
 	"repository": {
 		"type": "git",
@@ -43,6 +45,7 @@
 		"@cloudflare/workers-types": "^4.20240605.0",
 		"@types/mime": "^3.0.4",
 		"@types/node": "20.8.3",
+		"@types/service-worker-mock": "^2.0.1",
 		"ava": "^6.0.1",
 		"service-worker-mock": "^2.0.5"
 	},

packages/kv-asset-handler/src/index.ts

@@ -9,7 +9,8 @@ import {
 import type { AssetManifestType } from "./types";
 
 declare global {
-	const __STATIC_CONTENT: unknown, __STATIC_CONTENT_MANIFEST: string;
+	const __STATIC_CONTENT: KVNamespace | undefined,
+		__STATIC_CONTENT_MANIFEST: string;
 }
 
 const defaultCacheControl: CacheControl = {

packages/kv-asset-handler/src/mocks.ts

@@ -1,17 +1,19 @@
-const makeServiceWorkerEnv = require("service-worker-mock");
+import makeServiceWorkerEnv from "service-worker-mock";
 
 const HASH = "123HASHBROWN";
 
-export const getEvent = (request: Request): any => {
-	const waitUntil = async (callback: any) => {
-		await callback;
+export const getEvent = (
+	request: Request
+): Pick<FetchEvent, "request" | "waitUntil"> => {
+	const waitUntil = async (maybePromise: unknown) => {
+		await maybePromise;
 	};
 	return {
 		request,
 		waitUntil,
 	};
 };
-const store: any = {
+const store = {
 	"key1.123HASHBROWN.txt": "val1",
 	"key1.123HASHBROWN.png": "val1",
 	"index.123HASHBROWN.html": "index.html",
@@ -30,9 +32,9 @@ const store: any = {
 	"image.123HASHBROWN.webp": "imagewebp",
 	"你好/index.123HASHBROWN.html": "My path is non-ascii",
 };
-export const mockKV = (store: any) => {
+export const mockKV = (kvStore: Record<string, string>) => {
 	return {
-		get: (path: string) => store[path] || null,
+		get: (path: string) => kvStore[path] || null,
 	};
 };
 
@@ -57,22 +59,22 @@ export const mockManifest = () => {
 	});
 };
 
-let cacheStore: any = new Map();
+const cacheStore = new Map<string, Response>();
 interface CacheKey {
-	url: object;
-	headers: object;
+	url: string;
+	headers: HeadersInit;
 }
 export const mockCaches = () => {
 	return {
 		default: {
-			async match(key: any) {
-				let cacheKey: CacheKey = {
+			async match(key: Request) {
+				const cacheKey: CacheKey = {
 					url: key.url,
 					headers: {},
 				};
 				let response;
 				if (key.headers.has("if-none-match")) {
-					let makeStrongEtag = key.headers
+					const makeStrongEtag = key.headers
 						.get("if-none-match")
 						.replace("W/", "");
 					Reflect.set(cacheKey.headers, "etag", makeStrongEtag);
@@ -102,23 +104,25 @@ export const mockCaches = () => {
 					}
 					// ... which we are using in this repository to set status 206
 					if (response.headers.has("content-range")) {
+						// @ts-expect-error overridding status in this mock
 						response.status = 206;
 					} else {
+						// @ts-expect-error overridding status in this mock
 						response.status = 200;
 					}
-					let etag = response.headers.get("etag");
+					const etag = response.headers.get("etag");
 					if (etag && !etag.includes("W/")) {
 						response.headers.set("etag", `W/${etag}`);
 					}
 				}
 				return response;
 			},
-			async put(key: any, val: Response) {
-				let headers = new Headers(val.headers);
-				let url = new URL(key.url);
-				let resWithBody = new Response(val.body, { headers, status: 200 });
-				let resNoBody = new Response(null, { headers, status: 304 });
-				let cacheKey: CacheKey = {
+			async put(key: Request, val: Response) {
+				const headers = new Headers(val.headers);
+				const url = new URL(key.url);
+				const resWithBody = new Response(val.body, { headers, status: 200 });
+				const resNoBody = new Response(null, { headers, status: 304 });
+				const cacheKey: CacheKey = {
 					url: key.url,
 					headers: {
 						etag: `"${url.pathname.replace("/", "")}"`,
@@ -135,16 +139,16 @@ export const mockCaches = () => {
 
 // mocks functionality used inside worker request
 export function mockRequestScope() {
-	Object.assign(global, makeServiceWorkerEnv());
-	Object.assign(global, { __STATIC_CONTENT_MANIFEST: mockManifest() });
-	Object.assign(global, { __STATIC_CONTENT: mockKV(store) });
-	Object.assign(global, { caches: mockCaches() });
+	Object.assign(globalThis, makeServiceWorkerEnv());
+	Object.assign(globalThis, { __STATIC_CONTENT_MANIFEST: mockManifest() });
+	Object.assign(globalThis, { __STATIC_CONTENT: mockKV(store) });
+	Object.assign(globalThis, { caches: mockCaches() });
 }
 
 // mocks functionality used on global isolate scope. such as the KV namespace bind
 export function mockGlobalScope() {
-	Object.assign(global, { __STATIC_CONTENT_MANIFEST: mockManifest() });
-	Object.assign(global, { __STATIC_CONTENT: mockKV(store) });
+	Object.assign(globalThis, { __STATIC_CONTENT_MANIFEST: mockManifest() });
+	Object.assign(globalThis, { __STATIC_CONTENT: mockKV(store) });
 }
 
 export const sleep = (milliseconds: number) => {

packages/kv-asset-handler/src/test/getAssetFromKV-optional.ts

@@ -1,24 +1,18 @@
 import test from "ava";
-import {
-	getEvent,
-	mockGlobalScope,
-	mockKV,
-	mockManifest,
-	mockRequestScope,
-	sleep,
-} from "../mocks";
+import { getEvent, mockGlobalScope, mockRequestScope } from "../mocks";
 
 mockGlobalScope();
 
-const { getAssetFromKV, mapRequestToAsset } = require("../index");
-
+// @ts-expect-error we use a require for a mock
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const { getAssetFromKV } = require("../index");
 // manually reset manifest global, to test optional behaviour
-Object.assign(global, { __STATIC_CONTENT_MANIFEST: undefined });
+Object.assign(globalThis, { __STATIC_CONTENT_MANIFEST: undefined });
 
 test("getAssetFromKV return correct val from KV without manifest", async (t) => {
 	mockRequestScope();
 	// manually reset manifest global, to test optional behaviour
-	Object.assign(global, { __STATIC_CONTENT_MANIFEST: undefined });
+	Object.assign(globalThis, { __STATIC_CONTENT_MANIFEST: undefined });
 
 	const event = getEvent(new Request("https://blah.com/key1.123HASHBROWN.txt"));
 	const res = await getAssetFromKV(event);

packages/kv-asset-handler/src/test/getAssetFromKV.ts

@@ -7,12 +7,13 @@ import {
 	mockRequestScope,
 	sleep,
 } from "../mocks";
-import { KVError } from "../types";
+import type { KVError } from "../types";
 
 mockGlobalScope();
 
+// @ts-expect-error we use a require for a mock
+// eslint-disable-next-line @typescript-eslint/no-var-requires
 const { getAssetFromKV, mapRequestToAsset } = require("../index");
-
 test("getAssetFromKV return correct val from KV and default caching", async (t) => {
 	mockRequestScope();
 	const event = getEvent(new Request("https://blah.com/key1.txt"));
@@ -163,9 +164,9 @@ test("getAssetFromKV custom key modifier", async (t) => {
 	const event = getEvent(new Request("https://blah.com/docs/sub/blah.png"));
 
 	const customRequestMapper = (request: Request) => {
-		let defaultModifiedRequest = mapRequestToAsset(request);
+		const defaultModifiedRequest = mapRequestToAsset(request);
 
-		let url = new URL(defaultModifiedRequest.url);
+		const url = new URL(defaultModifiedRequest.url);
 		url.pathname = url.pathname.replace("/docs", "");
 		return new Request(url.toString(), request);
 	};
@@ -187,9 +188,9 @@ test("getAssetFromKV request override with existing manifest file", async (t) =>
 	const event = getEvent(new Request("https://blah.com/image.png")); // real file in manifest
 
 	const customRequestMapper = (request: Request) => {
-		let defaultModifiedRequest = mapRequestToAsset(request);
+		const defaultModifiedRequest = mapRequestToAsset(request);
 
-		let url = new URL(defaultModifiedRequest.url);
+		const url = new URL(defaultModifiedRequest.url);
 		url.pathname = "/image.webp"; // other different file in manifest
 		return new Request(url.toString(), request);
 	};
@@ -223,15 +224,16 @@ test("getAssetFromKV when setting custom cache setting", async (t) => {
 	const event1 = getEvent(new Request("https://blah.com/"));
 	const event2 = getEvent(new Request("https://blah.com/key1.png?blah=34"));
 	const cacheOnlyPngs = (req: Request) => {
-		if (new URL(req.url).pathname.endsWith(".png"))
+		if (new URL(req.url).pathname.endsWith(".png")) {
 			return {
 				browserTTL: 720,
 				edgeTTL: 720,
 			};
-		else
+		} else {
 			return {
 				bypassCache: true,
 			};
+		}
 	};
 
 	const res1 = await getAssetFromKV(event1, { cacheControl: cacheOnlyPngs });
@@ -367,10 +369,10 @@ test("getAssetFromKV TTls set to null should not cache on browser or edge", asyn
 });
 test("getAssetFromKV passing in a custom NAMESPACE serves correct asset", async (t) => {
 	mockRequestScope();
-	let CUSTOM_NAMESPACE = mockKV({
+	const CUSTOM_NAMESPACE = mockKV({
 		"key1.123HASHBROWN.txt": "val1",
 	});
-	Object.assign(global, { CUSTOM_NAMESPACE });
+	Object.assign(globalThis, { CUSTOM_NAMESPACE });
 	const event = getEvent(new Request("https://blah.com/"));
 	const res = await getAssetFromKV(event);
 	if (res) {
@@ -382,7 +384,7 @@ test("getAssetFromKV passing in a custom NAMESPACE serves correct asset", async
 });
 test("getAssetFromKV when custom namespace without the asset should fail", async (t) => {
 	mockRequestScope();
-	let CUSTOM_NAMESPACE = mockKV({
+	const CUSTOM_NAMESPACE = mockKV({
 		"key5.123HASHBROWN.txt": "customvalu",
 	});
 
@@ -394,8 +396,8 @@ test("getAssetFromKV when custom namespace without the asset should fail", async
 });
 test("getAssetFromKV when namespace not bound fails", async (t) => {
 	mockRequestScope();
-	var MY_CUSTOM_NAMESPACE = undefined;
-	Object.assign(global, { MY_CUSTOM_NAMESPACE });
+	const MY_CUSTOM_NAMESPACE: unknown = undefined;
+	Object.assign(globalThis, { MY_CUSTOM_NAMESPACE });
 
 	const event = getEvent(new Request("https://blah.com/"));
 	const error: KVError = await t.throwsAsync(

packages/kv-asset-handler/src/test/mapRequestToAsset.ts

@@ -6,33 +6,33 @@ mockGlobalScope();
 
 test("mapRequestToAsset() correctly changes /about -> /about/index.html", async (t) => {
 	mockRequestScope();
-	let path = "/about";
-	let request = new Request(`https://foo.com${path}`);
-	let newRequest = mapRequestToAsset(request);
+	const path = "/about";
+	const request = new Request(`https://foo.com${path}`);
+	const newRequest = mapRequestToAsset(request);
 	t.is(newRequest.url, request.url + "/index.html");
 });
 
 test("mapRequestToAsset() correctly changes /about/ -> /about/index.html", async (t) => {
 	mockRequestScope();
-	let path = "/about/";
-	let request = new Request(`https://foo.com${path}`);
-	let newRequest = mapRequestToAsset(request);
+	const path = "/about/";
+	const request = new Request(`https://foo.com${path}`);
+	const newRequest = mapRequestToAsset(request);
 	t.is(newRequest.url, request.url + "index.html");
 });
 
 test("mapRequestToAsset() correctly changes /about.me/ -> /about.me/index.html", async (t) => {
 	mockRequestScope();
-	let path = "/about.me/";
-	let request = new Request(`https://foo.com${path}`);
-	let newRequest = mapRequestToAsset(request);
+	const path = "/about.me/";
+	const request = new Request(`https://foo.com${path}`);
+	const newRequest = mapRequestToAsset(request);
 	t.is(newRequest.url, request.url + "index.html");
 });
 
 test("mapRequestToAsset() correctly changes /about -> /about/default.html", async (t) => {
 	mockRequestScope();
-	let path = "/about";
-	let request = new Request(`https://foo.com${path}`);
-	let newRequest = mapRequestToAsset(request, {
+	const path = "/about";
+	const request = new Request(`https://foo.com${path}`);
+	const newRequest = mapRequestToAsset(request, {
 		defaultDocument: "default.html",
 	});
 	t.is(newRequest.url, request.url + "/default.html");

packages/kv-asset-handler/src/test/serveSinglePageApp.ts

@@ -6,39 +6,39 @@ mockGlobalScope();
 
 function testRequest(path: string) {
 	mockRequestScope();
-	let url = new URL("https://example.com");
+	const url = new URL("https://example.com");
 	url.pathname = path;
-	let request = new Request(url.toString());
+	const request = new Request(url.toString());
 
 	return request;
 }
 
 test("serveSinglePageApp returns root asset path when request path ends in .html", async (t) => {
-	let path = "/foo/thing.html";
-	let request = testRequest(path);
+	const path = "/foo/thing.html";
+	const request = testRequest(path);
 
-	let expected_request = testRequest("/index.html");
-	let actual_request = serveSinglePageApp(request);
+	const expected_request = testRequest("/index.html");
+	const actual_request = serveSinglePageApp(request);
 
 	t.deepEqual(expected_request, actual_request);
 });
 
 test("serveSinglePageApp returns root asset path when request path does not have extension", async (t) => {
-	let path = "/foo/thing";
-	let request = testRequest(path);
+	const path = "/foo/thing";
+	const request = testRequest(path);
 
-	let expected_request = testRequest("/index.html");
-	let actual_request = serveSinglePageApp(request);
+	const expected_request = testRequest("/index.html");
+	const actual_request = serveSinglePageApp(request);
 
 	t.deepEqual(expected_request, actual_request);
 });
 
 test("serveSinglePageApp returns requested asset when request path has non-html extension", async (t) => {
-	let path = "/foo/thing.js";
-	let request = testRequest(path);
+	const path = "/foo/thing.js";
+	const request = testRequest(path);
 
-	let expected_request = request;
-	let actual_request = serveSinglePageApp(request);
+	const expected_request = request;
+	const actual_request = serveSinglePageApp(request);
 
 	t.deepEqual(expected_request, actual_request);
 });

packages/kv-asset-handler/src/types.ts

@@ -10,7 +10,7 @@ export type Options = {
 	cacheControl:
 		| ((req: Request) => Partial<CacheControl>)
 		| Partial<CacheControl>;
-	ASSET_NAMESPACE: any;
+	ASSET_NAMESPACE: KVNamespace;
 	ASSET_MANIFEST: AssetManifestType | string;
 	mapRequestToAsset?: (req: Request, options?: Partial<Options>) => Request;
 	defaultMimeType: string;