fix(deploy): ensure import map or config file is included in the mani…

…fest (#326)

This commit adds a check before uploading assets if an import map specified in
`--import-map` option or a config file (e.g. `deno.json`) is included in the
manifest. This will help users understand why import maps don't get applied
during deployment.

### case 1: import map not included

If the specified import map is not included in the manifest due to `--include`
and/or `--exclude` settings, this is a config conflict and most likely a wrong
setup. So in this case, deployctl will error out with a specific error message:

![import_map.json not included in the manifest](https://github.com/user-attachments/assets/bde459b0-6bea-4788-8a89-f015861d5075)

### case 2: deno.json not included

In this case, we can't necessarily say that this is a wrong setup, because
deployctl usually infers the config file location and the config file may not
have `imports` property. So instead of immediately erroring out, it shows a
warning message that tells users that any import map settings in the config file
won't be used:

![deno.json not included in the manifest](https://github.com/user-attachments/assets/96b01be7-5452-4292-badd-62609777c799)

Closes #324

.github/workflows/ci.yml

@@ -18,6 +18,12 @@ jobs:
         os: [macOS-latest, windows-latest, ubuntu-latest]
 
     steps:
+      # Some test cases are sensitive to line endings. Disable autocrlf on
+      # Windows to ensure consistent behavior.
+      - name: Disable autocrlf
+        if: runner.os == 'Windows'
+        run: git config --global core.autocrlf false
+
       - name: Setup repo
         uses: actions/checkout@v3
 

action/deps.js

@@ -3490,7 +3490,15 @@ function include(path, include, exclude) {
     }
     return true;
 }
-async function walk(cwd, dir, files, options) {
+async function walk(cwd, dir, options) {
+    const hashPathMap = new Map();
+    const manifestEntries = await walkInner(cwd, dir, hashPathMap, options);
+    return {
+        manifestEntries,
+        hashPathMap
+    };
+}
+async function walkInner(cwd, dir, hashPathMap, options) {
     const entries = {};
     for await (const file of Deno.readDir(dir)){
         const path = join2(dir, file.name);
@@ -3507,12 +3515,12 @@ async function walk(cwd, dir, files, options) {
                 gitSha1,
                 size: data.byteLength
             };
-            files.set(gitSha1, path);
+            hashPathMap.set(gitSha1, path);
         } else if (file.isDirectory) {
             if (relative === "/.git") continue;
             entry = {
                 kind: "directory",
-                entries: await walk(cwd, path, files, options)
+                entries: await walkInner(cwd, path, hashPathMap, options)
             };
         } else if (file.isSymlink) {
             const target = await Deno.readLink(path);

action/index.js

@@ -80,11 +80,14 @@ async function main() {
   if (!includes.some((i) => i.includes("node_modules"))) {
     excludes.push("**/node_modules");
   }
-  const assets = new Map();
-  const entries = await walk(cwd, cwd, assets, {
-    include: includes.map(convertPatternToRegExp),
-    exclude: excludes.map(convertPatternToRegExp),
-  });
+  const { manifestEntries: entries, hashPathMap: assets } = await walk(
+    cwd,
+    cwd,
+    {
+      include: includes.map(convertPatternToRegExp),
+      exclude: excludes.map(convertPatternToRegExp),
+    },
+  );
   core.debug(`Discovered ${assets.size} assets`);
 
   const api = new API(`GitHubOIDC ${token}`, ORIGIN, {

src/subcommands/deploy.ts

@@ -9,10 +9,16 @@ import { error } from "../error.ts";
 import { API, APIError, endpoint } from "../utils/api.ts";
 import type { ManifestEntry } from "../utils/api_types.ts";
 import { parseEntrypoint } from "../utils/entrypoint.ts";
-import { convertPatternToRegExp, walk } from "../utils/walk.ts";
+import {
+  containsEntryInManifest,
+  convertPatternToRegExp,
+  walk,
+} from "../utils/manifest.ts";
 import TokenProvisioner from "../utils/access_token.ts";
 import type { Args as RawArgs } from "../args.ts";
 import organization from "../utils/organization.ts";
+import { relative } from "@std/path/relative";
+import { yellow } from "@std/fmt/colors";
 
 const help = `deployctl deploy
 Deploy a script with static files to Deno Deploy.
@@ -274,14 +280,46 @@ async function deploy(opts: DeployOpts): Promise<void> {
   if (opts.static) {
     wait("").start().info(`Uploading all files from the current dir (${cwd})`);
     const assetSpinner = wait("Finding static assets...").start();
-    const assets = new Map<string, string>();
     const include = opts.include.map(convertPatternToRegExp);
     const exclude = opts.exclude.map(convertPatternToRegExp);
-    const entries = await walk(cwd, cwd, assets, { include, exclude });
+    const { manifestEntries: entries, hashPathMap: assets } = await walk(
+      cwd,
+      cwd,
+      { include, exclude },
+    );
     assetSpinner.succeed(
       `Found ${assets.size} asset${assets.size === 1 ? "" : "s"}.`,
     );
 
+    // If the import map is specified but not in the manifest, error out.
+    if (
+      opts.importMapUrl !== null &&
+      !containsEntryInManifest(
+        entries,
+        relative(cwd, fromFileUrl(opts.importMapUrl)),
+      )
+    ) {
+      error(
+        `Import map ${opts.importMapUrl} not found in the assets to be uploaded. Please check --include and --exclude options to make sure the import map is included.`,
+      );
+    }
+
+    // If the config file is present but not in the manifest, show a warning
+    // that any import map settings in the config file will not be used.
+    if (
+      opts.importMapUrl === null && opts.config !== null &&
+      !containsEntryInManifest(
+        entries,
+        relative(cwd, opts.config),
+      )
+    ) {
+      wait("").start().warn(
+        yellow(
+          `Config file ${opts.config} not found in the assets to be uploaded; any import map settings in the config file will not be applied during deployment. If this is not your intention, please check --include and --exclude options to make sure the config file is included.`,
+        ),
+      );
+    }
+
     uploadSpinner = wait("Determining assets to upload...").start();
     const neededHashes = await api.projectNegotiateAssets(project.id, {
       entries,

src/utils/walk.ts → src/utils/manifest.ts

@@ -38,7 +38,25 @@ function include(
 export async function walk(
   cwd: string,
   dir: string,
-  files: Map<string, string>,
+  options: { include: RegExp[]; exclude: RegExp[] },
+): Promise<
+  {
+    manifestEntries: Record<string, ManifestEntry>;
+    hashPathMap: Map<string, string>;
+  }
+> {
+  const hashPathMap = new Map<string, string>();
+  const manifestEntries = await walkInner(cwd, dir, hashPathMap, options);
+  return {
+    manifestEntries,
+    hashPathMap,
+  };
+}
+
+async function walkInner(
+  cwd: string,
+  dir: string,
+  hashPathMap: Map<string, string>,
   options: { include: RegExp[]; exclude: RegExp[] },
 ): Promise<Record<string, ManifestEntry>> {
   const entries: Record<string, ManifestEntry> = {};
@@ -65,12 +83,12 @@ export async function walk(
         gitSha1,
         size: data.byteLength,
       };
-      files.set(gitSha1, path);
+      hashPathMap.set(gitSha1, path);
     } else if (file.isDirectory) {
       if (relative === "/.git") continue;
       entry = {
         kind: "directory",
-        entries: await walk(cwd, path, files, options),
+        entries: await walkInner(cwd, path, hashPathMap, options),
       };
     } else if (file.isSymlink) {
       const target = await Deno.readLink(path);
@@ -98,3 +116,42 @@ export function convertPatternToRegExp(pattern: string): RegExp {
     ? new RegExp(globToRegExp(normalize(pattern)).toString().slice(1, -2))
     : new RegExp(`^${normalize(pattern)}`);
 }
+
+/**
+ * Determines if the manifest contains the entry at the given relative path.
+ *
+ * @param manifestEntries manifest entries to search
+ * @param entryRelativePathToLookup a relative path to look up in the manifest
+ * @returns `true` if the manifest contains the entry at the given relative path
+ */
+export function containsEntryInManifest(
+  manifestEntries: Record<string, ManifestEntry>,
+  entryRelativePathToLookup: string,
+): boolean {
+  for (const [entryName, entry] of Object.entries(manifestEntries)) {
+    switch (entry.kind) {
+      case "file":
+      case "symlink": {
+        if (entryName === entryRelativePathToLookup) {
+          return true;
+        }
+        break;
+      }
+      case "directory": {
+        if (!entryRelativePathToLookup.startsWith(entryName)) {
+          break;
+        }
+
+        const relativePath = entryRelativePathToLookup.slice(
+          entryName.length + 1,
+        );
+        return containsEntryInManifest(entry.entries, relativePath);
+      }
+      default: {
+        const _: never = entry;
+      }
+    }
+  }
+
+  return false;
+}