Skip to content

Commit

Permalink
chore: pass lint/typechecks for kv-asset-handler (#6102)
Browse files Browse the repository at this point in the history 
Followup from #6090, this enables typechecking and linting in kv-asset-handler, and fixes any failures.
  • Loading branch information
@threepointone
threepointone authored Jun 20, 2024
1 parent d0b4469 commit d4e1e9f
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 94 deletions.
7 changes: 7 additions & 0 deletions .changeset/healthy-snails-heal.md
View file
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 4 additions & 1 deletion packages/kv-asset-handler/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/kv-asset-handler/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
56 changes: 30 additions & 26 deletions packages/kv-asset-handler/src/mocks.ts
View file
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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,
};
};

Expand All @@ -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);
Expand Down Expand Up @@ -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("/", "")}"`,
Expand All @@ -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) => {
Expand Down
18 changes: 6 additions & 12 deletions packages/kv-asset-handler/src/test/getAssetFromKV-optional.ts
View file
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
28 changes: 15 additions & 13 deletions packages/kv-asset-handler/src/test/getAssetFromKV.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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);
};
Expand All @@ -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);
};
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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) {
Expand All @@ -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",
});

Expand All @@ -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(
Expand Down
24 changes: 12 additions & 12 deletions packages/kv-asset-handler/src/test/mapRequestToAsset.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
28 changes: 14 additions & 14 deletions packages/kv-asset-handler/src/test/serveSinglePageApp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
2 changes: 1 addition & 1 deletion packages/kv-asset-handler/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit d4e1e9f

Please sign in to comment.