fix(plugin-vite): correct module resolution when Vite optimizeDeps is enabled (#3895)

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

@@ -9,7 +9,12 @@ 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 {
+  depsOptimizerOf,
+  ensureVersionQuery,
+  tryOptimizedResolve,
+} from "./version_query.ts";
 import { builtinModules } from "node:module";
 
 // @ts-ignore Workaround for https://github.com/denoland/deno/issues/30850
@@ -155,6 +160,23 @@ export function deno(): Plugin {
           resolved = path.fromFileUrl(resolved);
         }
 
+        const depsOptimizer = depsOptimizerOf(this.environment);
+        if (depsOptimizer !== undefined) {
+          // A pre-bundled dependency is served from Vite's cache, so hand back
+          // the cache id rather than the file we just resolved to.
+          const optimized = await tryOptimizedResolve(
+            original,
+            resolved,
+            depsOptimizer,
+          );
+          if (optimized !== undefined) {
+            return { id: optimized };
+          }
+
+          // When `optimizeDeps` is enabled, ensure resolved URLs include versions to match Vite.
+          resolved = ensureVersionQuery(resolved, depsOptimizer);
+        }
+
         return {
           id: resolved,
           meta: {
@@ -217,7 +239,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 +281,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/plugins/version_query.ts

@@ -0,0 +1,111 @@
+import type { DevEnvironment } from "vite";
+import { cleanId } from "../utils.ts";
+
+/**
+ * Vite does not export `DepsOptimizer`, but it is reachable through the
+ * `DevEnvironment` type, so we can use Vite's own definition rather than
+ * describing the shape ourselves.
+ */
+export type DepsOptimizer = NonNullable<DevEnvironment["depsOptimizer"]>;
+
+/**
+ * Mirrors `tryOptimizedResolve()` from Vite's `vite:resolve` plugin.
+ *
+ * When a dependency has been pre-bundled, Vite serves it from its own cache
+ * (`/.vite/deps/…`) rather than from the package on disk. We have to return
+ * the same id, otherwise the specifiers we resolve keep pointing at the
+ * original files and the dependency is loaded a second time alongside the
+ * bundle.
+ *
+ * Vite looks the dependency up by specifier, which only works for the bare
+ * names it uses itself. Deno rewrites imports inside jsr modules to `npm:`
+ * specifiers (`npm:@preact/signals@^2.0.0`), so we additionally match on the
+ * file the dependency was pre-bundled from, which is independent of how the
+ * specifier was written.
+ */
+export async function tryOptimizedResolve(
+  specifier: string,
+  resolved: string,
+  depsOptimizer: DepsOptimizer,
+): Promise<string | undefined> {
+  // Metadata is incomplete until dependency scanning has settled.
+  await depsOptimizer.scanProcessing;
+
+  const { optimized, discovered, chunks, depInfoList } = depsOptimizer.metadata;
+
+  const bySpecifier = optimized[specifier] ?? discovered[specifier] ??
+    chunks[specifier];
+  if (bySpecifier !== undefined) {
+    return depsOptimizer.getOptimizedDepId(bySpecifier);
+  }
+
+  const file = cleanId(resolved);
+  const bySource = depInfoList.find((info) =>
+    info.src !== undefined && cleanId(info.src) === file
+  );
+  if (bySource !== undefined) {
+    return depsOptimizer.getOptimizedDepId(bySource);
+  }
+
+  return undefined;
+}
+
+const DEP_VERSION_REG = /[?&]v=/;
+/** Mirrors Vite's `OPTIMIZABLE_ENTRY_RE`, which also excludes `.jsx`/`.tsx`. */
+const OPTIMIZABLE_REG = /\.[cm]?[jt]s$/;
+
+/**
+ * The dependency optimizer only exists on a dev environment, and is absent
+ * from the base `Environment` type that plugin hooks are handed.
+ */
+export function depsOptimizerOf(
+  environment: unknown,
+): DepsOptimizer | undefined {
+  return (environment as { depsOptimizer?: DepsOptimizer }).depsOptimizer;
+}
+
+/**
+ * Mirrors `ensureVersionQuery()` from Vite's `vite:resolve` plugin.
+ *
+ * That is where Vite attaches the optimizer's `?v=<hash>` to a dependency,
+ * but `vite:resolve` never runs for the specifiers we resolve ourselves. Both
+ * resolvers have to produce the same url for a given file: the browser keys
+ * modules on the url it was told to fetch, so a file reachable under two urls
+ * is loaded twice as two independent instances.
+ */
+export function ensureVersionQuery(
+  resolved: string,
+  depsOptimizer: DepsOptimizer,
+): string {
+  // Only dependencies are versioned; app source uses `?t=` for HMR instead.
+  if (!resolved.includes("node_modules")) return resolved;
+  if (DEP_VERSION_REG.test(resolved)) return resolved;
+
+  // Empty until the optimizer has finished initialising.
+  const { browserHash } = depsOptimizer.metadata;
+  if (!browserHash) return resolved;
+
+  // Vite only versions what it could pre-bundle, so we must match.
+  const file = cleanId(resolved);
+  const { extensions } = depsOptimizer.options;
+  const optimizable = OPTIMIZABLE_REG.test(file) ||
+    (extensions?.some((ext) => file.endsWith(ext)) ?? false);
+  if (!optimizable) return resolved;
+
+  return injectVersionQuery(resolved, browserHash);
+}
+
+function injectVersionQuery(id: string, browserHash: string): string {
+  const hashIndex = id.indexOf("#");
+  const fragment = hashIndex >= 0 ? id.slice(hashIndex) : "";
+  const rest = hashIndex >= 0 ? id.slice(0, hashIndex) : id;
+
+  const queryIndex = rest.indexOf("?");
+  if (queryIndex >= 0) {
+    const pathname = rest.slice(0, queryIndex);
+    const query = rest.slice(queryIndex + 1);
+    return `${pathname}?v=${browserHash}&${query}${fragment}`;
+  }
+
+  return `${rest}?v=${browserHash}${fragment}`;
+}

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

@@ -0,0 +1,104 @@
+import { expect } from "@std/expect/expect";
+import {
+  type DepsOptimizer,
+  depsOptimizerOf,
+  ensureVersionQuery,
+  tryOptimizedResolve,
+} from "./version_query.ts";
+
+const DEP = "/app/node_modules/preact/dist/preact.mjs";
+
+interface FakeDepInfo {
+  file: string;
+  src?: string;
+  browserHash?: string;
+}
+
+function optimizer(
+  options: {
+    browserHash?: string;
+    extensions?: string[];
+    optimized?: Record<string, FakeDepInfo>;
+    depInfoList?: FakeDepInfo[];
+  } = {},
+): DepsOptimizer {
+  return {
+    metadata: {
+      browserHash: options.browserHash ?? "abc123",
+      optimized: options.optimized ?? {},
+      discovered: {},
+      chunks: {},
+      depInfoList: options.depInfoList ?? [],
+    },
+    options: { extensions: options.extensions },
+    getOptimizedDepId: (info: FakeDepInfo) =>
+      `${info.file}?v=${info.browserHash ?? "abc123"}`,
+    // The rest of Vite's `DepsOptimizer` is not exercised here.
+  } as unknown as DepsOptimizer;
+}
+
+Deno.test("version query - versions a dependency", () => {
+  expect(ensureVersionQuery(DEP, optimizer())).toEqual(`${DEP}?v=abc123`);
+});
+
+Deno.test("version query - ignores app source", () => {
+  const id = "/app/islands/Counter.ts";
+  expect(ensureVersionQuery(id, optimizer())).toEqual(id);
+});
+
+Deno.test("version query - is idempotent", () => {
+  const id = `${DEP}?v=abc123`;
+  expect(ensureVersionQuery(id, optimizer())).toEqual(id);
+});
+
+Deno.test("version query - ignores extensions Vite cannot pre-bundle", () => {
+  const id = "/app/node_modules/some-dep/Widget.tsx";
+  expect(ensureVersionQuery(id, optimizer())).toEqual(id);
+
+  // ...unless the optimizer was configured to handle them.
+  expect(ensureVersionQuery(id, optimizer({ extensions: [".tsx"] })))
+    .toEqual(`${id}?v=abc123`);
+});
+
+Deno.test("version query - waits for the optimizer to initialise", () => {
+  expect(ensureVersionQuery(DEP, optimizer({ browserHash: "" }))).toEqual(DEP);
+});
+
+Deno.test("version query - keeps an existing query and fragment", () => {
+  expect(ensureVersionQuery(`${DEP}?foo=1`, optimizer()))
+    .toEqual(`${DEP}?v=abc123&foo=1`);
+
+  expect(ensureVersionQuery(`${DEP}#bar`, optimizer()))
+    .toEqual(`${DEP}?v=abc123#bar`);
+});
+
+Deno.test("version query - depsOptimizerOf tolerates a missing optimizer", () => {
+  expect(depsOptimizerOf({})).toBeUndefined();
+  expect(depsOptimizerOf({ depsOptimizer: optimizer() })).toBeDefined();
+});
+
+Deno.test("optimized resolve - substitutes a pre-bundled dependency", async () => {
+  const deps = optimizer({
+    optimized: { preact: { file: "/app/.vite/deps/preact.js" } },
+  });
+
+  expect(await tryOptimizedResolve("preact", DEP, deps))
+    .toEqual("/app/.vite/deps/preact.js?v=abc123");
+});
+
+Deno.test("optimized resolve - matches by source for `npm:` specifiers", async () => {
+  // Deno rewrites imports inside jsr modules, so the specifier never matches
+  // the optimizer's keys and only the source file identifies the dependency.
+  const deps = optimizer({
+    optimized: { preact: { file: "/app/.vite/deps/preact.js", src: DEP } },
+    depInfoList: [{ file: "/app/.vite/deps/preact.js", src: DEP }],
+  });
+
+  expect(await tryOptimizedResolve("npm:preact@^10.0.0", DEP, deps))
+    .toEqual("/app/.vite/deps/preact.js?v=abc123");
+});
+
+Deno.test("optimized resolve - leaves dependencies that were not pre-bundled", async () => {
+  expect(await tryOptimizedResolve("preact", DEP, optimizer()))
+    .toBeUndefined();
+});

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;