From f9c1423f0c5b6008f05b9657c9b84eb6f173563a Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 18 Jan 2022 08:58:22 +0000 Subject: [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. Fixes #252 --- .changeset/heavy-paws-relate.md | 11 ++ .../wrangler/src/__tests__/publish.test.ts | 102 ++++++++++++++++++ packages/wrangler/src/publish.ts | 58 ++++++---- 3 files changed, 150 insertions(+), 21 deletions(-) create mode 100644 .changeset/heavy-paws-relate.md create mode 100644 packages/wrangler/src/__tests__/publish.test.ts diff --git a/.changeset/heavy-paws-relate.md b/.changeset/heavy-paws-relate.md new file mode 100644 index 000000000000..cb983e0fc6df --- /dev/null +++ b/.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. diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts new file mode 100644 index 000000000000..b006264e48c1 --- /dev/null +++ b/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 }; + }); +} diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index 51250d515039..ffba03ef924e 100644 --- a/packages/wrangler/src/publish.ts +++ b/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 { 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 { 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 { 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 { // 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 { 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 { 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 { // 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 { console.log(" ", target); } } + +function listEntryPoints(outputs: ValueOf[]): string { + return outputs.map((output) => output.entryPoint).join("\n"); +} + +type ValueOf = T[keyof T];