From 717e266c17e37002dcdcbfe9506d915b9aa0ca81 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 16 May 2026 17:16:06 +1000 Subject: [PATCH] fix(rsc): preserve exported client reference subpaths Client reference dedupe currently rewrites every package file imported through the RSC package proxy back to the package root. That loses subpath identity when a package exposes client modules through an exported subpath instead of its root barrel. The plugin now derives a bare import specifier from package metadata, preserving exact and pattern exports while keeping private internals on the package root so existing context dedupe behavior stays intact. Regression coverage exercises exported subpaths, scoped packages, export patterns, private internals, and legacy deep imports. --- .../src/plugins/client-reference-dedup.ts | 238 ++++++++++++++++-- tests/client-reference-dedup.test.ts | 210 ++++++++++++++-- 2 files changed, 407 insertions(+), 41 deletions(-) diff --git a/packages/vinext/src/plugins/client-reference-dedup.ts b/packages/vinext/src/plugins/client-reference-dedup.ts index 26ac3d60b..51c9fc027 100644 --- a/packages/vinext/src/plugins/client-reference-dedup.ts +++ b/packages/vinext/src/plugins/client-reference-dedup.ts @@ -1,5 +1,55 @@ +import { readFile as readFileFromFs } from "node:fs/promises"; import type { Plugin } from "vite"; +type ReadPackageJson = (path: string) => Promise; + +type ClientReferenceDedupOptions = { + readFile?: ReadPackageJson; +}; + +type PackageImportSpecifier = { + packageName: string; + specifier: string; +}; + +type PackagePath = { + packageName: string; + packageRoot: string; + relativePath: string; +}; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function defaultReadPackageJson(path: string): Promise { + return readFileFromFs(path, "utf8"); +} + +function parsePackagePath(absolutePath: string): PackagePath | null { + const marker = "/node_modules/"; + const lastIdx = absolutePath.lastIndexOf(marker); + if (lastIdx === -1) return null; + + const rest = absolutePath.slice(lastIdx + marker.length); + const parts = rest.split("/"); + const packagePartCount = rest.startsWith("@") ? 2 : 1; + const packageParts = parts.slice(0, packagePartCount); + + if (packageParts.length < packagePartCount || packageParts.some((part) => part === "")) { + return null; + } + + const packageName = packageParts.join("/"); + const relativeParts = parts.slice(packagePartCount); + + return { + packageName, + packageRoot: absolutePath.slice(0, lastIdx + marker.length + packageName.length), + relativePath: relativeParts.join("/"), + }; +} + /** * Extract the bare package name from an absolute file path containing node_modules. * @@ -7,20 +57,152 @@ import type { Plugin } from "vite"; * Returns `null` if the path doesn't contain `/node_modules/`. */ export function extractPackageName(absolutePath: string): string | null { - const marker = "/node_modules/"; - const lastIdx = absolutePath.lastIndexOf(marker); - if (lastIdx === -1) return null; + return parsePackagePath(absolutePath)?.packageName ?? null; +} - const rest = absolutePath.slice(lastIdx + marker.length); - if (rest.startsWith("@")) { - // Scoped package: @org/name - const parts = rest.split("/"); - if (parts.length < 2) return null; - return `${parts[0]}/${parts[1]}`; +function normalizeExportTarget(target: string): string { + return target.startsWith("./") ? target.slice(2) : target; +} + +function matchExportTarget(target: string, relativePath: string): string | null { + const normalizedTarget = normalizeExportTarget(target); + const wildcardIndex = normalizedTarget.indexOf("*"); + + if (wildcardIndex === -1) { + return normalizedTarget === relativePath ? "" : null; + } + + const beforeWildcard = normalizedTarget.slice(0, wildcardIndex); + const afterWildcard = normalizedTarget.slice(wildcardIndex + 1); + + if (!relativePath.startsWith(beforeWildcard) || !relativePath.endsWith(afterWildcard)) { + return null; + } + + return relativePath.slice(beforeWildcard.length, relativePath.length - afterWildcard.length); +} + +function exportKeyToSpecifier( + packageName: string, + exportKey: string, + wildcardMatch: string, +): string { + if (exportKey === ".") return packageName; + + const subpath = exportKey.startsWith("./") ? exportKey.slice(2) : exportKey; + const resolvedSubpath = subpath.includes("*") ? subpath.replace("*", wildcardMatch) : subpath; + + return `${packageName}/${resolvedSubpath}`; +} + +function collectExportTargets(value: unknown): string[] { + if (typeof value === "string") return [value]; + if (Array.isArray(value)) return value.flatMap((entry) => collectExportTargets(entry)); + if (!isRecord(value)) return []; + + return Object.values(value).flatMap((entry) => collectExportTargets(entry)); +} + +function findExportSpecifier( + packageName: string, + exportsValue: unknown, + relativePath: string, +): string | null { + if (!isRecord(exportsValue)) { + for (const target of collectExportTargets(exportsValue)) { + const wildcardMatch = matchExportTarget(target, relativePath); + if (wildcardMatch !== null) { + return exportKeyToSpecifier(packageName, ".", wildcardMatch); + } + } + return null; + } + + const entries = Object.entries(exportsValue); + const hasSubpathKeys = entries.some(([key]) => key === "." || key.startsWith("./")); + if (!hasSubpathKeys) { + for (const target of collectExportTargets(exportsValue)) { + const wildcardMatch = matchExportTarget(target, relativePath); + if (wildcardMatch !== null) { + return exportKeyToSpecifier(packageName, ".", wildcardMatch); + } + } + return null; } - // Regular package: name - const slashIdx = rest.indexOf("/"); - return slashIdx === -1 ? rest : rest.slice(0, slashIdx); + + let bestMatch: { key: string; wildcard: string } | null = null; + + for (const [key, value] of entries) { + if (key !== "." && !key.startsWith("./")) continue; + + for (const target of collectExportTargets(value)) { + const wildcardMatch = matchExportTarget(target, relativePath); + if (wildcardMatch === null) continue; + + if (!bestMatch || key.length > bestMatch.key.length) { + bestMatch = { key, wildcard: wildcardMatch }; + } + } + } + + return bestMatch ? exportKeyToSpecifier(packageName, bestMatch.key, bestMatch.wildcard) : null; +} + +function getLegacyEntry(packageJson: Record): string { + const browser = packageJson.browser; + if (typeof browser === "string") return browser; + + const module = packageJson.module; + if (typeof module === "string") return module; + + const main = packageJson.main; + return typeof main === "string" ? main : "index.js"; +} + +function matchesLegacyEntry(legacyEntry: string, relativePath: string): boolean { + const normalizedEntry = normalizeExportTarget(legacyEntry); + return normalizedEntry === relativePath || `${normalizedEntry}.js` === relativePath; +} + +/** + * Convert an absolute package file path into the least lossy bare import + * specifier that can be handed back to Vite's dependency optimizer. + */ +export async function extractPackageImportSpecifier( + absolutePath: string, + readPackageJson: ReadPackageJson = defaultReadPackageJson, +): Promise { + const packagePath = parsePackagePath(absolutePath); + if (!packagePath) return null; + + const { packageName, packageRoot, relativePath } = packagePath; + if (relativePath === "") { + return { packageName, specifier: packageName }; + } + + let packageJson: Record | null = null; + try { + const rawPackageJson = await readPackageJson(`${packageRoot}/package.json`); + const parsedPackageJson: unknown = JSON.parse(rawPackageJson); + packageJson = isRecord(parsedPackageJson) ? parsedPackageJson : null; + } catch { + packageJson = null; + } + + if (!packageJson) { + return { packageName, specifier: packageName }; + } + + if ("exports" in packageJson) { + const exportedSpecifier = findExportSpecifier(packageName, packageJson.exports, relativePath); + return { packageName, specifier: exportedSpecifier ?? packageName }; + } + + const specifier = matchesLegacyEntry(getLegacyEntry(packageJson), relativePath) + ? packageName + : `${packageName}/${relativePath}`; + + return { packageName, specifier }; } const DEDUP_PREFIX = "\0vinext:dedup/"; @@ -37,8 +219,10 @@ const PROXY_MARKER = "virtual:vite-rsc/client-in-server-package-proxy/"; * * Dev-only — production builds use the SSR manifest which handles this correctly. */ -export function clientReferenceDedupPlugin(): Plugin { +export function clientReferenceDedupPlugin(options: ClientReferenceDedupOptions = {}): Plugin { let excludeSet = new Set(); + const readPackageJson = options.readFile ?? defaultReadPackageJson; + const packageImportCache = new Map>(); return { name: "vinext:client-reference-dedup", @@ -55,7 +239,7 @@ export function clientReferenceDedupPlugin(): Plugin { resolveId: { filter: { id: /node_modules/ }, - handler(id, importer) { + async handler(id, importer) { // Only operate in the client environment if (this.environment?.name !== "client") return; @@ -65,19 +249,23 @@ export function clientReferenceDedupPlugin(): Plugin { // Only handle absolute paths through node_modules if (!id.startsWith("/") || !id.includes("/node_modules/")) return; - const pkgName = extractPackageName(id); - if (!pkgName) return; + const packageName = extractPackageName(id); + if (!packageName) return; // Respect user's optimizeDeps.exclude - if (excludeSet.has(pkgName)) return; - - // Lossy mapping: we collapse submodule paths (e.g. `pkg/dist/Button.js`) - // to the bare package name (`pkg`), assuming the package entry barrel-exports - // the same symbols. This holds for well-designed component libraries — the - // primary target of this plugin. A more precise approach would resolve through - // the package's `exports` map to find an exact subpath, but the barrel-export - // assumption is sufficient for the common case. - return `${DEDUP_PREFIX}${pkgName}`; + if (excludeSet.has(packageName)) return; + + let packageImportPromise = packageImportCache.get(id); + if (!packageImportPromise) { + packageImportPromise = extractPackageImportSpecifier(id, readPackageJson); + packageImportCache.set(id, packageImportPromise); + } + + const packageImport = await packageImportPromise; + if (!packageImport) return; + if (excludeSet.has(packageImport.specifier)) return; + + return `${DEDUP_PREFIX}${packageImport.specifier}`; }, }, diff --git a/tests/client-reference-dedup.test.ts b/tests/client-reference-dedup.test.ts index f09853671..aedc3715c 100644 --- a/tests/client-reference-dedup.test.ts +++ b/tests/client-reference-dedup.test.ts @@ -3,6 +3,7 @@ import type { Plugin } from "vite"; import vinext from "../packages/vinext/src/index.js"; import { extractPackageName, + extractPackageImportSpecifier, clientReferenceDedupPlugin, } from "../packages/vinext/src/plugins/client-reference-dedup.js"; @@ -44,6 +45,140 @@ describe("extractPackageName", () => { }); }); +describe("extractPackageImportSpecifier", () => { + it("preserves exported package subpaths instead of collapsing them to the package root", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/widget-lib/client/button.js", + async () => + JSON.stringify({ + name: "widget-lib", + exports: { + ".": "./index.js", + "./client/button": "./client/button.js", + }, + }), + ); + + expect(result).toEqual({ + packageName: "widget-lib", + specifier: "widget-lib/client/button", + }); + }); + + it("preserves scoped exported package subpaths", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/@scope/widget/client/button.js", + async () => + JSON.stringify({ + name: "@scope/widget", + exports: { + ".": "./index.js", + "./client/button": "./client/button.js", + }, + }), + ); + + expect(result).toEqual({ + packageName: "@scope/widget", + specifier: "@scope/widget/client/button", + }); + }); + + it("preserves package subpaths declared with export patterns", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/widget-lib/dist/client/button.js", + async () => + JSON.stringify({ + name: "widget-lib", + exports: { + ".": "./dist/index.js", + "./client/*": "./dist/client/*.js", + }, + }), + ); + + expect(result).toEqual({ + packageName: "widget-lib", + specifier: "widget-lib/client/button", + }); + }); + + it("preserves package subpaths declared with conditional exports", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/widget-lib/dist/client/button.js", + async () => + JSON.stringify({ + name: "widget-lib", + exports: { + ".": { + import: "./dist/index.mjs", + default: "./dist/index.js", + }, + "./client/button": { + import: "./dist/client/button.mjs", + default: "./dist/client/button.js", + }, + }, + }), + ); + + expect(result).toEqual({ + packageName: "widget-lib", + specifier: "widget-lib/client/button", + }); + }); + + it("keeps private package internals on the package root when exports block deep imports", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/fake-context-lib/internal/context.js", + async () => + JSON.stringify({ + name: "fake-context-lib", + exports: { + ".": "./index.js", + }, + }), + ); + + expect(result).toEqual({ + packageName: "fake-context-lib", + specifier: "fake-context-lib", + }); + }); + + it("preserves legacy deep imports when package exports do not restrict subpaths", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/legacy-lib/client/button.js", + async () => + JSON.stringify({ + name: "legacy-lib", + main: "index.js", + }), + ); + + expect(result).toEqual({ + packageName: "legacy-lib", + specifier: "legacy-lib/client/button.js", + }); + }); + + it("maps extensionless legacy main entries back to the package root", async () => { + const result = await extractPackageImportSpecifier( + "/project/node_modules/legacy-lib/dist/index.js", + async () => + JSON.stringify({ + name: "legacy-lib", + main: "dist/index", + }), + ); + + expect(result).toEqual({ + packageName: "legacy-lib", + specifier: "legacy-lib", + }); + }); +}); + describe("clientReferenceDedupPlugin", () => { const plugin = clientReferenceDedupPlugin(); const resolveId = (plugin.resolveId as any).handler; @@ -54,9 +189,9 @@ describe("clientReferenceDedupPlugin", () => { } describe("resolveId", () => { - it("redirects absolute node_modules imports from proxy modules in client env", () => { + it("redirects absolute node_modules imports from proxy modules in client env", async () => { const ctx = createContext("client"); - const result = resolveId.call( + const result = await resolveId.call( ctx, "/project/node_modules/@mantine/core/esm/MantineProvider.mjs", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", @@ -64,9 +199,52 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBe("\0vinext:dedup/@mantine/core"); }); - it("skips non-client environments", () => { + it("preserves package subpaths when package exports map them", async () => { + const subpathPlugin = clientReferenceDedupPlugin({ + readFile: async () => + JSON.stringify({ + name: "widget-lib", + exports: { + ".": "./index.js", + "./client/button": "./client/button.js", + }, + }), + }); + const subpathResolveId = (subpathPlugin.resolveId as any).handler; + const ctx = createContext("client"); + const result = await subpathResolveId.call( + ctx, + "/project/node_modules/widget-lib/client/button.js", + "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", + ); + + expect(result).toBe("\0vinext:dedup/widget-lib/client/button"); + }); + + it("keeps blocked private internals on the package root", async () => { + const privateInternalPlugin = clientReferenceDedupPlugin({ + readFile: async () => + JSON.stringify({ + name: "fake-context-lib", + exports: { + ".": "./index.js", + }, + }), + }); + const privateInternalResolveId = (privateInternalPlugin.resolveId as any).handler; + const ctx = createContext("client"); + const result = await privateInternalResolveId.call( + ctx, + "/project/node_modules/fake-context-lib/internal/context.js", + "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", + ); + + expect(result).toBe("\0vinext:dedup/fake-context-lib"); + }); + + it("skips non-client environments", async () => { const ctx = createContext("rsc"); - const result = resolveId.call( + const result = await resolveId.call( ctx, "/project/node_modules/@mantine/core/esm/MantineProvider.mjs", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", @@ -74,9 +252,9 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBeUndefined(); }); - it("skips imports not from proxy modules", () => { + it("skips imports not from proxy modules", async () => { const ctx = createContext("client"); - const result = resolveId.call( + const result = await resolveId.call( ctx, "/project/node_modules/@mantine/core/esm/MantineProvider.mjs", "/project/src/App.tsx", @@ -84,9 +262,9 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBeUndefined(); }); - it("skips non-absolute paths", () => { + it("skips non-absolute paths", async () => { const ctx = createContext("client"); - const result = resolveId.call( + const result = await resolveId.call( ctx, "@mantine/core", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", @@ -94,9 +272,9 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBeUndefined(); }); - it("skips paths without node_modules", () => { + it("skips paths without node_modules", async () => { const ctx = createContext("client"); - const result = resolveId.call( + const result = await resolveId.call( ctx, "/project/src/components/Foo.tsx", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", @@ -104,13 +282,13 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBeUndefined(); }); - it("skips when importer is undefined", () => { + it("skips when importer is undefined", async () => { const ctx = createContext("client"); - const result = resolveId.call(ctx, "/project/node_modules/react/index.js", undefined); + const result = await resolveId.call(ctx, "/project/node_modules/react/index.js", undefined); expect(result).toBeUndefined(); }); - it("respects optimizeDeps.exclude from resolved config", () => { + it("respects optimizeDeps.exclude from resolved config", async () => { const excludePlugin = clientReferenceDedupPlugin(); // Simulate configResolved with @mantine/core excluded (excludePlugin.configResolved as any)({ @@ -121,7 +299,7 @@ describe("clientReferenceDedupPlugin", () => { }); const excludeResolveId = (excludePlugin.resolveId as any).handler; const ctx = createContext("client"); - const result = excludeResolveId.call( + const result = await excludeResolveId.call( ctx, "/project/node_modules/@mantine/core/esm/MantineProvider.mjs", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123", @@ -129,14 +307,14 @@ describe("clientReferenceDedupPlugin", () => { expect(result).toBeUndefined(); }); - it("falls back to top-level optimizeDeps.exclude", () => { + it("falls back to top-level optimizeDeps.exclude", async () => { const excludePlugin = clientReferenceDedupPlugin(); (excludePlugin.configResolved as any)({ optimizeDeps: { exclude: ["some-pkg"] }, }); const excludeResolveId = (excludePlugin.resolveId as any).handler; const ctx = createContext("client"); - const result = excludeResolveId.call( + const result = await excludeResolveId.call( ctx, "/project/node_modules/some-pkg/dist/index.mjs", "\0virtual:vite-rsc/client-in-server-package-proxy/abc123",