Strict null checks dev and publish commands

.changeset/clever-humans-happen.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+refactor: update dev and publish commands to pass strict-null checks

packages/wrangler/src/dev.tsx

@@ -1,30 +1,32 @@
 import esbuild from "esbuild";
-import { readFile } from "fs/promises";
-import { existsSync } from "fs";
-import type { DirectoryResult } from "tmp-promise";
-import tmp from "tmp-promise";
-import type { CfPreviewToken } from "./api/preview";
+import assert from "node:assert";
+import { spawn } from "node:child_process";
+import { readFile } from "node:fs/promises";
+import { existsSync } from "node:fs";
+import path from "node:path";
 import { Box, Text, useApp, useInput } from "ink";
-import React, { useState, useEffect, useRef } from "react";
-import path from "path";
+import { watch } from "chokidar";
+import clipboardy from "clipboardy";
+import commandExists from "command-exists";
+import { execa } from "execa";
+import fetch from "node-fetch";
 import open from "open";
-import useInspector from "./inspect";
+import React, { useState, useEffect, useRef } from "react";
+import { withErrorBoundary, useErrorHandler } from "react-error-boundary";
+import onExit from "signal-exit";
+import tmp from "tmp-promise";
+import type { DirectoryResult } from "tmp-promise";
+
+import type { CfPreviewToken } from "./api/preview";
 import type { CfModule } from "./api/worker";
 import { createWorker } from "./api/worker";
 import type { CfWorkerInit } from "./api/worker";
-import { spawn } from "child_process";
-import onExit from "signal-exit";
-import { syncAssets } from "./sites";
-import clipboardy from "clipboardy";
-import commandExists from "command-exists";
-import assert from "assert";
-import { getAPIToken } from "./user";
-import fetch from "node-fetch";
+
+import useInspector from "./inspect";
 import makeModuleCollector from "./module-collection";
-import { withErrorBoundary, useErrorHandler } from "react-error-boundary";
 import { usePreviewServer } from "./proxy";
-import { execa } from "execa";
-import { watch } from "chokidar";
+import { syncAssets } from "./sites";
+import { getAPIToken } from "./user";
 
 type CfScriptFormat = undefined | "modules" | "service-worker";
 
@@ -56,7 +58,7 @@ function Dev(props: DevProps): JSX.Element {
       "You cannot use the service worker format with a `public` directory."
     );
   }
-  const port = props.port || 8787;
+  const port = props.port ?? 8787;
   const apiToken = getAPIToken();
   const directory = useTmpDir();
 
