fix: correctly handle entry-point path when publishing

.changeset/heavy-paws-relate.md

@@ -0,0 +1,11 @@
+---
+"wrangler": patch
+---
+
+fix: correctly handle entry-point path when publishing
+
+The `publish` command was failing when the entry-point was specified in the wrangler.toml file and the entry-point imported another file.
+
+This was because we were using the `metafile.inputs` to guess the entry-point file path. But the order in which the source-files were added to this object was not well defined, and so we could end up failing to find a match.
+
+This fix avoids this by using the fact that the `metadata.outputs` object will only contain one element that has the `entrypoint` property - and then using that as the entry-point path. For runtime safety, we now assert that there cannot be zero or multiple such elements.

.changeset/young-horses-jam.md

@@ -0,0 +1,13 @@
+---
+"wrangler": patch
+---
+
+chore: add test-watch script to the wrangler workspace
+
+Watch the files in the wrangler workspace, and run the tests when anything changes:
+
+```sh
+> npm run test-watch -w wrangler
+```
+
+This will also run all the tests in a single process (rather than in parallel shards) and will increase the test-timeout to 50 seconds, which is helpful when debugging.

CONTRIBUTING.md

@@ -135,8 +135,9 @@ Tests in a workspace are executed, by [Jest](https://jestjs.io/), which is confi
   ```
 - Watch the files in a specific workspace (e.g. wrangler), and run the tests when anything changes
   ```sh
-  > npm run test -w wrangler -- --watch
+  > npm run test-watch -w wrangler
   ```
+  This will also run all the tests in a single process (rather than in parallel shards) and will increase the test-timeout to 50 seconds, which is helpful when debugging.
 
 ## Steps For Making Changes
 

packages/wrangler/package.json

@@ -97,7 +97,8 @@
     "bundle": "node -r esbuild-register scripts/bundle.ts",
     "build": "npm run clean && npm run bundle",
     "start": "npm run bundle && NODE_OPTIONS=--enable-source-maps ./bin/wrangler.js",
-    "test": "CF_API_TOKEN=some-api-token CF_ACCOUNT_ID=some-account-id jest --silent=false --verbose=true"
+    "test": "CF_API_TOKEN=some-api-token CF_ACCOUNT_ID=some-account-id jest --silent=false --verbose=true",
+    "test-watch": "npm run test -- --runInBand --testTimeout=50000 --watch"
   },
   "engines": {
     "node": ">=16.7.0"

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

@@ -0,0 +1,102 @@
+import * as fs from "node:fs";
+import { setMockResponse } from "./mock-cfetch";
+import { runInTempDir } from "./run-in-tmp";
+import { runWrangler } from "./run-wrangler";
+
+describe("publish", () => {
+  runInTempDir();
+
+  it("should be able to use `index` with no extension as the entry-point", async () => {
+    writeWranglerToml();
+    writeEsmWorkerSource();
+    mockUploadWorkerRequest();
+    mockSubDomainRequest();
+
+    const { stdout, stderr, error } = await runWrangler("publish ./index");
+
+    expect(stdout).toMatchInlineSnapshot(`
+      "Uploaded
+      test-name
+      (0.00 sec)
+      Deployed
+      test-name
+      (0.00 sec)
+
+      test-name.test-sub-domain.workers.dev"
+    `);
+    expect(stderr).toMatchInlineSnapshot(`""`);
+    expect(error).toMatchInlineSnapshot(`undefined`);
+  });
+
+  it("should be able to use the `build.upload.main` config as the entry-point for ESM sources", async () => {
+    writeWranglerToml("./index.js");
+    writeEsmWorkerSource();
+    mockUploadWorkerRequest();
+    mockSubDomainRequest();
+
+    const { stdout, stderr, error } = await runWrangler("publish");
+
+    expect(stdout).toMatchInlineSnapshot(`
+      "Uploaded
+      test-name
+      (0.00 sec)
+      Deployed
+      test-name
+      (0.00 sec)
+
+      test-name.test-sub-domain.workers.dev"
+    `);
+    expect(stderr).toMatchInlineSnapshot(`""`);
+    expect(error).toMatchInlineSnapshot(`undefined`);
+  });
+});
+
+/** Write a mock wrangler.toml file to disk. */
+function writeWranglerToml(main?: string) {
+  fs.writeFileSync(
+    "./wrangler.toml",
+    [
+      `compatibility_date = "2022-01-12"`,
+      `name = "test-name"`,
+      main !== undefined ? `[build.upload]\nmain = "${main}"` : "",
+    ].join("\n"),
+    "utf-8"
+  );
+}
+
+/** Write a mock Worker script to disk. */
+function writeEsmWorkerSource() {
+  fs.writeFileSync(
+    "index.js",
+    [
+      `import { foo } from "./another";`,
+      `export default {`,
+      `  async fetch(request) {`,
+      `    return new Response('Hello' + foo);`,
+      `  },`,
+      `};`,
+    ].join("\n")
+  );
+  fs.writeFileSync("another.js", `export const foo = 100;`);
+}
+
+/** Create a mock handler for the request to upload a worker script. */
+function mockUploadWorkerRequest(available_on_subdomain = true) {
+  setMockResponse(
+    "/accounts/:accountId/workers/scripts/:scriptName",
+    "PUT",
+    ([_url, accountId, scriptName], _init, queryParams) => {
+      expect(accountId).toEqual("some-account-id");
+      expect(scriptName).toEqual("test-name");
+      expect(queryParams.get("available_on_subdomains")).toEqual("true");
+      return { available_on_subdomain };
+    }
+  );
+}
+
+/** Create a mock handler the request for the account's subdomain. */
+function mockSubDomainRequest(subdomain = "test-sub-domain") {
+  setMockResponse("/accounts/:accountId/workers/subdomain", () => {
+    return { subdomain };
+  });
+}

packages/wrangler/src/publish.ts

@@ -1,7 +1,8 @@
 import assert from "node:assert";
 import path from "node:path";
 import { readFile } from "node:fs/promises";
-import esbuild from "esbuild";
+import * as esbuild from "esbuild";
+import type { Metafile } from "esbuild";
 import { execa } from "execa";
 import tmp from "tmp-promise";
 import type { CfWorkerInit } from "./api/worker";
@@ -78,10 +79,12 @@ export default async function publish(props: Props): Promise<void> {
 
   let file: string;
   if (props.script) {
-    file = props.script;
+    // If the script name comes from the command line it is relative to the current working directory.
+    file = path.resolve(props.script);
   } else {
+    // If the script name comes from the config, then it is relative to the wrangler.toml file.
     assert(build?.upload?.main, "missing main file");
-    file = path.join(path.dirname(__path__), build.upload.main);
+    file = path.resolve(path.dirname(__path__), build.upload.main);
   }
 
   if (props.legacyEnv) {
@@ -90,7 +93,6 @@ export default async function publish(props: Props): Promise<void> {
   const envName = props.env ?? "production";
 
   const destination = await tmp.dir({ unsafeCleanup: true });
-
   if (props.config.build?.command) {
     // TODO: add a deprecation message here?
     console.log("running:", props.config.build.command);
@@ -112,7 +114,7 @@ export default async function publish(props: Props): Promise<void> {
                 path.join(__dirname, "../static-asset-facade.js"),
                 "utf8"
               )
-            ).replace("__ENTRY_POINT__", path.join(process.cwd(), file)),
+            ).replace("__ENTRY_POINT__", file),
             sourcefile: "static-asset-facade.js",
             resolveDir: path.dirname(file),
           },
@@ -137,21 +139,28 @@ export default async function publish(props: Props): Promise<void> {
   // result.metafile is defined because of the `metafile: true` option above.
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
   const metafile = result.metafile!;
-  const expectedEntryPoint = props.public
-    ? path.join(path.dirname(file), "static-asset-facade.js")
-    : file;
-  const outputEntry = Object.entries(metafile.outputs).find(
-    ([, { entryPoint }]) => entryPoint === expectedEntryPoint
+  const entryPoints = Object.values(metafile.outputs).filter(
+    (output) => output.entryPoint !== undefined
+  );
+  assert(
+    entryPoints.length > 0,
+    `Cannot find entry-point "${file}" in generated bundle.` +
+      listEntryPoints(entryPoints)
+  );
+  assert(
+    entryPoints.length < 2,
+    "More than one entry-point found for generated bundle." +
+      listEntryPoints(entryPoints)
+  );
+  const entryPointExports = entryPoints[0].exports;
+  const resolvedEntryPointPath = path.resolve(
+    destination.path,
+    entryPoints[0].entryPoint
   );
-  if (outputEntry === undefined) {
-    throw new Error(
-      `Cannot find entry-point "${expectedEntryPoint}" in generated bundle.`
-    );
-  }
   const { format } = props;
   const bundle = {
-    type: outputEntry[1].exports.length > 0 ? "esm" : "commonjs",
-    exports: outputEntry[1].exports,
+    type: entryPointExports.length > 0 ? "esm" : "commonjs",
+    exports: entryPointExports,
   };
 
   // TODO: instead of bundling the facade with the worker, we should just bundle the worker and expose it as a module.
@@ -168,7 +177,7 @@ export default async function publish(props: Props): Promise<void> {
     return;
   }
 
-  const content = await readFile(outputEntry[0], { encoding: "utf-8" });
+  const content = await readFile(resolvedEntryPointPath, { encoding: "utf-8" });
   await destination.cleanup();
 
   // if config.migrations
@@ -229,7 +238,7 @@ export default async function publish(props: Props): Promise<void> {
   const worker: CfWorkerInit = {
     name: scriptName,
     main: {
-      name: path.basename(outputEntry[0]),
+      name: path.basename(resolvedEntryPointPath),
       content: content,
       type: bundle.type === "esm" ? "esm" : "commonjs",
     },
@@ -262,12 +271,13 @@ export default async function publish(props: Props): Promise<void> {
 
   // Upload the script so it has time to propagate.
   const { available_on_subdomain } = await fetchResult(
-    `${workerUrl}?available_on_subdomain=true`,
+    workerUrl,
     {
       method: "PUT",
       // @ts-expect-error: TODO: fix this type error!
       body: toFormData(worker),
-    }
+    },
+    new URLSearchParams({ available_on_subdomains: "true" })
   );
 
   const uploadMs = Date.now() - start;
@@ -362,3 +372,9 @@ export default async function publish(props: Props): Promise<void> {
     console.log(" ", target);
   }
 }
+
+function listEntryPoints(outputs: ValueOf<Metafile["outputs"]>[]): string {
+  return outputs.map((output) => output.entryPoint).join("\n");
+}
+
+type ValueOf<T> = T[keyof T];