[CX-5945] Warn about https file (#24871)

If there is a `https.ts` or `https.js` file at the top level directory of user convex code and the file imports `httpRouter`:
1. Warn user that this file will not be used to set up HTTP actions
2. Log to Sentry

GitOrigin-RevId: 8bb9b523d4510894923daa205aa284d7c2c9dbb5

Author: pashabitz

package.json

@@ -145,6 +145,7 @@
     "prepack": "node scripts/prepack.mjs",
     "postpack": "node scripts/postpack.mjs",
     "test": "NODE_OPTIONS=\"--experimental-vm-modules --no-warnings=ExperimentalWarning\" jest --silent",
+    "test-not-silent": "NODE_OPTIONS=\"--experimental-vm-modules --no-warnings=ExperimentalWarning\" jest",
     "test-esm": "node ./scripts/test-esm.mjs && ./scripts/checkdeps.mjs",
     "pack-internal": "echo TODO maybe set an environment variable"
   },
@@ -192,6 +193,7 @@
   "devDependencies": {
     "@auth0/auth0-react": "2.0.1",
     "@babel/parser": "^7.21.3",
+    "@babel/types": "7.24.0",
     "@clerk/clerk-react": "4.18.0",
     "@commander-js/extra-typings": "^11.1.0",
     "@jest/globals": "^28.1.0",

src/bundler/index.test.ts

@@ -1,4 +1,4 @@
-import { test, expect } from "@jest/globals";
+import { jest, expect, test } from "@jest/globals";
 import { oneoffContext } from "./context.js";
 
 // Although these tests are run as ESM by ts-lint, this file is built as both
@@ -8,6 +8,8 @@ const dirname = "src/bundler";
 
 import {
   bundle,
+  doesImportConvexHttpRouter,
+  entryPoints,
   entryPointsByEnvironment,
   useNodeDirectiveRegex,
   mustBeIsolate,
@@ -23,21 +25,26 @@ const sorted = <T>(arr: T[], key: (el: T) => any): T[] => {
   return newArr.sort(cmp);
 };
 
+afterEach(() => {
+  jest.resetAllMocks();
+});
+
 test("bundle function is present", () => {
   expect(typeof bundle).toEqual("function");
 });
 
 test("bundle finds JavaScript functions", async () => {
+  const fixtureDir = dirname + "/test_fixtures/js/project01";
   const entryPoints = await entryPointsByEnvironment(
     oneoffContext,
-    dirname + "/test_fixtures/js",
+    fixtureDir,
     false,
   );
   const bundles = sorted(
     (
       await bundle(
         oneoffContext,
-        dirname + "/test_fixtures/js",
+        fixtureDir,
         entryPoints.isolate,
         false,
         "browser",
@@ -50,6 +57,85 @@ test("bundle finds JavaScript functions", async () => {
   expect(bundles[1].path).toEqual("foo.js");
 });
 
+test("returns true when simple import httpRouter found", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    import { httpRouter } from "convex/server";
+
+    export const val = 1;
+    `);
+  expect(result).toBeTruthy();
+});
+
+test("returns false when httpRouter is not imported", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    export const val = 1;
+    `);
+  expect(result).toBeFalsy();
+});
+
+test("returns true when multiline import httpRouter found", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    import {
+      httpRouter
+    } from "convex/server";
+
+    export const val = 1;
+    `);
+  expect(result).toBeTruthy();
+});
+
+test("returns true when httpRouter is imported with alias", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    import { httpRouter as router } from "convex/server";
+
+    export const val = 1;
+    `);
+  expect(result).toBeTruthy();
+});
+
+test("returns true when httpRouter is imported with alias and multiline", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    import {
+      httpRouter as router
+    } from "convex/server";
+
+    export const val = 1;
+    `);
+  expect(result).toBeTruthy();
+});
+
+test("returns true when multiple imports and httpRouter is imported", async () => {
+  const result = await doesImportConvexHttpRouter(`
+    import { cronJobs, httpRouter } from "convex/server";
+
+    export const val = 1;
+    `);
+  expect(result).toBeTruthy();
+});
+
+test("bundle warns about https.js|ts at top level", async () => {
+  const fixtureDir = dirname + "/test_fixtures/js/project_with_https";
+  const logSpy = jest.spyOn(console, "error");
+  await entryPoints(oneoffContext, fixtureDir, false);
+  expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("https"));
+});
+
+test("bundle does not warn about https.js|ts which is not at top level", async () => {
+  const fixtureDir =
+    dirname + "/test_fixtures/js/project_with_https_not_at_top_level";
+  const logSpy = jest.spyOn(console, "error");
+  await entryPoints(oneoffContext, fixtureDir, false);
+  expect(logSpy).toHaveBeenCalledTimes(0);
+});
+
+test("bundle does not warn about https.js|ts which does not import httpRouter", async () => {
+  const fixtureDir =
+    dirname + "/test_fixtures/js/project_with_https_without_router";
+  const logSpy = jest.spyOn(console, "error");
+  await entryPoints(oneoffContext, fixtureDir, false);
+  expect(logSpy).toHaveBeenCalledTimes(0);
+});
+
 test("use node regex", () => {
   // Double quotes
   expect('"use node";').toMatch(useNodeDirectiveRegex);

src/bundler/index.ts

@@ -1,7 +1,9 @@
-import { parse as parseAST } from "@babel/parser";
 import path from "path";
 import chalk from "chalk";
 import esbuild from "esbuild";
+import { parse as parseAST } from "@babel/parser";
+import { Identifier, ImportSpecifier } from "@babel/types";
+import * as Sentry from "@sentry/node";
 import { Filesystem } from "./fs.js";
 import { Context, logFailure, logWarning } from "./context.js";
 import { wasmPlugin } from "./wasm.js";
@@ -16,20 +18,22 @@ export type { Filesystem } from "./fs.js";
 
 export const actionsDir = "actions";
 
-// Returns a generator of { isDir, path } for all paths
+// Returns a generator of { isDir, path, depth } for all paths
 // within dirPath in some topological order (not including
 // dirPath itself).
 export function* walkDir(
   fs: Filesystem,
   dirPath: string,
-): Generator<{ isDir: boolean; path: string }, void, void> {
+  depth?: number,
+): Generator<{ isDir: boolean; path: string; depth: number }, void, void> {
+  depth = depth ?? 0;
   for (const dirEntry of fs.listDir(dirPath).sort()) {
     const childPath = path.join(dirPath, dirEntry.name);
     if (dirEntry.isDirectory()) {
-      yield { isDir: true, path: childPath };
-      yield* walkDir(fs, childPath);
+      yield { isDir: true, path: childPath, depth };
+      yield* walkDir(fs, childPath, depth + 1);
     } else if (dirEntry.isFile()) {
-      yield { isDir: false, path: childPath };
+      yield { isDir: false, path: childPath, depth };
     }
   }
 }
@@ -259,6 +263,29 @@ export async function bundleAuthConfig(ctx: Context, dir: string) {
   return result.modules;
 }
 
+export async function doesImportConvexHttpRouter(source: string) {
+  try {
+    const ast = parseAST(source, {
+      sourceType: "module",
+      plugins: ["typescript"],
+    });
+    return ast.program.body.some((node) => {
+      if (node.type !== "ImportDeclaration") return false;
+      return node.specifiers.some((s) => {
+        const specifier = s as ImportSpecifier;
+        const imported = specifier.imported as Identifier;
+        return imported.name === "httpRouter";
+      });
+    });
+  } catch {
+    return (
+      source.match(
+        /import\s*\{\s*httpRouter.*\}\s*from\s*"\s*convex\/server\s*"/,
+      ) !== null
+    );
+  }
+}
+
 export async function entryPoints(
   ctx: Context,
   dir: string,
@@ -272,20 +299,39 @@ export async function entryPoints(
     }
   };
 
-  for (const { isDir, path: fpath } of walkDir(ctx.fs, dir)) {
+  for (const { isDir, path: fpath, depth } of walkDir(ctx.fs, dir)) {
     if (isDir) {
       continue;
     }
     const relPath = path.relative(dir, fpath);
-    const base = path.parse(fpath).base;
+    const parsedPath = path.parse(fpath);
+    const base = parsedPath.base;
+    const extension = parsedPath.ext.toLowerCase();
 
     if (relPath.startsWith("_deps" + path.sep)) {
       logFailure(
         ctx,
         `The path "${fpath}" is within the "_deps" directory, which is reserved for dependencies. Please move your code to another directory.`,
       );
       return await ctx.crash(1, "invalid filesystem data");
-    } else if (relPath.startsWith("_generated" + path.sep)) {
+    }
+
+    if (depth === 0 && base.toLowerCase().startsWith("https.")) {
+      const source = ctx.fs.readUtf8File(fpath);
+      if (await doesImportConvexHttpRouter(source))
+        logWarning(
+          ctx,
+          chalk.yellow(
+            `Found ${fpath}. HTTP action routes will not be imported from this file. Did you mean to include http${extension}?`,
+          ),
+        );
+      Sentry.captureMessage(
+        `User code top level directory contains file ${base} which imports httpRouter.`,
+        "warning",
+      );
+    }
+
+    if (relPath.startsWith("_generated" + path.sep)) {
       log(chalk.yellow(`Skipping ${fpath}`));
     } else if (base.startsWith(".")) {
       log(chalk.yellow(`Skipping dotfile ${fpath}`));

src/bundler/test_fixtures/js/bar.js renamed to src/bundler/test_fixtures/js/project01/bar.js

@@ -0,0 +1,9 @@
+import { httpRouter } from "convex/server";
+
+export const val = 1;
+
+export let otherHttp = 20;
+
+const http = httpRouter();
+
+export default http;

src/bundler/test_fixtures/js/file with spaces.js renamed to src/bundler/test_fixtures/js/project01/file with spaces.js

@@ -0,0 +1,3 @@
+export default async function defaultExport() {
+  return 1;
+}

src/bundler/test_fixtures/js/foo.js renamed to src/bundler/test_fixtures/js/project01/foo.js

@@ -0,0 +1,5 @@
+import { httpRouter } from "convex/server";
+
+const http = httpRouter();
+
+export default http;

src/bundler/test_fixtures/js/project_with_https/https.js

@@ -0,0 +1 @@
+export const val = 1;