From 192f2876b25be3faf4e7be65dacf0efdee0322d5 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 15 Oct 2026 12:01:48 +0100 Subject: [PATCH] fix(rsc): reject client-only only when it survives RSC tree-shaking @vitejs/plugin-rsc's rsc:validate-imports checks the client-only import chain in buildEnd, before tree-shaking, so a Server Component importing one export from a sideEffects:false barrel failed the build when an unused leaf imported client-only (e.g. @heroui/react -> react-aria Calendar). Claim client-only in the RSC build and fail only when the marker module is still in the emitted RSC chunks, matching Turbopack, which reports the client-only import error only for modules kept in the tree-shaken graph. Fixes #4007 --- .gitignore | 2 + packages/vinext/src/index.ts | 9 + .../src/plugins/rsc-client-only-imports.ts | 102 ++++++ tests/client-only-tree-shaking.test.ts | 299 ++++++++++++++++++ .../client-only-barrel.spec.ts | 11 + .../app/client-only-barrel/page.tsx | 9 + .../node_modules/client-only-barrel/index.js | 2 + .../node_modules/client-only-barrel/label.js | 3 + .../client-only-barrel/package.json | 8 + .../node_modules/client-only-barrel/widget.js | 5 + 10 files changed, 450 insertions(+) create mode 100644 packages/vinext/src/plugins/rsc-client-only-imports.ts create mode 100644 tests/client-only-tree-shaking.test.ts create mode 100644 tests/e2e/cloudflare-encoded-paths/client-only-barrel.spec.ts create mode 100644 tests/fixtures/cf-app-basic/app/client-only-barrel/page.tsx create mode 100644 tests/fixtures/cf-app-basic/node_modules/client-only-barrel/index.js create mode 100644 tests/fixtures/cf-app-basic/node_modules/client-only-barrel/label.js create mode 100644 tests/fixtures/cf-app-basic/node_modules/client-only-barrel/package.json create mode 100644 tests/fixtures/cf-app-basic/node_modules/client-only-barrel/widget.js diff --git a/.gitignore b/.gitignore index e7461e9044..3753c9c222 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ tests/e2e/cloudflare-workers/fixtures/client-module-with-package-type/node_modul tests/fixtures/cf-app-basic/node_modules/* !tests/fixtures/cf-app-basic/node_modules/module-identity-dependency/ !tests/fixtures/cf-app-basic/node_modules/module-identity-dependency/** +!tests/fixtures/cf-app-basic/node_modules/client-only-barrel/ +!tests/fixtures/cf-app-basic/node_modules/client-only-barrel/** dist/ !packages/types/next/upstream/dist/ !packages/types/next/upstream/dist/** diff --git a/packages/vinext/src/index.ts b/packages/vinext/src/index.ts index aeb82e4221..809156bb6a 100644 --- a/packages/vinext/src/index.ts +++ b/packages/vinext/src/index.ts @@ -230,6 +230,7 @@ import { createCssModuleImportCompatibilityPlugin } from "./plugins/css-module-i import { createRscClientReferenceLoadersPlugin } from "./plugins/rsc-client-reference-loaders.js"; import { createRscReferenceValidationNormalizerPlugin } from "./plugins/rsc-reference-validation-normalizer.js"; import { createScanBuildCssPlugin } from "./plugins/scan-build-css.js"; +import { createRscClientOnlyImportsPlugin } from "./plugins/rsc-client-only-imports.js"; import { createInstrumentationClientTransformPlugin, createInstrumentationServerTransformPlugin, @@ -8651,6 +8652,14 @@ export const loadServerActionClient = ${ }), ); plugins.push(createScanBuildCssPlugin()); + plugins.push( + createRscClientOnlyImportsPlugin({ + async getManager(config) { + const rscPluginModule = await rscPluginModulePromise; + return rscPluginModule?.getPluginApi(config)?.manager; + }, + }), + ); } if (rscPluginPromise) { plugins.push(createRscReferenceValidationNormalizerPlugin()); diff --git a/packages/vinext/src/plugins/rsc-client-only-imports.ts b/packages/vinext/src/plugins/rsc-client-only-imports.ts new file mode 100644 index 0000000000..438a58ad1b --- /dev/null +++ b/packages/vinext/src/plugins/rsc-client-only-imports.ts @@ -0,0 +1,102 @@ +import type { RscPluginManager } from "@vitejs/plugin-rsc"; +import type { Plugin, ResolvedConfig } from "vite"; +import path from "pathslash"; + +const CLIENT_ONLY_SPECIFIER_RE = /^client-only$/; +const INVALID_CLIENT_ONLY_ID = "\0virtual:vinext-invalid-client-only"; +const INVALID_CLIENT_ONLY_ID_RE = /^\0virtual:vinext-invalid-client-only$/; + +/** + * Reject `client-only` in the RSC build only when it survives tree-shaking. + * + * @vitejs/plugin-rsc's `rsc:validate-imports` checks its `client-only` import + * chain in `buildEnd`, before tree-shaking. A Server Component that imports + * one export from a `sideEffects: false` barrel then fails the build when an + * unused leaf of that barrel imports `client-only`, even though the bundler + * drops the leaf and its marker from the output. + * + * Next.js (Turbopack) maps `client-only` in the server graph to an import + * error that is only reported for modules that stay in the tree-shaken + * module graph: + * crates/next-core/src/next_import_map.rs (insert_client_only_error_alias) + * (Next.js's webpack build still rejects these barrels before tree-shaking.) + * + * This plugin claims the RSC build's `client-only` resolutions before + * plugin-rsc does, and checks the emitted RSC chunks instead. The marker + * module has side effects, so it stays in the output whenever any module that + * imports it is retained, including direct imports from application code. + * Dev keeps plugin-rsc's resolution-time check because Vite dev does not + * tree-shake. + */ +export function createRscClientOnlyImportsPlugin(options: { + getManager: (config: ResolvedConfig) => Promise; +}): Plugin { + let enabled = false; + + return { + name: "vinext:rsc-client-only-imports", + apply: "build", + // Ahead of plugin-rsc's `order: "pre"` resolveId hook. + enforce: "pre", + configResolved(resolvedConfig) { + // Respect `rsc({ validateImports: false })` in a manual plugin-rsc setup. + enabled = resolvedConfig.plugins.some((plugin) => plugin.name === "rsc:validate-imports"); + }, + resolveId: { + order: "pre", + filter: { id: CLIENT_ONLY_SPECIFIER_RE }, + handler() { + if (!enabled || this.environment.name !== "rsc") return; + return { id: INVALID_CLIENT_ONLY_ID, moduleSideEffects: true }; + }, + }, + load: { + filter: { id: INVALID_CLIENT_ONLY_ID_RE }, + handler() { + return `throw new Error("'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.");`; + }, + }, + async generateBundle(_outputOptions, bundle) { + if (!enabled || this.environment.name !== "rsc") return; + // The scan build reduces every module to its imports, so nothing is + // tree-shaken there. The real RSC build that follows does the check. + if ((await options.getManager(this.environment.config))?.isScanBuild) return; + + const retained = new Set(); + for (const output of Object.values(bundle)) { + if (output.type !== "chunk") continue; + for (const id of output.moduleIds) retained.add(id); + } + if (!retained.has(INVALID_CLIENT_ONLY_ID)) return; + + // Report a chain through modules that are in the output, so the error + // names the import that kept the marker rather than a dropped one. + const chain: string[] = []; + let current: string | undefined = INVALID_CLIENT_ONLY_ID; + while (current && !chain.includes(current)) { + chain.push(current); + const info = this.getModuleInfo(current); + const importers = info ? [...info.importers, ...info.dynamicImporters] : []; + current = importers.find((id) => retained.has(id) && !chain.includes(id)) ?? importers[0]; + } + throw createClientOnlyImportError(chain, this.environment.config.root); + }, + }; +} + +// Same message shape as plugin-rsc's `rsc:validate-imports` so dev and build +// report the import chain the same way. +function createClientOnlyImportError(chain: string[], root: string): Error { + let message = `'client-only' cannot be imported in server build ('rsc' environment):\n`; + message += chain + .slice(1, 6) + .map( + (id, i) => + `${" ".repeat(i + 1)}imported by ${path.relative(root, id).replaceAll("\0", "")}\n`, + ) + .join(""); + if (chain.length > 6) message += `${" ".repeat(7)}...\n`; + const error = new Error(message); + if (chain[1]) Object.assign(error, { id: chain[1] }); + return error; +} diff --git a/tests/client-only-tree-shaking.test.ts b/tests/client-only-tree-shaking.test.ts new file mode 100644 index 0000000000..05798f661b --- /dev/null +++ b/tests/client-only-tree-shaking.test.ts @@ -0,0 +1,299 @@ +/** + * Build-time integration test: a `client-only` marker that tree-shaking drops + * from the RSC graph must not fail the build. + * + * Regression for https://github.com/cloudflare/vinext/issues/4007. A Server + * Component imported one component from a `sideEffects: false` barrel + * (`@heroui/react`). An unused leaf of that barrel (react-aria-components' + * `Calendar`) imports `client-only`, and @vitejs/plugin-rsc's + * `rsc:validate-imports` walked the pre-tree-shaking module graph, so the + * build failed even though the marker never reached the output. + * + * Next.js (Turbopack) resolves `client-only` in the server graph to an import + * error that is only reported when the importing module stays in the + * tree-shaken module graph: + * crates/next-core/src/next_import_map.rs (insert_client_only_error_alias) + */ +import fs from "node:fs"; +import path from "node:path"; +import { createBuilder } from "vite"; +import { afterAll, describe, expect, it } from "vite-plus/test"; +import vinext from "../packages/vinext/src/index.js"; + +const tmpRoots: string[] = []; + +afterAll(() => { + for (const root of tmpRoots) fs.rmSync(root, { recursive: true, force: true }); +}); + +function writeFile(file: string, source: string): void { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, source); +} + +type FixtureOptions = { + pageSource: string; + /** `sideEffects` field of the barrel package. Omitted when undefined. */ + sideEffects?: boolean; +}; + +/** + * Mirrors the graph from the issue: + * + * ui-barrel (index.js) ── re-exports ──> button.js (server-safe, used) + * ├─ re-exports ──> calendar.js (imports "client-only") + * └─ re-exports ──> toast.js ──> toast-queue.js + * │ imports a binding + * ▼ via "ui-primitives/toast" + * ui-primitives (sideEffects: ["*.css"]) toast.js ("client-only" + re-export + * of queue.js's binding) + * + * `ui-primitives/toast` is the react-aria-components `Toast.mjs` shape: a + * module in a package with a glob `sideEffects` list that imports + * `client-only` but only re-exports a binding from elsewhere, so the marker + * module is dropped even though the binding is used. + * + * `ui-barrel/legacy` is a separate CommonJS entry that requires `client-only`, + * used only by the CommonJS case. + */ +function createFixture({ pageSource, sideEffects }: FixtureOptions): string { + // Inside the repository so react, react-dom, and @vitejs/plugin-rsc resolve + // from the workspace node_modules while the fixture's own node_modules + // holds the barrel package. + const root = fs.mkdtempSync(path.join(import.meta.dirname, ".tmp-client-only-tree-shaking-")); + tmpRoots.push(root); + + writeFile( + path.join(root, "package.json"), + JSON.stringify({ name: "client-only-tree-shaking", private: true, type: "module" }), + ); + writeFile( + path.join(root, "app", "layout.tsx"), + `export default function RootLayout({ children }: { children: React.ReactNode }) { + return {children}; +} +`, + ); + writeFile(path.join(root, "app", "page.tsx"), pageSource); + + const pkg = path.join(root, "node_modules", "ui-barrel"); + writeFile( + path.join(pkg, "package.json"), + JSON.stringify({ + name: "ui-barrel", + version: "1.0.0", + type: "module", + exports: { ".": "./index.js", "./legacy": "./legacy.cjs" }, + ...(sideEffects === undefined ? {} : { sideEffects }), + }), + ); + writeFile( + path.join(pkg, "index.js"), + `export { Button } from "./button.js"; +export { Calendar } from "./calendar.js"; +export { Toast } from "./toast.js"; +`, + ); + writeFile( + path.join(pkg, "legacy.cjs"), + `require("client-only"); +exports.legacyLabel = "legacy"; +`, + ); + writeFile( + path.join(pkg, "button.js"), + `import { createElement } from "react"; +export function Button(props) { + return createElement("button", { "data-testid": "barrel-button" }, props.children); +} +`, + ); + writeFile( + path.join(pkg, "calendar.js"), + `import "client-only"; +import { createElement } from "react"; +export function Calendar() { + return createElement("div", null, "calendar"); +} +`, + ); + const primitives = path.join(root, "node_modules", "ui-primitives"); + writeFile( + path.join(primitives, "package.json"), + JSON.stringify({ + name: "ui-primitives", + version: "1.0.0", + type: "module", + exports: { "./toast": "./toast.js" }, + sideEffects: ["*.css"], + }), + ); + writeFile( + path.join(primitives, "queue.js"), + `export class Queue { + constructor() { + this.items = []; + } +} +`, + ); + writeFile( + path.join(primitives, "toast.js"), + `import { Queue as ToastQueue } from "./queue.js"; +import "client-only"; +export { ToastQueue as Queue }; +`, + ); + writeFile( + path.join(pkg, "toast-queue.js"), + `import { Queue } from "ui-primitives/toast"; +export const toastQueue = new Queue(); +`, + ); + writeFile( + path.join(pkg, "toast.js"), + `import { createElement } from "react"; +import { toastQueue } from "./toast-queue.js"; +function Provider() { + return createElement("div", { "data-testid": "toast-provider" }, String(toastQueue.items.length)); +} +export const Toast = { Provider }; +`, + ); + + return root; +} + +async function buildFixture(root: string): Promise { + const builder = await createBuilder({ + root, + configFile: false, + plugins: [vinext({ appDir: root })], + logLevel: "silent", + }); + await builder.buildApp(); +} + +async function fetchPage(root: string): Promise<{ status: number; html: string }> { + const { startProdServer } = await import("../packages/vinext/src/server/prod-server.js"); + const { server } = await startProdServer({ + port: 0, + host: "127.0.0.1", + outDir: path.join(root, "dist"), + noCompression: true, + }); + try { + const address = server.address(); + if (!address || typeof address !== "object") throw new Error("server has no port"); + const response = await fetch(`http://127.0.0.1:${address.port}/`); + return { status: response.status, html: await response.text() }; + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } +} + +describe("client-only in the RSC graph", () => { + it("builds when tree-shaking drops every client-only marker of a sideEffects:false barrel", async () => { + const root = createFixture({ + sideEffects: false, + pageSource: `import { Button, Toast } from "ui-barrel"; + +export default function Page() { + return ( +
+ + +
+ ); +} +`, + }); + + await buildFixture(root); + + const { status, html } = await fetchPage(root); + expect(status).toBe(200); + expect(html).toContain('data-testid="barrel-button"'); + expect(html).toContain('data-testid="toast-provider"'); + }, 120_000); + + it("fails when a used module of the barrel imports client-only", async () => { + const root = createFixture({ + sideEffects: false, + pageSource: `import { Calendar } from "ui-barrel"; + +export default function Page() { + return ; +} +`, + }); + + await expect(buildFixture(root)).rejects.toThrow( + /'client-only' cannot be imported in server build \('rsc' environment\)[\s\S]*ui-barrel\/calendar\.js/, + ); + }, 120_000); + + it("fails when a used CommonJS module requires client-only", async () => { + const root = createFixture({ + sideEffects: false, + pageSource: `import { legacyLabel } from "ui-barrel/legacy"; + +export default function Page() { + return
{legacyLabel}
; +} +`, + }); + + await expect(buildFixture(root)).rejects.toThrow( + /'client-only' cannot be imported in server build \('rsc' environment\)[\s\S]*ui-barrel\/legacy\.cjs/, + ); + }, 120_000); + + it("names the retained dynamic import rather than a dropped barrel leaf", async () => { + const root = createFixture({ + sideEffects: false, + pageSource: `import { Button } from "ui-barrel"; + +export default async function Page() { + await import("client-only"); + return ; +} +`, + }); + + await expect(buildFixture(root)).rejects.toThrow( + /'client-only' cannot be imported in server build \('rsc' environment\):\n imported by app\/page\.tsx\n/, + ); + }, 120_000); + + it("fails when the barrel package does not declare sideEffects: false", async () => { + const root = createFixture({ + pageSource: `import { Button } from "ui-barrel"; + +export default function Page() { + return ; +} +`, + }); + + await expect(buildFixture(root)).rejects.toThrow( + /'client-only' cannot be imported in server build \('rsc' environment\)[\s\S]*ui-barrel\/calendar\.js/, + ); + }, 120_000); + + it("fails when a Server Component imports client-only directly", async () => { + const root = createFixture({ + sideEffects: false, + pageSource: `import "client-only"; + +export default function Page() { + return
direct
; +} +`, + }); + + await expect(buildFixture(root)).rejects.toThrow( + /'client-only' cannot be imported in server build \('rsc' environment\)[\s\S]*app\/page\.tsx/, + ); + }, 120_000); +}); diff --git a/tests/e2e/cloudflare-encoded-paths/client-only-barrel.spec.ts b/tests/e2e/cloudflare-encoded-paths/client-only-barrel.spec.ts new file mode 100644 index 0000000000..54c7321157 --- /dev/null +++ b/tests/e2e/cloudflare-encoded-paths/client-only-barrel.spec.ts @@ -0,0 +1,11 @@ +import { expect, test } from "@playwright/test"; + +// Regression: https://github.com/cloudflare/vinext/issues/4007 +test("serves a Server Component that imports from a sideEffects:false barrel with a client-only leaf", async ({ + request, +}) => { + const response = await request.get("/client-only-barrel"); + + expect(response.status()).toBe(200); + expect(await response.text()).toContain("Server-safe barrel export"); +}); diff --git a/tests/fixtures/cf-app-basic/app/client-only-barrel/page.tsx b/tests/fixtures/cf-app-basic/app/client-only-barrel/page.tsx new file mode 100644 index 0000000000..93f442e3f8 --- /dev/null +++ b/tests/fixtures/cf-app-basic/app/client-only-barrel/page.tsx @@ -0,0 +1,9 @@ +import { BarrelLabel } from "client-only-barrel"; + +// Regression: https://github.com/cloudflare/vinext/issues/4007 +// `client-only-barrel` is `sideEffects: false`, and its unused `BarrelWidget` +// module imports `client-only`. Tree-shaking drops that module from the RSC +// build, so the build must not fail on the marker. +export default function ClientOnlyBarrelPage() { + return

{BarrelLabel()}

; +} diff --git a/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/index.js b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/index.js new file mode 100644 index 0000000000..019e06db69 --- /dev/null +++ b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/index.js @@ -0,0 +1,2 @@ +export { BarrelLabel } from "./label.js"; +export { BarrelWidget } from "./widget.js"; diff --git a/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/label.js b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/label.js new file mode 100644 index 0000000000..818c3830d6 --- /dev/null +++ b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/label.js @@ -0,0 +1,3 @@ +export function BarrelLabel() { + return "Server-safe barrel export"; +} diff --git a/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/package.json b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/package.json new file mode 100644 index 0000000000..2fb65d75c2 --- /dev/null +++ b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/package.json @@ -0,0 +1,8 @@ +{ + "name": "client-only-barrel", + "version": "1.0.0", + "private": true, + "type": "module", + "exports": "./index.js", + "sideEffects": false +} diff --git a/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/widget.js b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/widget.js new file mode 100644 index 0000000000..84ba6486c3 --- /dev/null +++ b/tests/fixtures/cf-app-basic/node_modules/client-only-barrel/widget.js @@ -0,0 +1,5 @@ +import "client-only"; + +export function BarrelWidget() { + return "Client-only barrel export"; +}