Skip to content

Commit

Permalink
fix: error when entry doesn't exist
Browse files Browse the repository at this point in the history
This adds an error when we use an entry point that doesn't exist, either for `wrangler dev` or `wrangler publish`, and either via cli arg or `build.upload.main` in `wrangler.toml`.  By using a common abstraction for `dev` and `publish`, This also adds support for using `build.config.main`/`build.config.dir` for `wrangler dev`.

Fixes #418
Fixes #390
  • Loading branch information
threepointone committed Feb 12, 2022
1 parent 8f47712 commit c234ed2
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 146 deletions.
26 changes: 25 additions & 1 deletion packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ import { render } from "ink-testing-library";
import patchConsole from "patch-console";
import React from "react";
import Dev from "../dev";
import { mockConsoleMethods } from "./helpers/mock-console";
import { runWrangler } from "./helpers/run-wrangler";
import writeWranglerToml from "./helpers/write-wrangler-toml";
import type { DevProps } from "../dev";

describe("Dev component", () => {
let restoreConsole: ReturnType<typeof patchConsole>;
beforeEach(() => (restoreConsole = patchConsole(() => {})));
afterEach(() => restoreConsole());
const std = mockConsoleMethods();

// This test needs to be rewritten because the error now throws asynchronously
// and the Ink framework does not yet have async testing support.
Expand All @@ -23,6 +27,26 @@ describe("Dev component", () => {
Error: You cannot use the service worker format with a \`public\` directory."
`);
});

describe("entry-points", () => {
it("should error if there is no entry-point specified", async () => {
writeWranglerToml();

await expect(
runWrangler("dev")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`build.upload.main\` config field."`
);

expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`
"Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`build.upload.main\` config field.
%s
If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
`);
});
});
});

