fix: do not publish to workers.dev if workers_dev is false

.changeset/rich-fans-shake.md

@@ -0,0 +1,11 @@
+---
+"wrangler": patch
+---
+
+fix: do not publish to workers.dev if workers_dev is false
+
+Previously we always published to the workers.dev subdomain, ignoring the `workers_dev` setting in the `wrangler.toml` configuration.
+
+Now we respect this configuration setting, and also disable an current workers.dev subdomain worker when we publish and `workers_dev` is `false`.
+
+Fixes #410

.changeset/sour-eagles-confess.md

@@ -0,0 +1,8 @@
+---
+"wrangler": patch
+---
+
+fix: pass correct query param when uploading a script
+
+In f9c1423f0c5b6008f05b9657c9b84eb6f173563a the query param was incorrectly changed from
+`available_on_subdomain` to `available_on_subdomains`.

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

@@ -1051,6 +1051,118 @@ export default{
     });
   });
 
+  describe("workers_dev setting", () => {
+    it("should deploy to a workers.dev domain if workers_dev is undefined", async () => {
+      writeWranglerToml();
+      writeWorkerSource();
+      mockUploadWorkerRequest();
+      mockSubDomainRequest();
+
+      await runWrangler("publish ./index");
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded
+        test-name
+        (TIMINGS)
+        Deployed
+        test-name
+        (TIMINGS)
+
+        test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+
+    it("should deploy to the workers.dev domain if workers_dev is `true`", async () => {
+      writeWranglerToml({
+        workers_dev: true,
+      });
+      writeWorkerSource();
+      mockUploadWorkerRequest();
+      mockSubDomainRequest();
+
+      await runWrangler("publish ./index");
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded
+        test-name
+        (TIMINGS)
+        Deployed
+        test-name
+        (TIMINGS)
+
+        test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+
+    it("should disable the workers.dev domain if workers_dev is `false`", async () => {
+      writeWranglerToml({
+        workers_dev: false,
+      });
+      writeWorkerSource();
+      mockUploadWorkerRequest();
+      mockSubDomainRequest();
+      mockUpdateWorkerRequest({ enabled: false });
+
+      await runWrangler("publish ./index");
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded
+        test-name
+        (TIMINGS)
+        No deployment targets for
+        test-name
+        (TIMINGS)"
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+
+    it("should enable the workers.dev domain if workers_dev is undefined and subdomain is not already available", async () => {
+      writeWranglerToml();
+      writeWorkerSource();
+      mockUploadWorkerRequest({ available_on_subdomain: false });
+      mockSubDomainRequest();
+      mockUpdateWorkerRequest({ enabled: true });
+
+      await runWrangler("publish ./index");
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded
+        test-name
+        (TIMINGS)
+        Deployed
+        test-name
+        (TIMINGS)
+
+        test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+
+    it("should enable the workers.dev domain if workers_dev is true and subdomain is not already available", async () => {
+      writeWranglerToml({ workers_dev: true });
+      writeWorkerSource();
+      mockUploadWorkerRequest({ available_on_subdomain: false });
+      mockSubDomainRequest();
+      mockUpdateWorkerRequest({ enabled: true });
+
+      await runWrangler("publish ./index");
+
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded
+        test-name
+        (TIMINGS)
+        Deployed
+        test-name
+        (TIMINGS)
+
+        test-name.test-sub-domain.workers.dev"
+      `);
+      expect(std.err).toMatchInlineSnapshot(`""`);
+    });
+  });
+
   describe("custom builds", () => {
     it("should run a custom build before publishing", async () => {
       writeWranglerToml({
@@ -1464,7 +1576,7 @@ function mockUploadWorkerRequest({
     async ([_url, accountId, scriptName], { body }, queryParams) => {
       expect(accountId).toEqual("some-account-id");
       expect(scriptName).toEqual("test-name");
-      expect(queryParams.get("available_on_subdomains")).toEqual("true");
+      expect(queryParams.get("available_on_subdomain")).toEqual("true");
       const formBody = body as FormData;
       if (expectedEntry !== undefined) {
         expect(await (formBody.get("index.js") as File).text()).toMatch(
@@ -1494,11 +1606,32 @@ function mockUploadWorkerRequest({
 
 /** Create a mock handler for the request to get the account's subdomain. */
 function mockSubDomainRequest(subdomain = "test-sub-domain") {
-  setMockResponse("/accounts/:accountId/workers/subdomain", () => {
+  setMockResponse("/accounts/:accountId/workers/subdomain", "GET", () => {
     return { subdomain };
   });
 }
 
+function mockUpdateWorkerRequest({
+  env,
+  enabled,
+}: {
+  env?: string | undefined;
+  enabled: boolean;
+}) {
+  const requests = { count: 0 };
+  const servicesOrScripts = env ? "services" : "scripts";
+  const environment = env ? "/environments/:envName" : "";
+  setMockResponse(
+    `/accounts/:accountId/workers/${servicesOrScripts}/:scriptName${environment}/subdomain`,
+    "POST",
+    (_, { body }) => {
+      expect(JSON.parse(body as string)).toEqual({ enabled });
+      return null;
+    }
+  );
+  return requests;
+}
+
 /** Create a mock handler for the request to get a list of all KV namespaces. */
 function mockListKVNamespacesRequest(...namespaces: KVNamespaceInfo[]) {
   setMockResponse(

packages/wrangler/src/publish.ts

@@ -37,7 +37,8 @@ function sleep(ms: number) {
 export default async function publish(props: Props): Promise<void> {
   // TODO: warn if git/hg has uncommitted changes
   const { config } = props;
-  const { account_id: accountId } = config;
+  const { account_id: accountId, workers_dev: deployToWorkersDev = true } =
+    config;
 
   const envRootObj =
     props.env && config.env ? config.env[props.env] || {} : config;
@@ -242,7 +243,7 @@ export default async function publish(props: Props): Promise<void> {
         method: "PUT",
         body: toFormData(worker),
       },
-      new URLSearchParams({ available_on_subdomains: "true" })
+      new URLSearchParams({ available_on_subdomain: "true" })
     );
 
     const uploadMs = Date.now() - start;
@@ -255,34 +256,44 @@ export default async function publish(props: Props): Promise<void> {
       )
     ).subdomain;
 
-    const scriptURL =
-      props.legacyEnv || !props.env
-        ? `${scriptName}.${userSubdomain}.workers.dev`
-        : `${envName}.${scriptName}.${userSubdomain}.workers.dev`;
-
-    // Enable the `workers.dev` subdomain.
-    // TODO: Make this configurable.
-    if (!available_on_subdomain) {
-      deployments.push(
-        fetchResult(`${workerUrl}/subdomain`, {
-          method: "POST",
-          body: JSON.stringify({ enabled: true }),
-          headers: {
-            "Content-Type": "application/json",
-          },
-        })
-          .then(() => [scriptURL])
-          // Add a delay when the subdomain is first created.
-          // This is to prevent an issue where a negative cache-hit
-          // causes the subdomain to be unavailable for 30 seconds.
-          // This is a temporary measure until we fix this on the edge.
-          .then(async (url) => {
-            await sleep(3000);
-            return url;
+    if (deployToWorkersDev) {
+      // Deploy to a subdomain of `workers.dev`
+      const scriptURL =
+        props.legacyEnv || !props.env
+          ? `${scriptName}.${userSubdomain}.workers.dev`
+          : `${envName}.${scriptName}.${userSubdomain}.workers.dev`;
+      if (!available_on_subdomain) {
+        // Enable the `workers.dev` subdomain.
+        deployments.push(
+          fetchResult(`${workerUrl}/subdomain`, {
+            method: "POST",
+            body: JSON.stringify({ enabled: true }),
+            headers: {
+              "Content-Type": "application/json",
+            },
           })
-      );
+            .then(() => [scriptURL])
+            // Add a delay when the subdomain is first created.
+            // This is to prevent an issue where a negative cache-hit
+            // causes the subdomain to be unavailable for 30 seconds.
+            // This is a temporary measure until we fix this on the edge.
+            .then(async (url) => {
+              await sleep(3000);
+              return url;
+            })
+        );
+      } else {
+        deployments.push(Promise.resolve([scriptURL]));
+      }
     } else {
-      deployments.push(Promise.resolve([scriptURL]));
+      // Disable the workers.dev deployment
+      fetchResult(`${workerUrl}/subdomain`, {
+        method: "POST",
+        body: JSON.stringify({ enabled: false }),
+        headers: {
+          "Content-Type": "application/json",
+        },
+      });
     }
 
     // Update routing table for the script.
@@ -326,15 +337,20 @@ export default async function publish(props: Props): Promise<void> {
       );
     }
 
-    if (!deployments.length) {
-      return;
-    }
-
     const targets = await Promise.all(deployments);
     const deployMs = Date.now() - start - uploadMs;
-    console.log("Deployed", workerName, formatTime(deployMs));
-    for (const target of targets.flat()) {
-      console.log(" ", target);
+
+    if (deployments.length > 0) {
+      console.log("Deployed", workerName, formatTime(deployMs));
+      for (const target of targets.flat()) {
+        console.log(" ", target);
+      }
+    } else {
+      console.log(
+        "No deployment targets for",
+        workerName,
+        formatTime(deployMs)
+      );
     }
   } finally {
     await destination.cleanup();