fix(plugin-vite): use Vite's pre-bundled dependency ids when

optimizeDeps is enabled (#3895)

Author: mryall-mawson

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

@@ -10,7 +10,11 @@ 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 {
+  depsOptimizerOf,
+  ensureVersionQuery,
+  tryOptimizedResolve,
+} from "./version_query.ts";
 import { builtinModules } from "node:module";
 
 // @ts-ignore Workaround for https://github.com/denoland/deno/issues/30850
@@ -158,6 +162,17 @@ export function deno(): Plugin {
 
         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);
         }

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

@@ -1,9 +1,53 @@
+import type { DevEnvironment } from "vite";
 import { cleanId } from "../utils.ts";
 
-/** The parts of Vite's `DepsOptimizer` we rely on. */
-export interface DepsOptimizerLike {
-  metadata: { browserHash: string };
-  options: { extensions?: string[] };
+/**
+ * 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=/;
@@ -16,8 +60,8 @@ const OPTIMIZABLE_REG = /\.[cm]?[jt]s$/;
  */
 export function depsOptimizerOf(
   environment: unknown,
-): DepsOptimizerLike | undefined {
-  return (environment as { depsOptimizer?: DepsOptimizerLike }).depsOptimizer;
+): DepsOptimizer | undefined {
+  return (environment as { depsOptimizer?: DepsOptimizer }).depsOptimizer;
 }
 
 /**
@@ -31,7 +75,7 @@ export function depsOptimizerOf(
  */
 export function ensureVersionQuery(
   resolved: string,
-  depsOptimizer: DepsOptimizerLike,
+  depsOptimizer: DepsOptimizer,
 ): string {
   // Only dependencies are versioned; app source uses `?t=` for HMR instead.
   if (!resolved.includes("node_modules")) return resolved;

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

@@ -1,19 +1,40 @@
 import { expect } from "@std/expect/expect";
 import {
-  type DepsOptimizerLike,
+  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[] } = {},
-): DepsOptimizerLike {
+  options: {
+    browserHash?: string;
+    extensions?: string[];
+    optimized?: Record<string, FakeDepInfo>;
+    depInfoList?: FakeDepInfo[];
+  } = {},
+): DepsOptimizer {
   return {
-    metadata: { browserHash: options.browserHash ?? "abc123" },
+    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", () => {
@@ -55,3 +76,29 @@ 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/tests/dev_server_test.ts

@@ -628,6 +628,10 @@ export default defineConfig({
 // 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.
+//
+// `@preact/signals` is deliberately left out of `include` here: pre-bundling
+// it would serve it from Vite's cache instead, so the clash this covers could
+// no longer happen. The test below pre-bundles it on purpose.
 integrationTest(
   "vite dev - optimizeDeps does not duplicate dependency instances",
   async () => {
@@ -647,7 +651,6 @@ export default defineConfig({
       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)
         );
@@ -665,6 +668,7 @@ export default defineConfig({
           queries.add(parsed.search);
         }
 
+        // A path fetched under more than one query string is loaded twice.
         const duplicated = Array.from(queriesByPath)
           .filter(([, queries]) => queries.size > 1)
           .map(([pathname]) => pathname);
@@ -681,6 +685,55 @@ export default defineConfig({
   },
 );
 
+// A pre-bundled dependency is served from Vite's own cache, so anything that
+// still resolves to the package on disk loads it alongside the bundle. Deno
+// rewrites imports inside jsr modules to `npm:` specifiers, which do not match
+// the optimizer's keys, so the lookup has to fall back to the pre-bundled
+// source to recognise them.
+integrationTest(
+  "vite dev - optimizeDeps serves pre-bundled dependencies once",
+  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", "@preact/signals"] },
+});
+`,
+    });
+
+    await launchDevServer(tmp.dir, async (address) => {
+      await withBrowser(async (page) => {
+        await page.goto(address, { waitUntil: "networkidle2" });
+
+        const paths: string[] = await page.evaluate(() =>
+          performance.getEntriesByType("resource").map((entry) =>
+            new URL(entry.name).pathname
+          )
+        );
+
+        // Sanity check that the optimizer actually ran.
+        expect(paths.some((p) => p.includes("/.vite/deps/"))).toEqual(true);
+
+        // The remote island imports both as `npm:` specifiers. They are
+        // pre-bundled, so neither may also be fetched from node_modules.
+        const rawCopies = paths.filter((p) =>
+          /node_modules\/.*\/(preact\/dist\/|@preact\/signals\/dist\/)/.test(p)
+        );
+
+        expect(rawCopies).toEqual([]);
+
+        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",