Skip to content

Commit

Permalink
fix: correctly handle entry-point path when publishing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
petebacondarwin committed Jan 19, 2022
1 parent 014a731 commit f9c1423
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 21 deletions.
11 changes: 11 additions & 0 deletions .changeset/heavy-paws-relate.md
Original file line number Diff line number Diff line change
@@ -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.
102 changes: 102 additions & 0 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
@@ -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 };
});
}
58 changes: 37 additions & 21 deletions packages/wrangler/src/publish.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -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),
},
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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];

0 comments on commit f9c1423

Please sign in to comment.