From 05b81c5809b9ceed10d0c21c0f5f5de76b23a67d Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 12 Mar 2022 18:52:32 +0530 Subject: [PATCH] fix: consolidate `getEntry()` logic (#567) This consolidates some logic into `getEntry()`, namely including `guessWorkerFormat()` and custom builds. This simplifies the code for both `dev` and `publish`. - Previously, the implementation of custom builds inside `dev` assumed it could be a long running process; however it's not (else consider that `publish` would never work). - By running custom builds inside `getEntry()`, we can be certain that the entry point exists as we validate it and before we enter `dev`/`publish`, simplifying their internals - We don't have to do periodic checks inside `wrangler dev` because it's now a one shot build (and always should have been) - This expands test coverage a little for both `dev` and `publish`. - The 'format' of a worker is intrinsic to its contents, so it makes sense to establish its value inside `getEntry()` - This also means less async logic inside ``, which is always a good thing --- .changeset/clean-trainers-clap.md | 14 ++ packages/wrangler/src/__tests__/dev.test.tsx | 78 +++++++++- .../src/__tests__/guess-worker-format.test.ts | 26 ++-- .../wrangler/src/__tests__/publish.test.ts | 24 +++ packages/wrangler/src/bundle.ts | 15 +- packages/wrangler/src/config/config.ts | 5 +- packages/wrangler/src/dev.tsx | 140 ++++-------------- packages/wrangler/src/entry.ts | 120 +++++++++++++-- packages/wrangler/src/guess-worker-format.ts | 68 --------- packages/wrangler/src/index.tsx | 12 +- packages/wrangler/src/publish.ts | 33 +---- 11 files changed, 281 insertions(+), 254 deletions(-) create mode 100644 .changeset/clean-trainers-clap.md delete mode 100644 packages/wrangler/src/guess-worker-format.ts diff --git a/.changeset/clean-trainers-clap.md b/.changeset/clean-trainers-clap.md new file mode 100644 index 000000000000..c2b4ef56d682 --- /dev/null +++ b/.changeset/clean-trainers-clap.md @@ -0,0 +1,14 @@ +--- +"wrangler": patch +--- + +fix: consolidate `getEntry()` logic + +This consolidates some logic into `getEntry()`, namely including `guessWorkerFormat()` and custom builds. This simplifies the code for both `dev` and `publish`. + +- Previously, the implementation of custom builds inside `dev` assumed it could be a long running process; however it's not (else consider that `publish` would never work). +- By running custom builds inside `getEntry()`, we can be certain that the entry point exists as we validate it and before we enter `dev`/`publish`, simplifying their internals +- We don't have to do periodic checks inside `wrangler dev` because it's now a one shot build (and always should have been) +- This expands test coverage a little for both `dev` and `publish`. +- The 'format' of a worker is intrinsic to its contents, so it makes sense to establish its value inside `getEntry()` +- This also means less async logic inside ``, which is always a good thing diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx index bca0b0b984ec..de0215a81a98 100644 --- a/packages/wrangler/src/__tests__/dev.test.tsx +++ b/packages/wrangler/src/__tests__/dev.test.tsx @@ -1,4 +1,5 @@ import * as fs from "node:fs"; +import { readFileSync } from "node:fs"; import patchConsole from "patch-console"; import Dev from "../dev"; import { mockAccountId, mockApiToken } from "./helpers/mock-account-id"; @@ -9,7 +10,6 @@ import { runWrangler } from "./helpers/run-wrangler"; import writeWranglerToml from "./helpers/write-wrangler-toml"; describe("Dev component", () => { - beforeEach(() => {}); mockAccountId(); mockApiToken(); runInTempDir(); @@ -231,6 +231,82 @@ describe("Dev component", () => { ); }); }); + + describe("custom builds", () => { + it("should run a custom build before starting `dev`", async () => { + writeWranglerToml({ + build: { + command: `node -e "console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')"`, + }, + }); + + await runWrangler("dev index.js"); + + expect(readFileSync("index.js", "utf-8")).toMatchInlineSnapshot( + `"export default { fetch(){ return new Response(123) } }"` + ); + + // and the command would pass through + expect((Dev as jest.Mock).mock.calls[0][0].buildCommand).toEqual({ + command: + "node -e \"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\"", + cwd: undefined, + watch_dir: undefined, + }); + expect(std.out).toMatchInlineSnapshot( + `"running: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""` + ); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + if (process.platform !== "win32") { + it("should run a custom build of multiple steps combined by && before starting `dev`", async () => { + writeWranglerToml({ + build: { + command: `echo "custom build" && echo "export default { fetch(){ return new Response(123) } }" > index.js`, + }, + }); + + await runWrangler("dev index.js"); + + expect(readFileSync("index.js", "utf-8")).toMatchInlineSnapshot(` + "export default { fetch(){ return new Response(123) } } + " + `); + + expect(std.out).toMatchInlineSnapshot( + `"running: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"` + ); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + } + + it("should throw an error if the entry doesn't exist after the build finishes", async () => { + writeWranglerToml({ + main: "index.js", + build: { + command: `node -e "console.log('custom build');"`, + }, + }); + + await expect( + runWrangler("dev") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""` + ); + expect(std.out).toMatchInlineSnapshot( + `"running: node -e \\"console.log('custom build');\\""` + ); + expect(std.err).toMatchInlineSnapshot(` + "Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\" + + %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new." + `); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + }); }); function mockGetZones(domain: string, zones: { id: string }[] = []) { diff --git a/packages/wrangler/src/__tests__/guess-worker-format.test.ts b/packages/wrangler/src/__tests__/guess-worker-format.test.ts index 37819fa6d94e..ae70df7ef410 100644 --- a/packages/wrangler/src/__tests__/guess-worker-format.test.ts +++ b/packages/wrangler/src/__tests__/guess-worker-format.test.ts @@ -1,6 +1,6 @@ import { writeFile } from "fs/promises"; import path from "path"; -import guessWorkerFormat from "../guess-worker-format"; +import guessWorkerFormat from "../entry"; import { runInTempDir } from "./helpers/run-in-tmp"; describe("guess worker format", () => { @@ -10,10 +10,8 @@ describe("guess worker format", () => { // Note that this isn't actually a valid worker, because it's missing // a fetch handler. Regardless, our heuristic is simply to check for exports. const guess = await guessWorkerFormat( - { - file: path.join(process.cwd(), "./index.ts"), - directory: process.cwd(), - }, + path.join(process.cwd(), "./index.ts"), + process.cwd(), undefined ); expect(guess).toBe("modules"); @@ -25,10 +23,8 @@ describe("guess worker format", () => { // a fetch listener. Regardless, our heuristic is simply to check for // the lack of exports. const guess = await guessWorkerFormat( - { - file: path.join(process.cwd(), "./index.ts"), - directory: process.cwd(), - }, + path.join(process.cwd(), "./index.ts"), + process.cwd(), undefined ); expect(guess).toBe("service-worker"); @@ -38,10 +34,8 @@ describe("guess worker format", () => { await writeFile("./index.ts", "export default {};"); await expect( guessWorkerFormat( - { - file: path.join(process.cwd(), "./index.ts"), - directory: process.cwd(), - }, + path.join(process.cwd(), "./index.ts"), + process.cwd(), "service-worker" ) ).rejects.toThrow( @@ -53,10 +47,8 @@ describe("guess worker format", () => { await writeFile("./index.ts", ""); await expect( guessWorkerFormat( - { - file: path.join(process.cwd(), "./index.ts"), - directory: process.cwd(), - }, + path.join(process.cwd(), "./index.ts"), + process.cwd(), "modules" ) ).rejects.toThrow( diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts index 64936abc84ba..a645a053a71e 100644 --- a/packages/wrangler/src/__tests__/publish.test.ts +++ b/packages/wrangler/src/__tests__/publish.test.ts @@ -1388,6 +1388,30 @@ export default{ expect(std.warn).toMatchInlineSnapshot(`""`); }); } + + it("should throw an error if the entry doesn't exist after the build finishes", async () => { + writeWranglerToml({ + main: "index.js", + build: { + command: `node -e "console.log('custom build');"`, + }, + }); + + await expect( + runWrangler("publish index.js") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""` + ); + expect(std.out).toMatchInlineSnapshot( + `"running: node -e \\"console.log('custom build');\\""` + ); + expect(std.err).toMatchInlineSnapshot(` + "Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\" + + %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new." + `); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); }); describe("[wasm_modules]", () => { diff --git a/packages/wrangler/src/bundle.ts b/packages/wrangler/src/bundle.ts index f71c8ef4d2a1..83a32a45c5bb 100644 --- a/packages/wrangler/src/bundle.ts +++ b/packages/wrangler/src/bundle.ts @@ -5,7 +5,7 @@ import * as esbuild from "esbuild"; import makeModuleCollector from "./module-collection"; import type { Config } from "./config"; import type { Entry } from "./entry"; -import type { CfModule, CfScriptFormat } from "./worker"; +import type { CfModule } from "./worker"; type BundleResult = { modules: CfModule[]; @@ -24,20 +24,13 @@ export async function bundleWorker( serveAssetsFromWorker: boolean; jsxFactory: string | undefined; jsxFragment: string | undefined; - format: CfScriptFormat; rules: Config["rules"]; watch?: esbuild.WatchMode; } ): Promise { - const { - serveAssetsFromWorker, - jsxFactory, - jsxFragment, - format, - rules, - watch, - } = options; - const moduleCollector = makeModuleCollector({ format, rules }); + const { serveAssetsFromWorker, jsxFactory, jsxFragment, rules, watch } = + options; + const moduleCollector = makeModuleCollector({ format: entry.format, rules }); const result = await esbuild.build({ ...getEntryPoint(entry.file, serveAssetsFromWorker), bundle: true, diff --git a/packages/wrangler/src/config/config.ts b/packages/wrangler/src/config/config.ts index f76c6fc54305..87c87ea317aa 100644 --- a/packages/wrangler/src/config/config.ts +++ b/packages/wrangler/src/config/config.ts @@ -209,7 +209,10 @@ export interface ConfigFields { cwd?: string; /** The directory to watch for changes while using wrangler dev, defaults to the current working directory */ watch_dir?: string; - /** Deprecated field previously used to configure the build and upload of the script. */ + /** + * Deprecated field previously used to configure the build and upload of the script. + * @deprecated + */ upload?: DeprecatedUpload; }; } diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index ac10b7565d42..7748bc236ec0 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -1,12 +1,11 @@ import assert from "node:assert"; import { spawn } from "node:child_process"; -import { existsSync, realpathSync } from "node:fs"; +import { realpathSync } from "node:fs"; import { readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import { watch } from "chokidar"; import clipboardy from "clipboardy"; import commandExists from "command-exists"; -import { execaCommand } from "execa"; import { Box, Text, useApp, useInput } from "ink"; import React, { useState, useEffect, useRef } from "react"; import { withErrorBoundary, useErrorHandler } from "react-error-boundary"; @@ -15,7 +14,7 @@ import tmp from "tmp-promise"; import { fetch } from "undici"; import { bundleWorker } from "./bundle"; import { createWorkerPreview } from "./create-worker-preview"; -import guessWorkerFormat from "./guess-worker-format"; +import { runCustomBuild } from "./entry"; import useInspector from "./inspect"; import { DEFAULT_MODULE_RULES } from "./module-collection"; import openInBrowser from "./open-in-browser"; @@ -28,7 +27,6 @@ import type { Entry } from "./entry"; import type { AssetPaths } from "./sites"; import type { CfModule, CfWorkerInit, CfScriptFormat } from "./worker"; import type { WatchMode } from "esbuild"; -import type { ExecaChildProcess } from "execa"; import type { DirectoryResult } from "tmp-promise"; export type DevProps = { @@ -36,7 +34,6 @@ export type DevProps = { entry: Entry; port?: number; inspectorPort: number; - format: CfScriptFormat | undefined; rules: Config["rules"]; accountId: undefined | string; initialMode: "local" | "remote"; @@ -69,33 +66,28 @@ function Dev(props: DevProps): JSX.Element { const apiToken = props.initialMode === "remote" ? getAPIToken() : undefined; const directory = useTmpDir(); - // if there isn't a build command, we just return the entry immediately - // ideally there would be a conditional here, but the rules of hooks - // kinda forbid that, so we thread the entry through useCustomBuild - const entry = useCustomBuild(props.entry, props.buildCommand); + useCustomBuild(props.entry, props.buildCommand); - const format = useWorkerFormat({ entry: props.entry, format: props.format }); - if (format && props.public && format === "service-worker") { + if (props.public && props.entry.format === "service-worker") { throw new Error( "You cannot use the service worker format with a `public` directory." ); } - if (props.bindings.wasm_modules && format === "modules") { + if (props.bindings.wasm_modules && props.entry.format === "modules") { throw new Error( "You cannot configure [wasm_modules] with an ES module worker. Instead, import the .wasm module directly in your code" ); } - if (props.bindings.text_blobs && format === "modules") { + if (props.bindings.text_blobs && props.entry.format === "modules") { throw new Error( "You cannot configure [text_blobs] with an ES module worker. Instead, import the file directly in your code, and optionally configure `[build.upload.rules]` in your wrangler.toml" ); } const bundle = useEsbuild({ - entry, - format, + entry: props.entry, destination: directory, staticRoot: props.public, jsxFactory: props.jsxFactory, @@ -121,7 +113,7 @@ function Dev(props: DevProps): JSX.Element { (); - useEffect(() => { - async function validateFormat() { - if (!props.entry || format) { - return; - } - setFormat(await guessWorkerFormat(props.entry, props.format)); - } - validateFormat().catch((err) => { - console.error("Failed to validate worker format:", err); - }); - }, [props.entry, props.format, format]); - return format; -} - function Remote(props: { name: string | undefined; bundle: EsbuildBundle | undefined; @@ -500,85 +473,32 @@ function useTmpDir(): string | undefined { function useCustomBuild( expectedEntry: Entry, - props: { + build: { command?: undefined | string; cwd?: undefined | string; watch_dir?: undefined | string; } -): undefined | Entry { - const [entry, setEntry] = useState( - // if there's no build command, just return the expected entry - !props.command ? expectedEntry : undefined - ); - const { command, cwd, watch_dir } = props; +): void { useEffect(() => { - if (!command) return; - let cmd: ExecaChildProcess | undefined, - interval: NodeJS.Timeout | undefined; - console.log("running:", command); - cmd = execaCommand(command, { - ...(cwd && { cwd }), - shell: true, - stderr: "inherit", - stdout: "inherit", - }); - if (watch_dir) { - watch(watch_dir, { persistent: true, ignoreInitial: true }).on( - "all", - (_event, filePath) => { - console.log(`The file ${filePath} changed, restarting build...`); - cmd?.kill(); - cmd = execaCommand(command, { - ...(cwd && { cwd }), - shell: true, - stderr: "inherit", - stdout: "inherit", - }); - } - ); + if (!build.command) return; + let watcher: ReturnType | undefined; + if (build.watch_dir) { + watcher = watch(build.watch_dir, { + persistent: true, + ignoreInitial: true, + }).on("all", (_event, filePath) => { + //TODO: we should buffer requests to the proxy until this completes + console.log(`The file ${filePath} changed, restarting build...`); + runCustomBuild(expectedEntry.file, build).catch((err) => { + console.error("Custom build failed:", err); + }); + }); } - // check every so often whether `expectedEntry` exists - // if it does, we're done - const startedAt = Date.now(); - interval = setInterval(() => { - let fileExists = false; - try { - // Use require.resolve to use node's resolution algorithm, - // this lets us use paths without explicit .js extension - // TODO: we should probably remove this, because it doesn't - // take into consideration other extensions like .tsx, .ts, .jsx, etc - fileExists = existsSync(require.resolve(expectedEntry.file)); - } catch (e) { - // fail silently, usually means require.resolve threw MODULE_NOT_FOUND - } - - if (fileExists === true) { - interval && clearInterval(interval); - setEntry(expectedEntry); - } else { - const elapsed = Date.now() - startedAt; - // timeout after 30 seconds of waiting - if (elapsed > 1000 * 30) { - console.error( - `⎔ Build timed out, Could not resolve ${expectedEntry.file}` - ); - interval && clearInterval(interval); - cmd?.kill(); - } - } - }, 200); - return () => { - if (cmd) { - cmd.kill(); - cmd = undefined; - } - interval && clearInterval(interval); - interval = undefined; + watcher?.close(); }; - }, [command, cwd, expectedEntry, watch_dir]); - return entry; + }, [build, expectedEntry.file]); } type EsbuildBundle = { @@ -596,13 +516,11 @@ function useEsbuild({ staticRoot, jsxFactory, jsxFragment, - format, rules, serveAssetsFromWorker, }: { - entry: undefined | Entry; + entry: Entry; destination: string | undefined; - format: CfScriptFormat | undefined; staticRoot: undefined | string; jsxFactory: string | undefined; jsxFragment: string | undefined; @@ -631,7 +549,7 @@ function useEsbuild({ }; async function build() { - if (!destination || !entry || !format) return; + if (!destination) return; const { resolvedEntryPointPath, bundleType, modules, stop } = await bundleWorker(entry, destination, { @@ -639,7 +557,6 @@ function useEsbuild({ serveAssetsFromWorker: false, jsxFactory, jsxFragment, - format, rules, watch: watchMode, }); @@ -669,7 +586,6 @@ function useEsbuild({ staticRoot, jsxFactory, jsxFragment, - format, serveAssetsFromWorker, rules, ]); diff --git a/packages/wrangler/src/entry.ts b/packages/wrangler/src/entry.ts index 079eba7c0042..565e7e4446b1 100644 --- a/packages/wrangler/src/entry.ts +++ b/packages/wrangler/src/entry.ts @@ -1,27 +1,32 @@ +import assert from "node:assert"; import { existsSync } from "node:fs"; import path from "node:path"; +import * as esbuild from "esbuild"; +import { execaCommand } from "execa"; import type { Config } from "./config"; +import type { CfScriptFormat } from "./worker"; +import type { Metafile } from "esbuild"; /** * An entry point for the Worker. * * It consists not just of a `file`, but also of a `directory` that is used to resolve relative paths. */ -export type Entry = { file: string; directory: string }; +export type Entry = { file: string; directory: string; format: CfScriptFormat }; /** * Compute the entry-point for the Worker. */ -export function getEntry( +export async function getEntry( + args: { script?: string; format?: CfScriptFormat | undefined }, config: Config, - command: string, - script?: string -): Entry { + command: string +): Promise { let file: string; let directory = process.cwd(); - if (script) { + if (args.script) { // If the script name comes from the command line it is relative to the current working directory. - file = path.resolve(script); + file = path.resolve(args.script); } else if (config.main === undefined) { throw new Error( `Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler ${command} path/to/script\`) or the \`main\` config field.` @@ -31,14 +36,111 @@ export function getEntry( file = path.resolve(directory, config.main); } - if (!config.build?.command && fileExists(file) === false) { + await runCustomBuild(file, config.build); + + if (fileExists(file) === false) { throw new Error( `Could not resolve "${path.relative(process.cwd(), file)}".` ); } - return { file, directory }; + const format = await guessWorkerFormat( + file, + directory, + args.format ?? config.build?.upload?.format + ); + return { file, directory, format }; +} + +export async function runCustomBuild( + expectedEntry: string, + build: Config["build"] +) { + if (build?.command) { + // TODO: add a deprecation message here? + console.log("running:", build.command); + await execaCommand(build.command, { + shell: true, + stdout: "inherit", + stderr: "inherit", + timeout: 1000 * 30, + ...(build.cwd && { cwd: build.cwd }), + }); + + if (fileExists(expectedEntry) === false) { + throw new Error( + `Could not resolve "${path.relative( + process.cwd(), + expectedEntry + )}" after running custom build: ${build.command}` + ); + } + } +} + +/** + * A function to "guess" the type of worker. + * We do this by running a lightweight build of the actual script, + * and looking at the metafile generated by esbuild. If it has a default + * export (or really, any exports), that means it's a "modules" worker. + * Else, it's a "service-worker" worker. This seems hacky, but works remarkably + * well in practice. + */ +export default async function guessWorkerFormat( + entryFile: string, + entryWorkingDirectory: string, + hint: CfScriptFormat | undefined +): Promise { + const result = await esbuild.build({ + entryPoints: [entryFile], + absWorkingDir: entryWorkingDirectory, + metafile: true, + bundle: false, + format: "esm", + write: false, + }); + // 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 entryPoints = Object.entries(metafile.outputs).filter( + ([_path, output]) => output.entryPoint !== undefined + ); + assert( + entryPoints.length > 0, + `Cannot find entry-point "${entryFile}" in generated bundle.` + + listEntryPoints(entryPoints) + ); + assert( + entryPoints.length < 2, + "More than one entry-point found for generated bundle." + + listEntryPoints(entryPoints) + ); + const guessedWorkerFormat = + entryPoints[0][1].exports.length > 0 ? "modules" : "service-worker"; + + if (hint) { + if (hint !== guessedWorkerFormat) { + if (hint === "service-worker") { + throw new Error( + "You configured this worker to be a 'service-worker', but the file you are trying to build appears to have ES module exports. Please pass `--format modules`, or simply remove the configuration." + ); + } else { + throw new Error( + "You configured this worker to be 'modules', but the file you are trying to build doesn't export a handler. Please pass `--format service-worker`, or simply remove the configuration." + ); + } + } + } + return guessedWorkerFormat; +} + +function listEntryPoints( + outputs: [string, ValueOf][] +): string { + return outputs.map(([_input, output]) => output.entryPoint).join("\n"); } +type ValueOf = T[keyof T]; + /** * Returns true if the given `filePath` exists as-is, * or if some version of it (by appending a common extension) exists. diff --git a/packages/wrangler/src/guess-worker-format.ts b/packages/wrangler/src/guess-worker-format.ts deleted file mode 100644 index b71075572665..000000000000 --- a/packages/wrangler/src/guess-worker-format.ts +++ /dev/null @@ -1,68 +0,0 @@ -import assert from "node:assert"; -import { build } from "esbuild"; -import type { Entry } from "./entry"; -import type { CfScriptFormat } from "./worker"; -import type { Metafile } from "esbuild"; - -/** - * A function to "guess" the type of worker. - * We do this by running a lightweight build of the actual script, - * and looking at the metafile generated by esbuild. If it has a default - * export (or really, any exports), that means it's a "modules" worker. - * Else, it's a "service-worker" worker. This seems hacky, but works remarkably - * well in practice. - */ -export default async function guessWorkerFormat( - entry: Entry, - hint: CfScriptFormat | undefined -): Promise { - const result = await build({ - entryPoints: [entry.file], - absWorkingDir: entry.directory, - metafile: true, - bundle: false, - format: "esm", - write: false, - }); - // 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 entryPoints = Object.entries(metafile.outputs).filter( - ([_path, output]) => output.entryPoint !== undefined - ); - assert( - entryPoints.length > 0, - `Cannot find entry-point "${entry.file}" in generated bundle.` + - listEntryPoints(entryPoints) - ); - assert( - entryPoints.length < 2, - "More than one entry-point found for generated bundle." + - listEntryPoints(entryPoints) - ); - const guessedWorkerFormat = - entryPoints[0][1].exports.length > 0 ? "modules" : "service-worker"; - - if (hint) { - if (hint !== guessedWorkerFormat) { - if (hint === "service-worker") { - throw new Error( - "You configured this worker to be a 'service-worker', but the file you are trying to build appears to have ES module exports. Please pass `--format modules`, or simply remove the configuration." - ); - } else { - throw new Error( - "You configured this worker to be 'modules', but the file you are trying to build doesn't export a handler. Please pass `--format service-worker`, or simply remove the configuration." - ); - } - } - } - return guessedWorkerFormat; -} - -function listEntryPoints( - outputs: [string, ValueOf][] -): string { - return outputs.map(([_input, output]) => output.entryPoint).join("\n"); -} - -type ValueOf = T[keyof T]; diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index 34f77af0f128..b8a95dc551d4 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -102,6 +102,9 @@ function getScriptName( : shortScriptName; } +/** + * Ensure that a user is logged in, and a valid account_id is available. + */ async function requireAuth( config: Config, isInteractive = true @@ -771,7 +774,7 @@ export async function main(argv: string[]): Promise { (args.config as ConfigPath) || (args.script && findWranglerToml(path.dirname(args.script))) ); - const entry = getEntry(config, "dev", args.script); + const entry = await getEntry(args, config, "dev"); if (args["experimental-public"]) { console.warn( @@ -874,7 +877,6 @@ export async function main(argv: string[]): Promise { rules={getRules(config)} legacyEnv={isLegacyEnv(args, config)} buildCommand={config.build || {}} - format={args.format || config.build?.upload?.format} initialMode={args.local ? "local" : "remote"} jsxFactory={args["jsx-factory"] || envRootObj?.jsx_factory} jsxFragment={args["jsx-fragment"] || envRootObj?.jsx_fragment} @@ -1031,7 +1033,7 @@ export async function main(argv: string[]): Promise { (args.config as ConfigPath) || (args.script && findWranglerToml(path.dirname(args.script))) ); - const entry = getEntry(config, "publish", args.script); + const entry = await getEntry(args, config, "publish"); if (args.latest) { console.warn( @@ -1052,7 +1054,6 @@ export async function main(argv: string[]): Promise { accountId, name: getScriptName(args, config), rules: getRules(config), - format: args.format || config.build?.upload?.format, entry, env: args.env, compatibilityDate: args.latest @@ -1243,7 +1244,7 @@ export async function main(argv: string[]): Promise { ); const config = readConfig(args.config as ConfigPath); - const entry = getEntry(config, "dev"); + const entry = await getEntry({}, config, "dev"); const accountId = await requireAuth(config); @@ -1259,7 +1260,6 @@ export async function main(argv: string[]): Promise { zone={undefined} legacyEnv={isLegacyEnv(args, config)} buildCommand={config.build || {}} - format={config.build?.upload?.format} initialMode={args.local ? "local" : "remote"} jsxFactory={envRootObj?.jsx_factory} jsxFragment={envRootObj?.jsx_fragment} diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index f798db65bcab..1589894af6ba 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -2,23 +2,20 @@ import assert from "node:assert"; import { readFileSync } from "node:fs"; import path from "node:path"; import { URLSearchParams } from "node:url"; -import { execaCommand } from "execa"; import tmp from "tmp-promise"; import { bundleWorker } from "./bundle"; import { fetchResult } from "./cfetch"; import { createWorkerUploadForm } from "./create-worker-upload-form"; -import { fileExists } from "./entry"; -import guessWorkerFormat from "./guess-worker-format"; import { syncAssets } from "./sites"; import type { Config } from "./config"; +import type { Entry } from "./entry"; import type { AssetPaths } from "./sites"; -import type { CfScriptFormat, CfWorkerInit } from "./worker"; +import type { CfWorkerInit } from "./worker"; type Props = { config: Config; accountId: string; - format: CfScriptFormat | undefined; - entry: { file: string; directory: string }; + entry: Entry; rules: Config["rules"]; name: string | undefined; env: string | undefined; @@ -84,28 +81,7 @@ export default async function publish(props: Props): Promise { try { const envName = props.env ?? "production"; - if (config.build.command) { - // TODO: add a deprecation message here? - console.log("running:", config.build.command); - await execaCommand(config.build.command, { - shell: true, - stdout: "inherit", - stderr: "inherit", - timeout: 1000 * 30, - ...(config.build.cwd && { cwd: config.build.cwd }), - }); - - if (fileExists(props.entry.file) === false) { - throw new Error( - `Could not resolve "${path.relative( - process.cwd(), - props.entry.file - )}".` - ); - } - } - - const format = await guessWorkerFormat(props.entry, props.format); + const { format } = props.entry; if (props.experimentalPublic && format === "service-worker") { // TODO: check config too @@ -133,7 +109,6 @@ export default async function publish(props: Props): Promise { serveAssetsFromWorker: props.experimentalPublic, jsxFactory, jsxFragment, - format, rules: props.rules, } );