diff --git a/packages/plugin-vite/src/plugins/deno.ts b/packages/plugin-vite/src/plugins/deno.ts index 6cefed7e463..acc2be64446 100644 --- a/packages/plugin-vite/src/plugins/deno.ts +++ b/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=` 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); diff --git a/packages/plugin-vite/src/plugins/version_query.ts b/packages/plugin-vite/src/plugins/version_query.ts new file mode 100644 index 00000000000..704fe692c20 --- /dev/null +++ b/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; + +/** + * 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 { + // 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=` 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}`; +} diff --git a/packages/plugin-vite/src/plugins/version_query_test.ts b/packages/plugin-vite/src/plugins/version_query_test.ts new file mode 100644 index 00000000000..56095c6e584 --- /dev/null +++ b/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; + 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(); +}); diff --git a/packages/plugin-vite/src/utils.ts b/packages/plugin-vite/src/utils.ts index 0a866ef347f..6cb0aa8ae6d 100644 --- a/packages/plugin-vite/src/utils.ts +++ b/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=` to + * dependency ids when `optimizeDeps` is active, `?commonjs-es-import` for + * interop shims and `?t=` 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; diff --git a/packages/plugin-vite/tests/dev_server_test.ts b/packages/plugin-vite/tests/dev_server_test.ts index 1572154f96a..8aa8f8f131e 100644 --- a/packages/plugin-vite/tests/dev_server_test.ts +++ b/packages/plugin-vite/tests/dev_server_test.ts @@ -564,6 +564,176 @@ Deno.test({ sanitizeResources: false, }); +// issue: https://github.com/freshframework/fresh/issues/3895 +// Vite appends a `?v=` 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=` urls that must still resolve to a file. + const seen = new Set(); + 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); + }); +}); + +// With `optimizeDeps` active, Vite's import analysis appends `?v=` 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. +// +// `@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 () => { + 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" }); + + const urls: string[] = await page.evaluate(() => + performance.getEntriesByType("resource").map((entry) => entry.name) + ); + + const queriesByPath = new Map>(); + 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); + } + + // 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); + + 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"); + }); + }); + }, +); + +// 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", diff --git a/packages/plugin-vite/tests/test_utils.ts b/packages/plugin-vite/tests/test_utils.ts index 2d7a4d5c6b6..26b03ade35a 100644 --- a/packages/plugin-vite/tests/test_utils.ts +++ b/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({