/**
Expand All @@ -32,7 +56,7 @@ describe("Dev component", () => {
*/
function renderDev({
name,
entry = "some/entry.ts",
entry = { file: "some/entry.ts", directory: process.cwd() },
port,
format,
accountId,
Expand Down
20 changes: 20 additions & 0 deletions packages/wrangler/src/__tests__/helpers/write-wrangler-toml.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import * as fs from "fs";
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
fs.writeFileSync(
"./wrangler.toml",
TOML.stringify({
compatibility_date: "2022-01-12",
name: "test-name",
...(config as TOML.JsonMap),
}),

"utf-8"
);
}
4 changes: 2 additions & 2 deletions packages/wrangler/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("wrangler", () => {
Commands:
wrangler init [name] 📥 Create a wrangler.toml configuration file
wrangler whoami 🕵️ Retrieve your user info and test your auth config
wrangler dev <filename> 👂 Start a local server for developing your worker
wrangler dev [script] 👂 Start a local server for developing your worker
wrangler publish [script] 🆙 Publish your Worker to Cloudflare.
wrangler tail [name] 🦚 Starts a log tailing session for a deployed Worker.
wrangler secret 🤫 Generate a secret that can be referenced in the worker script
Expand Down Expand Up @@ -71,7 +71,7 @@ describe("wrangler", () => {
Commands:
wrangler init [name] 📥 Create a wrangler.toml configuration file
wrangler whoami 🕵️ Retrieve your user info and test your auth config
wrangler dev <filename> 👂 Start a local server for developing your worker
wrangler dev [script] 👂 Start a local server for developing your worker
wrangler publish [script] 🆙 Publish your Worker to Cloudflare.
wrangler tail [name] 🦚 Starts a log tailing session for a deployed Worker.
wrangler secret 🤫 Generate a secret that can be referenced in the worker script
Expand Down
19 changes: 1 addition & 18 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { mockConsoleMethods } from "./helpers/mock-console";
import { mockKeyListRequest } from "./helpers/mock-kv";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import writeWranglerToml from "./helpers/write-wrangler-toml";
import type { WorkerMetadata } from "../api/form_data";
import type { Config } from "../config";
import type { KVNamespaceInfo } from "../kv";
import type { FormData, File } from "undici";

Expand Down Expand Up @@ -1373,23 +1373,6 @@ export default{
});
});

/** Write a mock wrangler.toml file to disk. */
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
fs.writeFileSync(
"./wrangler.toml",
TOML.stringify({
compatibility_date: "2022-01-12",
name: "test-name",
...(config as TOML.JsonMap),
}),

"utf-8"
);
}

/** Write a mock Worker script to disk. */
function writeWorkerSource({
basePath = ".",
Expand Down
16 changes: 10 additions & 6 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import * as esbuild from "esbuild";
import makeModuleCollector from "./module-collection";
import type { CfModule, CfScriptFormat } from "./api/worker";

export type Entry = { file: string; directory: string };

type BundleResult = {
modules: CfModule[];
resolvedEntryPointPath: string;
Expand All @@ -16,9 +18,8 @@ type BundleResult = {
* Generate a bundle for the worker identified by the arguments passed in.
*/
export async function bundleWorker(
entryFile: string,
entry: Entry,
serveAssetsFromWorker: boolean,
workingDir: string,
destination: string,
jsxFactory: string | undefined,
jsxFragment: string | undefined,
Expand All @@ -27,9 +28,9 @@ export async function bundleWorker(
): Promise<BundleResult> {
const moduleCollector = makeModuleCollector({ format });
const result = await esbuild.build({
...getEntryPoint(entryFile, serveAssetsFromWorker),
...getEntryPoint(entry.file, serveAssetsFromWorker),
bundle: true,
absWorkingDir: workingDir,
absWorkingDir: entry.directory,
outdir: destination,
external: ["__STATIC_CONTENT_MANIFEST"],
format: "esm",
Expand All @@ -53,7 +54,7 @@ export async function bundleWorker(
);
assert(
entryPointOutputs.length > 0,
`Cannot find entry-point "${entryFile}" in generated bundle.` +
`Cannot find entry-point "${entry.file}" in generated bundle.` +
listEntryPoints(entryPointOutputs)
);
assert(
Expand All @@ -67,7 +68,10 @@ export async function bundleWorker(

return {
modules: moduleCollector.modules,
resolvedEntryPointPath: path.resolve(workingDir, entryPointOutputs[0][0]),
resolvedEntryPointPath: path.resolve(
entry.directory,
entryPointOutputs[0][0]
),
bundleType,
stop: result.stop,
};
Expand Down
117 changes: 48 additions & 69 deletions packages/wrangler/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,75 +410,54 @@ export type Config = {
* in wrangler2. We infer the format automatically, and we can pass
* the path to the script either in the CLI (or, @todo, as the top level
* `entry` property).
*/ (
| {
upload?: {
/**
* The format of the Worker script, must be "service-worker".
*
* @deprecated We infer the format automatically now.
*/
format?: "service-worker";

/**
* The path to the Worker script. This should be replaced
* by the top level `entry' property.
*
* @deprecated This will be replaced by the top level `entry' property.
*/
main: string;
};
}
| {
/**
* When we use the module format, we only really
* need to specify the entry point. The format is deduced
* automatically in wrangler2.
*/
upload?: {
/**
* The format of the Worker script, must be "modules".
*
* @deprecated We infer the format automatically now.
*/
format?: "modules";

/**
* The directory you wish to upload your modules from,
* relative to the wrangler.toml file.
*
* Defaults to the directory containing the wrangler.toml file.
*
* @deprecated
* @breaking
*/
dir?: string;

/**
* The path to the Worker script, relative to `upload.dir`.
*
* @deprecated This will be replaced by a command line argument.
*/
main?: string;

/**
* An ordered list of rules that define which modules to import,
* and what type to import them as. You will need to specify rules
* to use Text, Data, and CompiledWasm modules, or when you wish to
* have a .js file be treated as an ESModule instead of CommonJS.
*
* @deprecated These are now inferred automatically for major file types, but you can still specify them manually.
* @todo this needs to be implemented!
* @breaking
*/
rules?: {
type: "ESModule" | "CommonJS" | "Text" | "Data" | "CompiledWasm";
globs: string[];
fallthrough?: boolean;
};
};
}
);
*/ {
/**
* We only really need to specify the entry point.
* The format is deduced automatically in wrangler2.
*/
upload?: {
/**
* The format of the Worker script.
*
* @deprecated We infer the format automatically now.
*/
format?: "modules" | "service-worker";

/**
* The directory you wish to upload your worker from,
* relative to the wrangler.toml file.
*
* Defaults to the directory containing the wrangler.toml file.
*
* @deprecated
* @breaking
*/
dir?: string;

/**
* The path to the Worker script, relative to `upload.dir`.
*
* @deprecated This will be replaced by a command line argument.
*/
main?: string;

/**
* An ordered list of rules that define which modules to import,
* and what type to import them as. You will need to specify rules
* to use Text, Data, and CompiledWasm modules, or when you wish to
* have a .js file be treated as an ESModule instead of CommonJS.
*
* @deprecated These are now inferred automatically for major file types, but you can still specify them manually.
* @todo this needs to be implemented!
* @breaking
*/
rules?: {
type: "ESModule" | "CommonJS" | "Text" | "Data" | "CompiledWasm";
globs: string[];
fallthrough?: boolean;
};
};
};

/**
* The `env` section defines overrides for the configuration for
Expand Down
26 changes: 14 additions & 12 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ import { syncAssets } from "./sites";
import { getAPIToken } from "./user";
import type { CfPreviewToken } from "./api/preview";
import type { CfModule, CfWorkerInit, CfScriptFormat } from "./api/worker";
import type { Entry } from "./bundle";
import type { AssetPaths } from "./sites";
import type { WatchMode } from "esbuild";
import type { DirectoryResult } from "tmp-promise";

export type DevProps = {
name?: string;
entry: string;
entry: Entry;
port?: number;
format: CfScriptFormat | undefined;
accountId: undefined | string;
Expand Down Expand Up @@ -61,7 +62,7 @@ function Dev(props: DevProps): JSX.Element {
// kinda forbid that, so we thread the entry through useCustomBuild
const entry = useCustomBuild(props.entry, props.buildCommand);

const format = useWorkerFormat({ file: entry, format: props.format });
const format = useWorkerFormat({ file: entry?.file, format: props.format });
if (format && props.public && format === "service-worker") {
throw new Error(
"You cannot use the service worker format with a `public` directory."
Expand Down Expand Up @@ -439,16 +440,16 @@ function useTmpDir(): string | undefined {
}

function useCustomBuild(
expectedEntry: string,
expectedEntry: Entry,
props: {
command?: undefined | string;
cwd?: undefined | string;
watch_dir?: undefined | string;
}
): undefined | string {
const [entry, setEntry] = useState<string | undefined>(
): undefined | Entry {
const [entry, setEntry] = useState<Entry | undefined>(
// if there's no build command, just return the expected entry
props.command || expectedEntry
!props.command ? expectedEntry : undefined
);
const { command, cwd, watch_dir } = props;
useEffect(() => {
Expand Down Expand Up @@ -481,14 +482,16 @@ function useCustomBuild(
// if it does, we're done
const startedAt = Date.now();
interval = setInterval(() => {
if (existsSync(expectedEntry)) {
if (existsSync(expectedEntry.file)) {
clearInterval(interval);
setEntry(expectedEntry);
} else {
const elapsed = Date.now() - startedAt;
// timeout after 30 seconds of waiting
if (elapsed > 1000 * 60 * 30) {
console.error("⎔ Build timed out.");
if (elapsed > 1000 * 30) {
console.error(
`⎔ Build timed out, could not resolve ${expectedEntry}`
);
clearInterval(interval);
cmd.kill();
}
Expand All @@ -511,7 +514,7 @@ function useCustomBuild(
type EsbuildBundle = {
id: number;
path: string;
entry: string;
entry: Entry;
type: "esm" | "commonjs";
modules: CfModule[];
serveAssetsFromWorker: boolean;
Expand All @@ -526,7 +529,7 @@ function useEsbuild({
format,
serveAssetsFromWorker,
}: {
entry: undefined | string;
entry: undefined | Entry;
destination: string | undefined;
format: CfScriptFormat | undefined;
staticRoot: undefined | string;
Expand Down Expand Up @@ -563,7 +566,6 @@ function useEsbuild({
entry,
// In dev, we server assets from the local proxy before we send the request to the worker.
/* serveAssetsFromWorker */ false,
process.cwd(),
destination,
jsxFactory,
jsxFragment,
Expand Down
Loading

0 comments on commit c234ed2

Please sign in to comment.