fix(plugin-vite): make resolveId URLs consistent with Vite when

optimizeDeps is enabled (#3895)

Author: mryall-mawson

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

@@ -10,6 +10,7 @@ import * as path from "@std/path";
 import * as babel from "@babel/core";
 import { httpAbsolute } from "./patches/http_absolute.ts";
 import { cleanId, JS_REG, JSX_REG } from "../utils.ts";
+import { depsOptimizerOf, ensureVersionQuery } from "./version_query.ts";
 import { builtinModules } from "node:module";
 
 // @ts-ignore Workaround for https://github.com/denoland/deno/issues/30850
@@ -155,6 +156,12 @@ export function deno(): Plugin {
           resolved = path.fromFileUrl(resolved);
         }
 
+        const depsOptimizer = depsOptimizerOf(this.environment);
+        if (depsOptimizer !== undefined) {
+          // When `optimizeDeps` is enabled, ensure resolved URLs include versions to match Vite.
+          resolved = ensureVersionQuery(resolved, depsOptimizer);
+        }
+
         return {
           id: resolved,
           meta: {

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

@@ -0,0 +1,67 @@
+import { cleanId } from "../utils.ts";
+
+/** The parts of Vite's `DepsOptimizer` we rely on. */
+export interface DepsOptimizerLike {
+  metadata: { browserHash: string };
+  options: { extensions?: string[] };
+}
+
+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,
+): DepsOptimizerLike | undefined {
+  return (environment as { depsOptimizer?: DepsOptimizerLike }).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: DepsOptimizerLike,
+): 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,57 @@
+import { expect } from "@std/expect/expect";
+import {
+  type DepsOptimizerLike,
+  depsOptimizerOf,
+  ensureVersionQuery,
+} from "./version_query.ts";
+
+const DEP = "/app/node_modules/preact/dist/preact.mjs";
+
+function optimizer(
+  options: { browserHash?: string; extensions?: string[] } = {},
+): DepsOptimizerLike {
+  return {
+    metadata: { browserHash: options.browserHash ?? "abc123" },
+    options: { extensions: options.extensions },
+  };
+}
+
+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();
+});

packages/plugin-vite/tests/dev_server_test.ts

@@ -623,6 +623,64 @@ export default defineConfig({
   });
 });
 
+// With `optimizeDeps` active, Vite's import analysis appends `?v=<hash>` to
+// the imports it rewrites, while imports from inside a `\0deno::` virtual
+// module are resolved by the Deno plugin and emitted unhashed. The browser
+// keys modules on the url it fetched, so one file under two urls becomes two
+// instances. This is a resolve-time problem, not a load-time one.
+integrationTest(
+  "vite dev - optimizeDeps does not duplicate dependency instances",
+  async () => {
+    const fixture = path.join(FIXTURE_DIR, "remote_island");
+    await using tmp = await prepareDevServer(fixture, {
+      config: `import { defineConfig } from "vite";
+import { fresh } from "@fresh/plugin-vite";
+
+export default defineConfig({
+  plugins: [fresh({ islandSpecifiers: ["@marvinh-test/fresh-island"] })],
+  optimizeDeps: { include: ["preact"] },
+});
+`,
+    });
+
+    await launchDevServer(tmp.dir, async (address) => {
+      await withBrowser(async (page) => {
+        await page.goto(address, { waitUntil: "networkidle2" });
+
+        // A path fetched under more than one query string is loaded twice.
+        const urls: string[] = await page.evaluate(() =>
+          performance.getEntriesByType("resource").map((entry) => entry.name)
+        );
+
+        const queriesByPath = new Map<string, Set<string>>();
+        for (const url of urls) {
+          const parsed = new URL(url);
+          if (!/\.[mc]?[tj]sx?$/.test(parsed.pathname)) continue;
+
+          let queries = queriesByPath.get(parsed.pathname);
+          if (queries === undefined) {
+            queries = new Set();
+            queriesByPath.set(parsed.pathname, queries);
+          }
+          queries.add(parsed.search);
+        }
+
+        const duplicated = Array.from(queriesByPath)
+          .filter(([, queries]) => queries.size > 1)
+          .map(([pathname]) => pathname);
+
+        expect(duplicated).toEqual([]);
+
+        // A duplicated Preact makes hydration throw, so the island never
+        // becomes interactive.
+        await page.locator(".remote-island").wait();
+        await page.locator(".increment").click();
+        await waitForText(page, ".result", "Count: 1");
+      });
+    });
+  },
+);
+
 // issue: https://github.com/denoland/fresh/issues/3666
 integrationTest(
   "vite dev - basePath does not intercept Vite URLs",