fix: publish environment specific routes

This adds some tests for publishing routes, and fixes a couple of bugs with the flow.
- fixes publishing environment specific routes, closes #513
- default workers_dev to false if there are any routes specified
- catches a hanging promise when we were toggling off a workers.dev subdomain (which should have been caught by the no-floating-promises lint rule, so that's concerning)
- this also fixes publishing environment specific crons, but I'll write tests for that when I'm doing that feature in depth

I expect to send some followup PRs for routes, but this should make it a little more stable right now

.changeset/brown-spies-deliver.md

@@ -0,0 +1,12 @@
+---
+"wrangler": patch
+---
+
+fix: publish environment specific routes
+
+This adds some tests for publishing routes, and fixes a couple of bugs with the flow.
+
+- fixes publishing environment specific routes, closes https://github.com/cloudflare/wrangler2/issues/513
+- default `workers_dev` to `false` if there are any routes specified
+- catches a hanging promise when we were toggling off a `workers.dev` subdomain (which should have been caught by the `no-floating-promises` lint rule, so that's concerning)
+- this also fixes publishing environment specific crons, but I'll write tests for that when I'm doing that feature in depth

packages/wrangler/src/__tests__/helpers/write-wrangler-toml.ts

@@ -3,10 +3,7 @@ import TOML from "@iarna/toml";
 import type { Config } from "../../config";
 
 /** Write a mock wrangler.toml file to disk. */
-export default function writeWranglerToml(config: Omit<Config, "env"> = {}) {
-  // We Omit `env` from config because TOML.stringify() appears to
-  // have a weird type signature that appears to fail. We'll revisit this
-  // when we write tests for publishing environments
+export default function writeWranglerToml(config: Config = {}) {
   fs.writeFileSync(
     "./wrangler.toml",
     TOML.stringify({

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

@@ -160,6 +160,67 @@ describe("publish", () => {
     expect(std.err).toMatchInlineSnapshot(`""`);
   });
 
+  describe("routes", () => {
+    it("should publish the worker to a route", async () => {
+      writeWranglerToml({
+        routes: ["example.com/some-route/*"],
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ enabled: false });
+      mockUploadWorkerRequest({ expectedType: "esm" });
+      mockPublishRoutesRequest({ routes: ["example.com/some-route/*"] });
+      await runWrangler("publish ./index");
+    });
+
+    it("should publish to legacy environment specific routes", async () => {
+      writeWranglerToml({
+        routes: ["example.com/some-route/*"],
+        env: {
+          dev: {
+            routes: ["dev-example.com/some-route/*"],
+          },
+        },
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ enabled: false, legacyEnv: true, env: "dev" });
+      mockUploadWorkerRequest({
+        expectedType: "esm",
+        legacyEnv: true,
+        env: "dev",
+      });
+      mockPublishRoutesRequest({
+        routes: ["dev-example.com/some-route/*"],
+        legacyEnv: true,
+        env: "dev",
+      });
+      await runWrangler("publish ./index --env dev --legacy-env true");
+    });
+
+    it("services: should publish to service environment specific routes", async () => {
+      writeWranglerToml({
+        routes: ["example.com/some-route/*"],
+        env: {
+          dev: {
+            routes: ["dev-example.com/some-route/*"],
+          },
+        },
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ enabled: false, env: "dev" });
+      mockUploadWorkerRequest({
+        expectedType: "esm",
+        env: "dev",
+      });
+      mockPublishRoutesRequest({
+        routes: ["dev-example.com/some-route/*"],
+        env: "dev",
+      });
+      await runWrangler("publish ./index --env dev");
+    });
+
+    it.todo("should error if it's a workers.dev route");
+  });
+
   describe("entry-points", () => {
     it("should be able to use `index` with no extension as the entry-point (esm)", async () => {
       writeWranglerToml();
@@ -1770,11 +1831,14 @@ function mockUploadWorkerRequest({
       ? "/accounts/:accountId/workers/services/:scriptName/environments/:envName"
       : "/accounts/:accountId/workers/scripts/:scriptName",
     "PUT",
-    async ([_url, accountId, scriptName], { body }, queryParams) => {
+    async ([_url, accountId, scriptName, envName], { body }, queryParams) => {
       expect(accountId).toEqual("some-account-id");
       expect(scriptName).toEqual(
         legacyEnv && env ? `test-name-${env}` : "test-name"
       );
+      if (!legacyEnv) {
+        expect(envName).toEqual(env);
+      }
       expect(queryParams.get("available_on_subdomain")).toEqual("true");
       const formBody = body as FormData;
       if (expectedEntry !== undefined) {
@@ -1810,27 +1874,62 @@ function mockSubDomainRequest(subdomain = "test-sub-domain") {
   });
 }
 
+/** Create a mock handler to toggle a <script>.<user>.workers.dev subdomain */
 function mockUpdateWorkerRequest({
   env,
   enabled,
+  legacyEnv = false,
 }: {
-  env?: string | undefined;
   enabled: boolean;
+  env?: string | undefined;
+  legacyEnv?: boolean | undefined;
 }) {
   const requests = { count: 0 };
-  const servicesOrScripts = env ? "services" : "scripts";
-  const environment = env ? "/environments/:envName" : "";
+  const servicesOrScripts = env && !legacyEnv ? "services" : "scripts";
+  const environment = env && !legacyEnv ? "/environments/:envName" : "";
   setMockResponse(
     `/accounts/:accountId/workers/${servicesOrScripts}/:scriptName${environment}/subdomain`,
     "POST",
-    (_, { body }) => {
+    ([_url, accountId, scriptName], { body }) => {
+      expect(accountId).toEqual("some-account-id");
+      expect(scriptName).toEqual(
+        legacyEnv && env ? `test-name-${env}` : "test-name"
+      );
       expect(JSON.parse(body as string)).toEqual({ enabled });
       return null;
     }
   );
   return requests;
 }
 
+function mockPublishRoutesRequest({
+  routes,
+  env = undefined,
+  legacyEnv = false,
+}: {
+  routes: string[];
+  env?: string | undefined;
+  legacyEnv?: boolean | undefined;
+}) {
+  const servicesOrScripts = env && !legacyEnv ? "services" : "scripts";
+  const environment = env && !legacyEnv ? "/environments/:envName" : "";
+
+  setMockResponse(
+    `/accounts/:accountId/workers/${servicesOrScripts}/:scriptName${environment}/routes`,
+    "PUT",
+    ([_url, accountId, scriptName], { body }) => {
+      expect(accountId).toEqual("some-account-id");
+      expect(scriptName).toEqual(
+        legacyEnv && env ? `test-name-${env}` : "test-name"
+      );
+      expect(JSON.parse(body as string)).toEqual(
+        routes.map((pattern) => ({ pattern }))
+      );
+      return null;
+    }
+  );
+}
+
 /** Create a mock handler for the request to get a list of all KV namespaces. */
 function mockListKVNamespacesRequest(...namespaces: KVNamespaceInfo[]) {
   setMockResponse(

packages/wrangler/src/index.tsx

@@ -831,7 +831,7 @@ export async function main(argv: string[]): Promise<void> {
             (args["compatibility-flags"] as string[]) ||
             config.compatibility_flags
           }
-          usageModel={config.usage_model}
+          usageModel={envRootObj.usage_model}
           bindings={{
             kv_namespaces: envRootObj.kv_namespaces?.map(
               ({ binding, preview_id, id: _id }) => {

packages/wrangler/src/publish.ts

@@ -37,8 +37,6 @@ 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, workers_dev: deployToWorkersDev = true } =
-    config;
 
   const envRootObj =
     props.env && config.env ? config.env[props.env] || {} : config;
@@ -48,15 +46,21 @@ export default async function publish(props: Props): Promise<void> {
     "A compatibility_date is required when publishing. Add one to your wrangler.toml file, or pass it in your terminal as --compatibility_date. See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
   );
 
+  const triggers = props.triggers || envRootObj.triggers?.crons;
+  const routes =
+    props.routes ||
+    envRootObj.routes ||
+    (envRootObj.route ? [envRootObj.route] : undefined);
+
+  const { account_id: accountId, workers_dev: deployToWorkersDev = !routes } =
+    config;
+
   if (accountId === undefined) {
     throw new Error("No account_id provided.");
   }
 
-  const triggers = props.triggers || config.triggers?.crons;
-  const routes = props.routes || config.routes;
-
-  const jsxFactory = props.jsxFactory || config.jsx_factory;
-  const jsxFragment = props.jsxFragment || config.jsx_fragment;
+  const jsxFactory = props.jsxFactory || envRootObj.jsx_factory;
+  const jsxFragment = props.jsxFragment || envRootObj.jsx_fragment;
 
   assert(config.account_id, "missing account id");
 
@@ -253,14 +257,13 @@ export default async function publish(props: Props): Promise<void> {
     console.log("Uploaded", workerName, formatTime(uploadMs));
     const deployments: Promise<string[]>[] = [];
 
-    const userSubdomain = (
-      await fetchResult<{ subdomain: string }>(
-        `/accounts/${accountId}/workers/subdomain`
-      )
-    ).subdomain;
-
     if (deployToWorkersDev) {
       // Deploy to a subdomain of `workers.dev`
+      const userSubdomain = (
+        await fetchResult<{ subdomain: string }>(
+          `/accounts/${accountId}/workers/subdomain`
+        )
+      ).subdomain;
       const scriptURL =
         props.legacyEnv || !props.env
           ? `${scriptName}.${userSubdomain}.workers.dev`
@@ -290,7 +293,7 @@ export default async function publish(props: Props): Promise<void> {
       }
     } else {
       // Disable the workers.dev deployment
-      fetchResult(`${workerUrl}/subdomain`, {
+      await fetchResult(`${workerUrl}/subdomain`, {
         method: "POST",
         body: JSON.stringify({ enabled: false }),
         headers: {
@@ -300,7 +303,7 @@ export default async function publish(props: Props): Promise<void> {
     }
 
     // Update routing table for the script.
-    if (routes && routes.length) {
+    if (routes && routes.length > 0) {
       deployments.push(
         fetchResult(`${workerUrl}/routes`, {
           // TODO: PATCH will not delete previous routes on this script,