fix: plugin-vite: strip query from URLs before loading (#3895)

Author: mryall-mawson

packages/plugin-vite/src/plugins/deno.ts

@@ -9,7 +9,7 @@ import {
 import * as path from "@std/path";
 import * as babel from "@babel/core";
 import { httpAbsolute } from "./patches/http_absolute.ts";
-import { JS_REG, JSX_REG } from "../utils.ts";
+import { cleanId, JS_REG, JSX_REG } from "../utils.ts";
 import { builtinModules } from "node:module";
 
 // @ts-ignore Workaround for https://github.com/denoland/deno/issues/30850
@@ -217,7 +217,9 @@ export function deno(): Plugin {
         return;
       }
 
-      const url = path.toFileUrl(id);
+      // Vite appends suffixes like `?v=<hash>` to dependency ids. They are
+      // not part of the file path and must be dropped before hitting the fs.
+      const url = path.toFileUrl(cleanId(id));
 
       const result = await loader.load(url.href, meta.type);
       if (result.kind === "external") {
@@ -257,7 +259,7 @@ export function deno(): Plugin {
           const { specifier } = parseDenoSpecifier(id);
           actualId = specifier;
         }
-        actualId = actualId.replace("?commonjs-es-import", "");
+        actualId = cleanId(actualId);
 
         if (actualId.startsWith("\0")) {
           actualId = actualId.slice(1);

packages/plugin-vite/src/utils.ts

@@ -5,6 +5,18 @@ import type { ImportCheck } from "./plugins/verify_imports.ts";
 export const JS_REG = /\.([tj]sx?|[mc]?[tj]s)(\?.*)?$/;
 export const JSX_REG = /\.[tj]sx(\?.*)?$/;
 
+const QUERY_REG = /[?#].*$/s;
+
+/**
+ * Strip Vite's query and hash suffixes from a module id, so that it can be
+ * treated as a file path again. Vite appends suffixes like `?v=<hash>` to
+ * dependency ids when `optimizeDeps` is active, `?commonjs-es-import` for
+ * interop shims and `?t=<timestamp>` on HMR updates.
+ */
+export function cleanId(id: string): string {
+  return id.replace(QUERY_REG, "");
+}
+
 export function pathWithRoot(fileOrDir: string, root?: string): string {
   if (path.isAbsolute(fileOrDir)) return fileOrDir;
 

packages/plugin-vite/tests/dev_server_test.ts

@@ -564,6 +564,65 @@ Deno.test({
   sanitizeResources: false,
 });
 
+// issue: https://github.com/freshframework/fresh/issues/3895
+// Vite appends a `?v=<hash>` suffix to dependency ids when `optimizeDeps` is
+// active. The Deno loader must strip it instead of treating it as part of the
+// file path, in both the client and the ssr environment.
+integrationTest("vite dev - works with optimizeDeps enabled", async () => {
+  const fixture = path.join(FIXTURE_DIR, "no_islands");
+  await using tmp = await prepareDevServer(fixture, {
+    config: `import { defineConfig } from "vite";
+import { fresh } from "@fresh/plugin-vite";
+
+export default defineConfig({
+  plugins: [fresh()],
+  optimizeDeps: { include: ["preact"] },
+  environments: {
+    ssr: { optimizeDeps: { include: ["preact"] } },
+  },
+});
+`,
+  });
+
+  await launchDevServer(tmp.dir, async (address) => {
+    // ssr environment
+    const res = await fetch(address);
+    const text = await res.text();
+    expect(res.status).toEqual(200);
+    expect(text).toContain("ok");
+
+    // client environment: walk the module graph from the client entry. Vite
+    // versions dependency ids whether or not it pre-bundles them, so the
+    // graph contains `?v=<hash>` urls that must still resolve to a file.
+    const seen = new Set<string>();
+    const queue = ["/@id/fresh:client-entry"];
+    const failed: string[] = [];
+    let versioned = 0;
+
+    while (queue.length > 0) {
+      const url = queue.pop()!;
+      if (seen.has(url) || url.startsWith("/@vite/")) continue;
+      seen.add(url);
+
+      const modRes = await fetch(`${address}${url}`);
+      const code = await modRes.text();
+      if (modRes.status !== 200) {
+        failed.push(`${modRes.status} ${url}`);
+        continue;
+      }
+
+      if (url.includes("?v=")) versioned++;
+
+      for (const match of code.matchAll(/from "(\/[^"]+)"/g)) {
+        queue.push(match[1]);
+      }
+    }
+
+    expect(failed).toEqual([]);
+    expect(versioned).toBeGreaterThan(0);
+  });
+});
+
 // issue: https://github.com/denoland/fresh/issues/3666
 integrationTest(
   "vite dev - basePath does not intercept Vite URLs",

packages/plugin-vite/tests/test_utils.ts

@@ -49,7 +49,10 @@ async function copyDir(from: string, to: string) {
   }
 }
 
-export async function prepareDevServer(fixtureDir: string) {
+export async function prepareDevServer(
+  fixtureDir: string,
+  options: { config?: string } = {},
+) {
   const tmp = await withTmpDir({
     dir: path.join(import.meta.dirname!, ".."),
     prefix: "tmp_vite_",
@@ -59,7 +62,7 @@ export async function prepareDevServer(fixtureDir: string) {
 
   await Deno.writeTextFile(
     path.join(tmp.dir, "vite.config.ts"),
-    `import { defineConfig } from "vite";
+    options.config ?? `import { defineConfig } from "vite";
 import { fresh } from "@fresh/plugin-vite";
 
 export default defineConfig({