@@ -98,7 +100,7 @@ function Dev(props: DevProps): JSX.Element {
           bindings={props.bindings}
           site={props.site}
           public={props.public}
-          port={props.port}
+          port={port}
         />
       ) : (
         <Remote
@@ -110,7 +112,7 @@ function Dev(props: DevProps): JSX.Element {
           bindings={props.bindings}
           site={props.site}
           public={props.public}
-          port={props.port}
+          port={port}
           compatibilityDate={props.compatibilityDate}
           compatibilityFlags={props.compatibilityFlags}
           usageModel={props.usageModel}
@@ -263,11 +265,11 @@ function useLocalWorker(props: {
         }
       });
 
-      local.current.stdout.on("data", (data: Buffer) => {
+      local.current.stdout?.on("data", (data: Buffer) => {
         console.log(`${data.toString()}`);
       });
 
-      local.current.stderr.on("data", (data: Buffer) => {
+      local.current.stderr?.on("data", (data: Buffer) => {
         console.error(`${data.toString()}`);
         const matches =
           /Debugger listening on (ws:\/\/127\.0\.0\.1:9229\/[A-Za-z0-9-]+)/.exec(
@@ -361,7 +363,7 @@ function useCustomBuild(
 ): undefined | string {
   const [entry, setEntry] = useState<string | undefined>(
     // if there's no build command, just return the expected entry
-    props.command ? null : expectedEntry
+    props.command || expectedEntry
   );
   const { command, cwd, watch_dir } = props;
   useEffect(() => {
@@ -465,26 +467,37 @@ function useEsbuild(props: {
             else {
               // nothing really changes here, so let's increment the id
               // to change the return object's identity
-              setBundle((previousBundle) => ({
-                ...previousBundle,
-                id: previousBundle.id + 1,
-              }));
+              setBundle((previousBundle) => {
+                if (previousBundle === undefined) {
+                  assert.fail(
+                    "Rebuild triggered with no previous build available"
+                  );
+                }
+                return { ...previousBundle, id: previousBundle.id + 1 };
+              });
             }
           },
         },
       });
 
-      const chunks = Object.entries(result.metafile.outputs).find(
+      // 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 outputEntry = Object.entries(metafile.outputs).find(
         ([_path, { entryPoint }]) =>
-          entryPoint === Object.keys(result.metafile.inputs)[0]
+          entryPoint === Object.keys(metafile.inputs)[0]
       ); // assumedly only one entry point
-
+      if (outputEntry === undefined) {
+        throw new Error(
+          `Cannot find entry-point "${entry}" in generated bundle.`
+        );
+      }
       setBundle({
         id: 0,
         entry,
-        path: chunks[0],
-        type: chunks[1].exports.length > 0 ? "esm" : "commonjs",
-        exports: chunks[1].exports,
+        path: outputEntry[0],
+        type: outputEntry[1].exports.length > 0 ? "esm" : "commonjs",
+        exports: outputEntry[1].exports,
         modules: moduleCollector.modules,
       });
     }
@@ -495,7 +508,7 @@ function useEsbuild(props: {
       // so this is a no-op error handler
     });
     return () => {
-      result?.stop();
+      result.stop?.();
     };
   }, [entry, destination, staticRoot, jsxFactory, jsxFragment]);
   return bundle;

packages/wrangler/src/publish.ts

@@ -50,13 +50,18 @@ export default async function publish(props: Props): Promise<void> {
     __path__,
   } = config;
 
-  const envRootObj = props.env ? config.env[props.env] || {} : config;
+  const envRootObj =
+    props.env && config.env ? config.env[props.env] || {} : config;
 
   assert(
     envRootObj.compatibility_date || props["compatibility-date"],
     "A compatibility_date is required when publishing. Add one to your wrangler.toml file, or pass it in your terminal as --compatibility_date. See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
   );
 
+  if (accountId === undefined) {
+    throw new Error("No account_id provided.");
+  }
+
   const triggers = props.triggers || config.triggers?.crons;
   const routes = props.routes || config.routes;
 
@@ -129,18 +134,24 @@ export default async function publish(props: Props): Promise<void> {
     ...(jsxFragment && { jsxFragment }),
   });
 
-  const chunks = Object.entries(result.metafile.outputs).find(
-    ([_path, { entryPoint }]) =>
-      entryPoint ===
-      (props.public
-        ? path.join(path.dirname(file), "static-asset-facade.js")
-        : Object.keys(result.metafile.inputs)[0])
+  // 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")
+    : Object.keys(metafile.inputs)[0];
+  const outputEntry = Object.entries(metafile.outputs).find(
+    ([, { entryPoint }]) => entryPoint === expectedEntryPoint
   );
-
+  if (outputEntry === undefined) {
+    throw new Error(
+      `Cannot find entry-point "${expectedEntryPoint}" in generated bundle.`
+    );
+  }
   const { format } = props;
   const bundle = {
-    type: chunks[1].exports.length > 0 ? "esm" : "commonjs",
-    exports: chunks[1].exports,
+    type: outputEntry[1].exports.length > 0 ? "esm" : "commonjs",
+    exports: outputEntry[1].exports,
   };
 
   // TODO: instead of bundling the facade with the worker, we should just bundle the worker and expose it as a module.
@@ -157,13 +168,13 @@ export default async function publish(props: Props): Promise<void> {
     return;
   }
 
-  const content = await readFile(chunks[0], { encoding: "utf-8" });
+  const content = await readFile(outputEntry[0], { encoding: "utf-8" });
   await destination.cleanup();
 
   // if config.migrations
   // get current migration tag
   let migrations;
-  if ("migrations" in config) {
+  if (config.migrations !== undefined) {
     const scripts = await fetchResult<{ id: string; migration_tag: string }[]>(
       `/accounts/${accountId}/workers/scripts`
     );
@@ -199,15 +210,10 @@ export default async function publish(props: Props): Promise<void> {
     }
   }
 
-  const assets =
-    props.public || props.site || props.config.site?.bucket // TODO: allow both
-      ? await syncAssets(
-          accountId,
-          scriptName,
-          props.public || props.site || props.config.site?.bucket,
-          false
-        )
-      : { manifest: undefined, namespace: undefined };
+  const assetPath = props.public || props.site || props.config.site?.bucket; // TODO: allow both
+  const assets = assetPath
+    ? await syncAssets(accountId, scriptName, assetPath, false)
+    : { manifest: undefined, namespace: undefined };
 
   const bindings: CfWorkerInit["bindings"] = {
     kv_namespaces: envRootObj.kv_namespaces?.concat(
@@ -223,7 +229,7 @@ export default async function publish(props: Props): Promise<void> {
   const worker: CfWorkerInit = {
     name: scriptName,
     main: {
-      name: path.basename(chunks[0]),
+      name: path.basename(outputEntry[0]),
       content: content,
       type: bundle.type === "esm" ? "esm" : "commonjs",
     },