diff --git a/packages/vinext/src/plugins/optimize-imports.ts b/packages/vinext/src/plugins/optimize-imports.ts index 4c7b1757f2..aaef84d029 100644 --- a/packages/vinext/src/plugins/optimize-imports.ts +++ b/packages/vinext/src/plugins/optimize-imports.ts @@ -49,6 +49,17 @@ type BarrelExportEntry = { source: string; isNamespace: boolean; originalName?: string; + /** + * The outermost module on the way to `source` that RSC replaces with + * references or wrappers because of its directive. RSC must import the name + * from it instead. + */ + rscBoundary?: string; + /** + * The same for SSR, where only "use server" and "use cache" modules are + * replaced with references. + */ + ssrBoundary?: string; }; type BarrelExportMap = Map; @@ -398,7 +409,7 @@ async function buildExportMapFromFile( } const exportMap: BarrelExportMap = new Map(); - const isBoundary = findBoundaryDirective(ast) !== null; + const boundaryDirective = findBoundaryDirective(ast); // Track import bindings: local name → { source, isNamespace, originalName } const importBindings = new Map< @@ -614,11 +625,17 @@ async function buildExportMapFromFile( } } - if (isBoundary) { - // Every name a boundary module exports must be imported from the module - // itself, including names reached through `export *` into or out of it. - for (const name of exportMap.keys()) { - exportMap.set(name, { source: filePath, isNamespace: false, originalName: name }); + if (boundaryDirective) { + // Record this module as the boundary for every name it exports, including + // names reached through `export *` into or out of it. Outer modules + // override inner ones, since an importer reaches them first. + const isSsrBoundary = boundaryDirective !== "use client"; + for (const [name, entry] of exportMap) { + exportMap.set(name, { + ...entry, + rscBoundary: filePath, + ssrBoundary: isSsrBoundary ? filePath : entry.ssrBoundary, + }); } } @@ -759,14 +776,12 @@ export function createOptimizeImportsPlugin( // In pnpm strict mode, sub-packages like @radix-ui/react-slot are only // resolvable from the barrel package's location, not from user code. // Use Vite's own resolver (not createRequire) so it picks the ESM entry. - // subpkgOrigin is keyed by environment; prefer the current env's map but - // fall back to the other env's map for the case where only one environment - // has transformed files that import from a given barrel (e.g. a barrel - // only reachable from the RSC graph may still need resolving from SSR). - const envName = (this as PluginCtx).environment?.name ?? "ssr"; - const barrelEntry = - barrelCaches.subpkgOrigin.get(envName)?.get(source) ?? - barrelCaches.subpkgOrigin.get(envName === "rsc" ? "ssr" : "rsc")?.get(source); + // subpkgOrigin is keyed like the transform below, which registers every + // specifier it emits in its own environment's map. The other environment's + // map may hold specifiers this one never emits, such as packages behind a + // "use client" boundary in RSC. + const envKey = (this as PluginCtx).environment?.name === "rsc" ? "rsc" : "ssr"; + const barrelEntry = barrelCaches.subpkgOrigin.get(envKey)?.get(source); if (!barrelEntry) return; const resolved = await this.resolve(source, barrelEntry, { skipSelf: true }); return resolved ?? undefined; @@ -852,6 +867,13 @@ export function createOptimizeImportsPlugin( barrelCaches.exportMapCache, ); if (!exportMap || !barrelEntry) continue; + // The module to import a name from in this environment, stopping at + // the module that this environment replaces with references. + const getEntry = (name: string): BarrelExportEntry | undefined => { + const entry = exportMap.get(name); + const boundary = preferReactServer ? entry?.rscBoundary : entry?.ssrBoundary; + return boundary ? { source: boundary, isNamespace: false, originalName: name } : entry; + }; // Register sub-package sources so resolveId can find them from // the barrel's context (needed for pnpm strict hoisting). @@ -876,16 +898,15 @@ export function createOptimizeImportsPlugin( envOriginMap = new Map(); barrelCaches.subpkgOrigin.set(envKey, envOriginMap); } - for (const entry of exportMap.values()) { - if ( - !entry.source.startsWith("/") && - !entry.source.startsWith(".") && - !envOriginMap.has(entry.source) - ) { + for (const name of exportMap.keys()) { + // Only the sources this environment can emit, so a package behind + // a boundary isn't registered. + const source = getEntry(name)!.source; + if (!source.startsWith("/") && !source.startsWith(".") && !envOriginMap.has(source)) { // First barrel to register this specifier (within this environment) wins. // Sub-package specifiers are keyed per environment so that RSC and SSR // barrel entries don't cross-contaminate each other's resolution context. - envOriginMap.set(entry.source, barrelEntry); + envOriginMap.set(source, barrelEntry); } } } @@ -952,7 +973,7 @@ export function createOptimizeImportsPlugin( // name gains nothing from a rewrite. if ( isSubpath && - specifiers.every(({ imported }) => exportMap.get(imported)?.source === barrelEntry) + specifiers.every(({ imported }) => getEntry(imported)?.source === barrelEntry) ) { continue; } @@ -962,7 +983,7 @@ export function createOptimizeImportsPlugin( const resolvedPackageSources = new Map(); if (isSubpath) { for (const { imported } of specifiers) { - const source = exportMap.get(imported)?.source; + const source = getEntry(imported)?.source; if (!source || path.isAbsolute(source) || resolvedPackageSources.has(source)) continue; const resolvedId = await resolveFileId(this, source, barrelEntry); @@ -985,7 +1006,7 @@ export function createOptimizeImportsPlugin( } >(); for (const { local, imported } of specifiers) { - const entry = exportMap.get(imported); + const entry = getEntry(imported); if (!entry) continue; // Sources in the export map are already absolute paths (for file references) // or bare package specifiers — no further resolution needed. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 08a2d8d51b..fb10597796 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1451,6 +1451,9 @@ importers: '@vitejs/plugin-rsc': specifier: 'catalog:' version: 0.5.38(@voidzero-dev/vite-plus-core@1.1.0(@types/node@25.9.2)(esbuild@0.28.1)(jiti@2.7.0)(sass@1.100.0)(tsx@4.21.1)(typescript@7.0.2)(yaml@2.9.0))(react-dom@19.3.1(react@19.3.1))(react-server-dom-webpack@19.3.1(react-dom@19.3.1(react@19.3.1))(react@19.3.1))(react@19.3.1) + fake-client-barrel: + specifier: file:./__test_packages__/fake-client-barrel + version: file:tests/fixtures/app-basic/__test_packages__/fake-client-barrel fake-context-lib: specifier: file:./__test_packages__/fake-context-lib version: file:tests/fixtures/app-basic/__test_packages__/fake-context-lib @@ -8458,6 +8461,9 @@ packages: extendable-error@0.1.7: resolution: {integrity: sha512-UOiS2in6/Q0FK0R0q6UY9vYpQ21mr/Qn1KOnte7vsACuNJf514WvCCUHSRCPcgjPT2bAhNIJdlE6bVap1GKmeg==} + fake-client-barrel@file:tests/fixtures/app-basic/__test_packages__/fake-client-barrel: + resolution: {directory: tests/fixtures/app-basic/__test_packages__/fake-client-barrel, type: directory} + fake-context-lib@file:tests/fixtures/app-basic/__test_packages__/fake-context-lib: resolution: {directory: tests/fixtures/app-basic/__test_packages__/fake-context-lib, type: directory} @@ -16933,6 +16939,8 @@ snapshots: extendable-error@0.1.7: {} + fake-client-barrel@file:tests/fixtures/app-basic/__test_packages__/fake-client-barrel: {} + fake-context-lib@file:tests/fixtures/app-basic/__test_packages__/fake-context-lib: {} fake-css-lib@file:tests/fixtures/app-basic/__test_packages__/fake-css-lib: {} diff --git a/tests/after-response-close-worker.test.ts b/tests/after-response-close-worker.test.ts index aea6b9bee9..9ac73f94ca 100644 --- a/tests/after-response-close-worker.test.ts +++ b/tests/after-response-close-worker.test.ts @@ -57,10 +57,11 @@ describe("closeAfterResponseWithBody on the Cloudflare Workers runtime", () => { // createIsolatedFixture swaps in a plain workspace node_modules symlink // (here, cf-app-basic's, for @cloudflare/vite-plugin + wrangler), which // drops app-basic's own `file:./__test_packages__/*` local packages. - // Exclude the four routes that depend on those — unrelated to this + // Exclude the routes that depend on those — unrelated to this // regression — so the rest of the fixture still builds. (src) => !src.includes(`${path.sep}app${path.sep}context-dedup-test`) && + !src.includes(`${path.sep}app${path.sep}optimize-imports-client-barrel`) && !src.includes(`${path.sep}app${path.sep}nextjs-compat${path.sep}node-modules-css`) && !src.includes( `${path.sep}app${path.sep}nextjs-compat${path.sep}dynamic${path.sep}out-of-root-package`, diff --git a/tests/e2e/app-router/optimize-imports-client-barrel.spec.ts b/tests/e2e/app-router/optimize-imports-client-barrel.spec.ts new file mode 100644 index 0000000000..119501c47e --- /dev/null +++ b/tests/e2e/app-router/optimize-imports-client-barrel.spec.ts @@ -0,0 +1,24 @@ +import { test, expect } from "@playwright/test"; +import { waitForAppRouterHydration } from "../helpers"; + +const BASE = "http://localhost:4174"; + +// fake-client-barrel is in the fixture's optimizePackageImports. Its entry is a +// "use client" barrel, and its unused leaf sets a global when it is evaluated. +test.describe('optimizePackageImports with a "use client" barrel', () => { + test("SSR only evaluates the modules that define the imported names", async ({ request }) => { + const res = await request.get(`${BASE}/optimize-imports-client-barrel`); + expect(res.status()).toBe(200); + const html = await res.text(); + expect(html).toContain('data-testid="client-barrel-counter"'); + expect(html).toContain('data-unused-loaded="false"'); + }); + + test("the component hydrates", async ({ page }) => { + await page.goto(`${BASE}/optimize-imports-client-barrel`); + await waitForAppRouterHydration(page); + const counter = page.getByTestId("client-barrel-counter"); + await counter.click(); + await expect(counter).toHaveText("Count: 1"); + }); +}); diff --git a/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/counter.js b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/counter.js new file mode 100644 index 0000000000..7529e54d0d --- /dev/null +++ b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/counter.js @@ -0,0 +1,16 @@ +import { createElement, useState } from "react"; + +export function Counter() { + const [count, setCount] = useState(0); + return createElement( + "button", + { + "data-testid": "client-barrel-counter", + // Hydration can't match: the client loads the whole barrel. + "data-unused-loaded": String(globalThis.__fakeClientBarrelUnusedLoaded === true), + suppressHydrationWarning: true, + onClick: () => setCount((value) => value + 1), + }, + `Count: ${count}`, + ); +} diff --git a/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/index.js b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/index.js new file mode 100644 index 0000000000..a94604604d --- /dev/null +++ b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/index.js @@ -0,0 +1,5 @@ +"use client"; + +// A "use client" barrel, like antd's entry. +export { Counter } from "./counter.js"; +export { Unused } from "./unused.js"; diff --git a/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/package.json b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/package.json new file mode 100644 index 0000000000..8cc882913e --- /dev/null +++ b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/package.json @@ -0,0 +1,11 @@ +{ + "name": "fake-client-barrel", + "version": "1.0.0", + "private": true, + "type": "module", + "sideEffects": false, + "main": "index.js", + "exports": { + ".": "./index.js" + } +} diff --git a/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/unused.js b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/unused.js new file mode 100644 index 0000000000..003fe1da5b --- /dev/null +++ b/tests/fixtures/app-basic/__test_packages__/fake-client-barrel/unused.js @@ -0,0 +1,5 @@ +globalThis.__fakeClientBarrelUnusedLoaded = true; + +export function Unused() { + return null; +} diff --git a/tests/fixtures/app-basic/app/optimize-imports-client-barrel/page.tsx b/tests/fixtures/app-basic/app/optimize-imports-client-barrel/page.tsx new file mode 100644 index 0000000000..74037ed77d --- /dev/null +++ b/tests/fixtures/app-basic/app/optimize-imports-client-barrel/page.tsx @@ -0,0 +1,7 @@ +"use client"; + +import { Counter } from "fake-client-barrel"; + +export default function OptimizeImportsClientBarrelPage() { + return ; +} diff --git a/tests/fixtures/app-basic/next.config.ts b/tests/fixtures/app-basic/next.config.ts index b534d8177f..ea25e3300e 100644 --- a/tests/fixtures/app-basic/next.config.ts +++ b/tests/fixtures/app-basic/next.config.ts @@ -32,6 +32,9 @@ const nextConfig: NextConfig = { // don't assert the method's absence against this fixture. experimental: { gestureTransition: true, + // Used by E2E: optimize-imports-client-barrel.spec.ts — a "use client" + // barrel whose imports SSR rewrites to the modules that define them. + optimizePackageImports: ["fake-client-barrel"], }, // Used by Vitest: app-router-production-server.test.ts — a next/dynamic() diff --git a/tests/fixtures/app-basic/package.json b/tests/fixtures/app-basic/package.json index 7de39b89ca..6ceaef0650 100644 --- a/tests/fixtures/app-basic/package.json +++ b/tests/fixtures/app-basic/package.json @@ -4,6 +4,7 @@ "type": "module", "dependencies": { "@vitejs/plugin-rsc": "catalog:", + "fake-client-barrel": "file:./__test_packages__/fake-client-barrel", "fake-context-lib": "file:./__test_packages__/fake-context-lib", "fake-css-lib": "file:./__test_packages__/fake-css-lib", "fake-css-module-lib": "file:./__test_packages__/fake-css-module-lib", diff --git a/tests/optimize-imports.test.ts b/tests/optimize-imports.test.ts index 5ac33b04ba..4d4e49a053 100644 --- a/tests/optimize-imports.test.ts +++ b/tests/optimize-imports.test.ts @@ -391,28 +391,47 @@ export { Mo as Listbox };`; expect(map!.get("parse")).toBeDefined(); }); - it("does not follow re-exports through a directive boundary", async () => { + it("records the directive boundaries that re-exports pass through", async () => { const entryPath = "/fake/wildcard-boundary/index.js"; const files: Record = { - [entryPath]: `export * from "./client.js";\nexport { Button } from "./button.js";`, - "/fake/wildcard-boundary/client.js": `"use client";\nexport { Dialog } from "./dialog.js";\nexport * as Icons from "./icons.js";`, + [entryPath]: `export * from "./client.js";\nexport * from "./actions.js";\nexport { Button } from "./button.js";`, + "/fake/wildcard-boundary/client.js": `"use client";\nexport { Dialog } from "./dialog.js";\nexport * as Icons from "./icons.js";\nexport * from "./client-actions.js";`, + "/fake/wildcard-boundary/client-actions.js": `"use server";\nexport { save } from "./save.js";`, + "/fake/wildcard-boundary/actions.js": `"use server";\nexport { submit } from "./submit.js";`, }; const map = await buildBarrelExportMap(entryPath, (fp) => Promise.resolve(files[fp] ?? null)); - // Names exported by the "use client" module are imported from it, so the - // client reference boundary is kept. + // RSC stops at any directive; SSR only at "use server" and "use cache". expect(map!.get("Dialog")).toEqual({ - source: "/fake/wildcard-boundary/client.js", + source: "/fake/wildcard-boundary/dialog.js", isNamespace: false, originalName: "Dialog", + rscBoundary: "/fake/wildcard-boundary/client.js", + ssrBoundary: undefined, }); expect(map!.get("Icons")).toEqual({ - source: "/fake/wildcard-boundary/client.js", + source: "/fake/wildcard-boundary/icons.js", + isNamespace: true, + rscBoundary: "/fake/wildcard-boundary/client.js", + ssrBoundary: undefined, + }); + // The outermost boundary wins for each environment. + expect(map!.get("save")).toMatchObject({ + source: "/fake/wildcard-boundary/save.js", + rscBoundary: "/fake/wildcard-boundary/client.js", + ssrBoundary: "/fake/wildcard-boundary/client-actions.js", + }); + expect(map!.get("submit")).toMatchObject({ + source: "/fake/wildcard-boundary/submit.js", + rscBoundary: "/fake/wildcard-boundary/actions.js", + ssrBoundary: "/fake/wildcard-boundary/actions.js", + }); + expect(map!.get("Button")).toEqual({ + source: "/fake/wildcard-boundary/button.js", isNamespace: false, - originalName: "Icons", + originalName: "Button", }); - expect(map!.get("Button")!.source).toBe("/fake/wildcard-boundary/button.js"); }); it("does not overwrite existing exports when wildcard sub-module has same name", async () => { @@ -632,7 +651,9 @@ describe("vinext:optimize-imports transform", () => { async function setupTransform( packageName: string, barrelContents: string, - ): Promise<(code: string, id: string) => Promise any>>> { + ): Promise< + (code: string, id: string, envName?: string) => Promise any>> + > { // Create tmp project with a fake package in node_modules. // The package name must be in DEFAULT_OPTIMIZE_PACKAGES (or configured via // next.config.js) for the plugin's buildStart to include it. @@ -662,9 +683,9 @@ describe("vinext:optimize-imports transform", () => { if (buildStartHook) await buildStartHook.call(plugin); const transform = unwrapHook(plugin.transform)!; - // Return a caller that fakes the environment context as RSC (server) - return async (code: string, id: string) => - await (transform as any).call({ ...plugin, environment: { name: "rsc" } }, code, id); + // Return a caller that fakes the environment context, RSC by default + return async (code: string, id: string, envName = "rsc") => + await (transform as any).call({ ...plugin, environment: { name: envName } }, code, id); } afterEach(() => { @@ -806,6 +827,78 @@ describe("vinext:optimize-imports transform", () => { expect(importLines[0]).toContain(expected); }); + it('stops at a "use client" entry in RSC but not in SSR', async () => { + // antd is in DEFAULT_OPTIMIZE_PACKAGES, and its entry starts with "use client". + const call = await setupTransform("antd", `"use client";\nexport { Button } from "./button";`); + const pkgDir = path.posix.join(tmpDir, "node_modules", "antd"); + const code = `import { Button } from "antd";`; + const rscResult = await call(code, "/app/page.tsx", "rsc"); + expect(rscResult!.code).toContain( + `import { Button } from ${JSON.stringify(path.posix.join(pkgDir, "index.js"))}`, + ); + + const ssrResult = await call(code, "/app/page.tsx", "ssr"); + expect(ssrResult!.code).toContain( + `import { Button } from ${JSON.stringify(path.posix.join(pkgDir, "button"))}`, + ); + }); + + it("only registers the packages this environment imports for resolveId", async () => { + tmpDir = toSlash( + fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "vinext-optimize-test-"))), + ); + fs.writeFileSync( + path.join(tmpDir, "package.json"), + JSON.stringify({ name: "test-app", type: "module" }), + ); + const pkgDir = path.join(tmpDir, "node_modules", "antd"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: "antd", type: "module", main: "./index.js" }), + ); + fs.writeFileSync( + path.join(pkgDir, "index.js"), + `"use client";\nexport { Slot } from "@radix-ui/react-slot";`, + ); + + const plugin = createOptimizeImportsPlugin( + () => undefined, + () => tmpDir, + ) as Plugin; + const buildStartHook = unwrapHook((plugin as any).buildStart); + if (buildStartHook) await buildStartHook.call(plugin); + const transform = unwrapHook(plugin.transform)!; + const resolveId = unwrapHook(plugin.resolveId)!; + const rsc = { ...plugin, environment: { name: "rsc" }, resolve: vi.fn() }; + const ssr = { ...plugin, environment: { name: "ssr" }, resolve: vi.fn() }; + + // SSR imports Slot from @radix-ui/react-slot, but RSC imports it from the + // "use client" entry, whichever environment transforms first. + for (const context of [ssr, rsc]) { + await (transform as any).call(context, `import { Slot } from "antd";`, "/app/page.tsx"); + await (resolveId as any).call(context, "@radix-ui/react-slot", "/app/other.tsx"); + } + expect(ssr.resolve).toHaveBeenCalledOnce(); + expect(rsc.resolve).not.toHaveBeenCalled(); + }); + + it('stops at "use server" modules in SSR', async () => { + const call = await setupTransform("antd", `export * from "./actions.js";`); + const pkgDir = path.posix.join(tmpDir, "node_modules", "antd"); + fs.writeFileSync( + path.join(pkgDir, "actions.js"), + `"use server";\nexport { submit } from "./submit.js";`, + ); + const code = `import { submit } from "antd";`; + for (const envName of ["rsc", "ssr"]) { + const result = await call(code, "/app/page.tsx", envName); + expect(result!.code).toContain( + `import { submit } from ${JSON.stringify(path.posix.join(pkgDir, "actions.js"))}`, + ); + } + }); + it("produces separate statements for namespace and named imports from the same source", async () => { // lodash-es is in DEFAULT_OPTIMIZE_PACKAGES. // The barrel exports both a namespace re-export and a named re-export pointing at @@ -1158,7 +1251,8 @@ describe("vinext:optimize-imports subpath imports", () => { return { call, racDir, nestedStately, resolveCalls }; } - const heroToastQueue = `${"/app"}/node_modules/@heroui/react/dist/components/toast/toast-queue.js?v=abc`; + const heroToastQueue = + "/app/node_modules/@heroui/react/dist/components/toast/toast-queue.js?v=abc"; it("optimizes react-aria-components subpaths by default", () => { expect(SUBPATH_OPTIMIZE_PACKAGES).toContain("react-aria-components");