feat: delete site/assets namespace when a worker is deleted

This patch deletes any site/asset kv namespaces associated with a worker when `wrangler delete` is used. It finds the namespace associated with a worker by using the names it would have otherwise used, and deletes it. It also does the same for the preview namespace that's used with `wrangler dev`.

.changeset/poor-cycles-grow.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+feat: delete site/assets namespace when a worker is deleted
+
+This patch deletes any site/asset kv namespaces associated with a worker when `wrangler delete` is used. It finds the namespace associated with a worker by using the names it would have otherwise used, and deletes it. It also does the same for the preview namespace that's used with `wrangler dev`.

packages/wrangler/src/__tests__/delete.test.ts

@@ -5,6 +5,7 @@ import { mockConfirm } from "./helpers/mock-dialogs";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 import writeWranglerToml from "./helpers/write-wrangler-toml";
+import type { KVNamespaceInfo } from "../kv/helpers";
 
 describe("delete", () => {
 	mockAccountId();
@@ -22,6 +23,7 @@ describe("delete", () => {
 			text: `Are you sure you want to delete my-script? This action cannot be undone.`,
 			result: true,
 		});
+		mockListKVNamespacesRequest();
 		mockDeleteWorkerRequest({ name: "my-script" });
 		await runWrangler("delete --name my-script");
 
@@ -41,6 +43,7 @@ describe("delete", () => {
 			result: true,
 		});
 		writeWranglerToml();
+		mockListKVNamespacesRequest();
 		mockDeleteWorkerRequest();
 		await runWrangler("delete");
 
@@ -84,6 +87,105 @@ describe("delete", () => {
 		}
 	`);
 	});
+
+	it("should delete a site namespace associated with a worker", async () => {
+		const kvNamespaces = [
+			{
+				title: "__my-script-workers_sites_assets",
+				id: "id-for-my-script-site-ns",
+			},
+			// this one isn't associated with the worker
+			{
+				title: "__test-name-workers_sites_assets",
+				id: "id-for-another-site-ns",
+			},
+		];
+
+		mockConfirm({
+			text: `Are you sure you want to delete my-script? This action cannot be undone.`,
+			result: true,
+		});
+		mockListKVNamespacesRequest(...kvNamespaces);
+		// it should only try to delete the site namespace associated with this worker
+		setMockResponse(
+			"/accounts/:accountId/storage/kv/namespaces/id-for-my-script-site-ns",
+			"DELETE",
+			([_url, accountId]) => {
+				expect(accountId).toEqual("some-account-id");
+				return null;
+			}
+		);
+		mockDeleteWorkerRequest({ name: "my-script" });
+		await runWrangler("delete --name my-script");
+		expect(std).toMatchInlineSnapshot(`
+		Object {
+		  "debug": "",
+		  "err": "",
+		  "out": "🌀 Deleted asset namespace for Workers Site \\"__my-script-workers_sites_assets\\"
+		Successfully deleted my-script",
+		  "warn": "",
+		}
+	`);
+	});
+
+	it("should delete a site namespace associated with a worker, including it's preview namespace", async () => {
+		// This is the same test as the previous one, but it includes a preview namespace
+		const kvNamespaces = [
+			{
+				title: "__my-script-workers_sites_assets",
+				id: "id-for-my-script-site-ns",
+			},
+			// this is the preview namespace
+			{
+				title: "__my-script-workers_sites_assets_preview",
+				id: "id-for-my-script-site-preview-ns",
+			},
+
+			// this one isn't associated with the worker
+			{
+				title: "__test-name-workers_sites_assets",
+				id: "id-for-another-site-ns",
+			},
+		];
+
+		mockConfirm({
+			text: `Are you sure you want to delete my-script? This action cannot be undone.`,
+			result: true,
+		});
+		mockListKVNamespacesRequest(...kvNamespaces);
+		// it should only try to delete the site namespace associated with this worker
+
+		setMockResponse(
+			"/accounts/:accountId/storage/kv/namespaces/id-for-my-script-site-ns",
+			"DELETE",
+			([_url, accountId]) => {
+				expect(accountId).toEqual("some-account-id");
+				return null;
+			}
+		);
+
+		setMockResponse(
+			"/accounts/:accountId/storage/kv/namespaces/id-for-my-script-site-preview-ns",
+			"DELETE",
+			([_url, accountId]) => {
+				expect(accountId).toEqual("some-account-id");
+				return null;
+			}
+		);
+
+		mockDeleteWorkerRequest({ name: "my-script" });
+		await runWrangler("delete --name my-script");
+		expect(std).toMatchInlineSnapshot(`
+		Object {
+		  "debug": "",
+		  "err": "",
+		  "out": "🌀 Deleted asset namespace for Workers Site \\"__my-script-workers_sites_assets\\"
+		🌀 Deleted asset namespace for Workers Site \\"__my-script-workers_sites_assets_preview\\"
+		Successfully deleted my-script",
+		  "warn": "",
+		}
+	`);
+	});
 });
 
 /** Create a mock handler for the request to upload a worker script. */
@@ -114,3 +216,15 @@ function mockDeleteWorkerRequest(
 		}
 	);
 }
+
+/** Create a mock handler for the request to get a list of all KV namespaces. */
+function mockListKVNamespacesRequest(...namespaces: KVNamespaceInfo[]) {
+	setMockResponse(
+		"/accounts/:accountId/storage/kv/namespaces",
+		"GET",
+		([_url, accountId]) => {
+			expect(accountId).toEqual("some-account-id");
+			return namespaces;
+		}
+	);
+}

packages/wrangler/src/delete.ts

@@ -1,9 +1,11 @@
+import assert from "assert";
 import path from "path";
 import { fetchResult } from "./cfetch";
 import { findWranglerToml, readConfig } from "./config";
 import { confirm } from "./dialogs";
 import { CI } from "./is-ci";
 import isInteractive from "./is-interactive";
+import { deleteKVNamespace, listKVNamespaces } from "./kv/helpers";
 import { logger } from "./logger";
 import * as metrics from "./metrics";
 import { requireAuth } from "./user";
@@ -60,11 +62,18 @@ export async function deleteHandler(args: ArgumentsCamelCase<DeleteArgs>) {
 
 	const scriptName = getScriptName(args, config);
 
+	assert(
+		scriptName,
+		"A worker name must be defined, either via --name, or in wrangler.toml"
+	);
+
 	if (args.dryRun) {
 		logger.log(`--dry-run: exiting now.`);
 		return;
 	}
 
+	assert(accountId, "Missing accountId");
+
 	let confirmed = true;
 	if (isInteractive() || !CI.isCI()) {
 		confirmed = await confirm(
@@ -79,8 +88,24 @@ export async function deleteHandler(args: ArgumentsCamelCase<DeleteArgs>) {
 			new URLSearchParams({ force: "true" })
 		);
 
+		await deleteSiteNamespaceIfExisting(scriptName, accountId);
+
 		logger.log("Successfully deleted", scriptName);
 	}
+}
 
-	// TODO: maybe delete sites/assets kv namespace as well?
+async function deleteSiteNamespaceIfExisting(
+	scriptName: string,
+	accountId: string
+): Promise<void> {
+	const title = `__${scriptName}-workers_sites_assets`;
+	const previewTitle = `__${scriptName}-workers_sites_assets_preview`;
+	const allNamespaces = await listKVNamespaces(accountId);
+	const namespacesToDelete = allNamespaces.filter(
+		(ns) => ns.title === title || ns.title === previewTitle
+	);
+	for (const ns of namespacesToDelete) {
+		await deleteKVNamespace(accountId, ns.id);
+		logger.log(`🌀 Deleted asset namespace for Workers Site "${ns.title}"`);
+	}
 }

packages/wrangler/src/sites.tsx

@@ -77,7 +77,7 @@ async function createKVNamespaceIfNotAlreadyExisting(
 	// check if it already exists
 	// TODO: this is super inefficient, should be made better
 	const namespaces = await listKVNamespaces(accountId);
-	const found = namespaces.find((x) => x.title === title);
+	const found = namespaces.find((ns) => ns.title === title);
 	if (found) {
 		return { created: false, id: found.id };
 	}