diff --git a/packages/plugin-rsc/e2e/validate-imports.test.ts b/packages/plugin-rsc/e2e/validate-imports.test.ts index edce8c868..9df657ef6 100644 --- a/packages/plugin-rsc/e2e/validate-imports.test.ts +++ b/packages/plugin-rsc/e2e/validate-imports.test.ts @@ -1,5 +1,7 @@ +import { stripVTControlCharacters } from 'node:util' import { test, expect } from '@playwright/test' import { x } from 'tinyexec' +import * as vite from 'vite' import { setupInlineFixture, useFixture, type Fixture } from './fixture' import { expectNoPageError, waitForHydration } from './helper' @@ -161,3 +163,392 @@ test.describe('validate imports', () => { }) }) }) + +// `client-only` / `server-only` are only invalid when the importing module +// survives tree-shaking. The packages below mirror the graph from +// https://github.com/cloudflare/vinext/issues/4007, where a server component +// imports one component from `@heroui/react`: +// +// @vitejs/test-barrel (sideEffects: false) +// |-- index.js +// | |-- button.js (server and client safe) +// | |-- calendar.js (imports "client-only") +// | |-- server-data.js (imports "server-only") +// | `-- toast.js -> toast-queue.js -> @vitejs/test-primitives/toast +// `-- legacy.cjs (requires "client-only") +// +// @vitejs/test-primitives (sideEffects: ["*.css"]) +// `-- toast.js imports "client-only" but only re-exports a binding of +// queue.js, so rolldown drops the module even when the binding +// is used (react-aria-components' `exports/Toast.mjs`). rollup +// keeps the side-effect imports of modules on a re-export path, +// so there the marker stays in the output. +test.describe('validate imports after tree-shaking', () => { + function setupTreeShakingFixture(options: { + root: string + // `sideEffects` of @vitejs/test-barrel. omitted when undefined. + sideEffects?: false + files: Record + }) { + const json = (value: unknown) => JSON.stringify(value, null, 2) + return setupInlineFixture({ + src: 'examples/starter-extra', + dest: options.root, + files: { + 'package.json': { + edit: (source) => { + const packageJson = JSON.parse(source) + packageJson.dependencies = { + ...packageJson.dependencies, + '@vitejs/test-barrel': '1.0.0', + '@vitejs/test-primitives': '1.0.0', + } + return json(packageJson) + '\n' + }, + }, + 'node_modules/@vitejs/test-barrel/package.json': json({ + name: '@vitejs/test-barrel', + version: '1.0.0', + type: 'module', + exports: { + '.': './index.js', + './toast': './toast.js', + './legacy': './legacy.cjs', + }, + ...(options.sideEffects === undefined + ? {} + : { sideEffects: options.sideEffects }), + dependencies: { '@vitejs/test-primitives': '1.0.0' }, + peerDependencies: { react: '*' }, + }), + 'node_modules/@vitejs/test-barrel/index.js': /* js */ ` + export { Button } from './button.js' + export { Calendar } from './calendar.js' + export { getServerData } from './server-data.js' + export { Toast } from './toast.js' + `, + 'node_modules/@vitejs/test-barrel/button.js': /* js */ ` + import React from 'react' + export function Button(props) { + return React.createElement('button', null, props.children) + } + `, + 'node_modules/@vitejs/test-barrel/calendar.js': /* js */ ` + import 'client-only' + import React from 'react' + export function Calendar() { + return React.createElement('div', null, '[test-calendar]') + } + `, + 'node_modules/@vitejs/test-barrel/server-data.js': /* js */ ` + import 'server-only' + export function getServerData() { + return '[test-server-data]' + } + `, + 'node_modules/@vitejs/test-barrel/toast.js': /* js */ ` + import React from 'react' + import { toastQueue } from './toast-queue.js' + function Provider() { + return React.createElement('div', null, '[test-toast:' + toastQueue.items.length + ']') + } + export const Toast = { Provider } + `, + 'node_modules/@vitejs/test-barrel/toast-queue.js': /* js */ ` + import { Queue } from '@vitejs/test-primitives/toast' + export const toastQueue = new Queue() + `, + 'node_modules/@vitejs/test-barrel/legacy.cjs': /* js */ ` + require('client-only') + exports.legacyLabel = '[test-legacy]' + `, + 'node_modules/@vitejs/test-primitives/package.json': json({ + name: '@vitejs/test-primitives', + version: '1.0.0', + type: 'module', + exports: { './toast': './toast.js' }, + sideEffects: ['*.css'], + peerDependencies: { react: '*' }, + }), + 'node_modules/@vitejs/test-primitives/queue.js': /* js */ ` + export class Queue { + constructor() { + this.items = [] + } + } + `, + 'node_modules/@vitejs/test-primitives/toast.js': /* js */ ` + import { Queue as ToastQueue } from './queue.js' + import 'client-only' + export { ToastQueue as Queue } + `, + ...options.files, + }, + }) + } + + function rootSource(options: { imports: string; body: string }) { + return /* tsx */ ` + ${options.imports} + + export async function Root() { + return ( + + + + + + ${options.body} + + + ) + } + ` + } + + async function build(root: string) { + const result = await x('pnpm', ['build'], { + throwOnError: false, + nodeOptions: { cwd: root }, + }) + return { + exitCode: result.exitCode, + stderr: stripVTControlCharacters(result.stderr), + } + } + + test.describe('client-only in an unused barrel leaf', () => { + const root = 'examples/e2e/temp/validate-client-only-tree-shaking' + test.beforeAll(async () => { + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/root.tsx': rootSource({ + imports: `import { Button } from '@vitejs/test-barrel'`, + body: ``, + }), + }, + }) + }) + + const f = useFixture({ root, mode: 'build' }) + + test('build', async ({ page }) => { + using _ = expectNoPageError(page) + await page.goto(f.url()) + await waitForHydration(page) + await expect(page.getByRole('button')).toHaveText('[test-server-button]') + }) + }) + + test.describe('client-only in a module that only re-exports a used binding', () => { + // rollup keeps the marker, see "client-only kept by a re-export on rollup" + test.skip(!('rolldownVersion' in vite), 'rolldown only') + + const root = 'examples/e2e/temp/validate-client-only-tree-shaking-reexport' + test.beforeAll(async () => { + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + // bypass the barrel so that the re-export is the only marker import + 'src/root.tsx': rootSource({ + imports: `import { Toast } from '@vitejs/test-barrel/toast'`, + body: ``, + }), + }, + }) + }) + + const f = useFixture({ root, mode: 'build' }) + + test('build', async ({ page }) => { + using _ = expectNoPageError(page) + await page.goto(f.url()) + await waitForHydration(page) + await expect(page.getByText('[test-toast:0]')).toBeVisible() + }) + }) + + test.describe('server-only in an unused barrel leaf', () => { + const root = 'examples/e2e/temp/validate-server-only-tree-shaking' + test.beforeAll(async () => { + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/client.tsx': /* tsx */ ` + "use client"; + import React from 'react' + import { Button } from '@vitejs/test-barrel' + + export function TestClient() { + const [count, setCount] = React.useState(0) + return ( +
setCount(count + 1)}> + +
+ ) + } + `, + 'src/root.tsx': rootSource({ + imports: `import { TestClient } from './client.tsx'`, + body: ``, + }), + }, + }) + }) + + const f = useFixture({ root, mode: 'build' }) + + test('build', async ({ page }) => { + using _ = expectNoPageError(page) + await page.goto(f.url()) + await waitForHydration(page) + const button = page.getByRole('button') + await expect(button).toHaveText('[test-client-button:0]') + await button.click() + await expect(button).toHaveText('[test-client-button:1]') + }) + }) + + test.describe('invalid imports kept by tree-shaking', () => { + test('client-only in a used barrel leaf', async () => { + const root = 'examples/e2e/temp/validate-client-only-used-leaf' + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/root.tsx': rootSource({ + imports: `import { Calendar } from '@vitejs/test-barrel'`, + body: ``, + }), + }, + }) + const result = await build(root) + expect(result.stderr).toContain(`rsc:validate-imports`) + expect(result.stderr).toContain( + `'client-only' cannot be imported in server build ('rsc' environment):\n` + + ` imported by node_modules/@vitejs/test-barrel/calendar.js\n` + + ` imported by node_modules/@vitejs/test-barrel/index.js\n` + + ` imported by src/root.tsx\n`, + ) + expect(result.exitCode).not.toBe(0) + }) + + test('client-only in a package without sideEffects: false', async () => { + const root = 'examples/e2e/temp/validate-client-only-side-effects' + await setupTreeShakingFixture({ + root, + files: { + 'src/root.tsx': rootSource({ + imports: `import { Button } from '@vitejs/test-barrel'`, + body: ``, + }), + }, + }) + const result = await build(root) + expect(result.stderr).toContain( + `'client-only' cannot be imported in server build ('rsc' environment):\n` + + ` imported by node_modules/@vitejs/test-barrel/calendar.js\n`, + ) + expect(result.exitCode).not.toBe(0) + }) + + test('client-only required by a used CommonJS module', async () => { + const root = 'examples/e2e/temp/validate-client-only-require' + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/root.tsx': rootSource({ + imports: `import { legacyLabel } from '@vitejs/test-barrel/legacy'`, + body: `
{legacyLabel}
`, + }), + }, + }) + const result = await build(root) + // rollup's commonjs plugin adds proxy modules to the chain + expect(result.stderr).toMatch( + /'client-only' cannot be imported in server build \('rsc' environment\):\n(?: +imported by .*\n)*? +imported by node_modules\/@vitejs\/test-barrel\/legacy\.cjs\n(?: +imported by .*\n)*? +imported by src\/root\.tsx\n/, + ) + expect(result.exitCode).not.toBe(0) + }) + + test('client-only kept by a re-export on rollup', async () => { + test.skip('rolldownVersion' in vite, 'rollup only') + const root = 'examples/e2e/temp/validate-client-only-reexport-rollup' + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/root.tsx': rootSource({ + imports: `import { Button, Toast } from '@vitejs/test-barrel'`, + body: ``, + }), + }, + }) + const result = await build(root) + // the chain names the re-export that kept the marker rather than the + // dropped `calendar.js`, which also imports `client-only` + expect(result.stderr).toContain( + `'client-only' cannot be imported in server build ('rsc' environment):\n` + + ` imported by node_modules/@vitejs/test-primitives/toast.js\n` + + ` imported by node_modules/@vitejs/test-barrel/toast-queue.js\n` + + ` imported by node_modules/@vitejs/test-barrel/toast.js\n`, + ) + expect(result.exitCode).not.toBe(0) + }) + + test('client-only dynamically imported next to a dropped barrel leaf', async () => { + const root = 'examples/e2e/temp/validate-client-only-dynamic-import' + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/root.tsx': rootSource({ + imports: `import { Button } from '@vitejs/test-barrel'`, + body: `{await import('client-only').then(() => null)}`, + }), + }, + }) + const result = await build(root) + // the chain names the import that is kept rather than the dropped + // `calendar.js`, which also imports `client-only` + expect(result.stderr).toContain( + `'client-only' cannot be imported in server build ('rsc' environment):\n` + + ` imported by src/root.tsx\n`, + ) + expect(result.exitCode).not.toBe(0) + }) + + test('server-only in a used barrel leaf', async () => { + const root = 'examples/e2e/temp/validate-server-only-used-leaf' + await setupTreeShakingFixture({ + root, + sideEffects: false, + files: { + 'src/client.tsx': /* tsx */ ` + "use client"; + import { getServerData } from '@vitejs/test-barrel' + + export function TestClient() { + return
{getServerData()}
+ } + `, + 'src/root.tsx': rootSource({ + imports: `import { TestClient } from './client.tsx'`, + body: ``, + }), + }, + }) + const result = await build(root) + expect(result.stderr).toContain(`rsc:validate-imports`) + expect(result.stderr).toMatch( + /'server-only' cannot be imported in client build \('(client|ssr)' environment\):\n imported by node_modules\/@vitejs\/test-barrel\/server-data\.js\n imported by node_modules\/@vitejs\/test-barrel\/index\.js\n imported by src\/client\.tsx\n/, + ) + expect(result.exitCode).not.toBe(0) + }) + }) +}) diff --git a/packages/plugin-rsc/src/plugin.ts b/packages/plugin-rsc/src/plugin.ts index 6cb24453e..ee7735216 100644 --- a/packages/plugin-rsc/src/plugin.ts +++ b/packages/plugin-rsc/src/plugin.ts @@ -1393,7 +1393,7 @@ import.meta.hot.on("rsc:update", () => { ...vitePluginFindSourceMapURL(), ...vitePluginRscCss(rscPluginOptions, manager), { - ...validateImportPlugin(), + ...validateImportPlugin({ manager }), apply: () => rscPluginOptions.validateImports !== false, }, scanBuildStripPlugin({ manager }), diff --git a/packages/plugin-rsc/src/plugins/validate-import.ts b/packages/plugin-rsc/src/plugins/validate-import.ts index 2c9c00504..6d0daa278 100644 --- a/packages/plugin-rsc/src/plugins/validate-import.ts +++ b/packages/plugin-rsc/src/plugins/validate-import.ts @@ -1,11 +1,16 @@ import path from 'node:path' import { prefixRegex } from '@rolldown/pluginutils' import type { DevEnvironment, Plugin, Rollup } from 'vite' +import type { RscPluginManager } from '../plugin' // https://github.com/vercel/next.js/blob/90f564d376153fe0b5808eab7b83665ee5e08aaf/packages/next/src/build/webpack-config.ts#L1249-L1280 // https://github.com/pcattori/vite-env-only/blob/68a0cc8546b9a37c181c0b0a025eb9b62dbedd09/src/deny-imports.ts // https://github.com/sveltejs/kit/blob/84298477a014ec471839adf7a4448d91bc7949e4/packages/kit/src/exports/vite/index.js#L513 -export function validateImportPlugin(): Plugin { +export function validateImportPlugin({ + manager, +}: { + manager: RscPluginManager +}): Plugin { return { name: 'rsc:validate-imports', resolveId: { @@ -69,28 +74,32 @@ export function validateImportPlugin(): Plugin { } }, }, - // for build, use PluginContext.getModuleInfo during buildEnd. - // rollup shows multiple errors if there are other build error from `buildEnd(error)`. - buildEnd() { - if (this.environment.mode === 'build') { - const serverOnly = getImportChainBuild( - this, - '\0virtual:vite-rsc/validate-imports/invalid/server-only', - ) - validateImportChain( - serverOnly, - this.environment.name, - this.environment.config.root, - ) - const clientOnly = getImportChainBuild( - this, - '\0virtual:vite-rsc/validate-imports/invalid/client-only', - ) - validateImportChain( - clientOnly, - this.environment.name, - this.environment.config.root, - ) + // for build, check the output after tree-shaking. the invalid module has + // side effects, so the bundler keeps it whenever an import of it can run + // and drops it along with unused modules. checking the module graph + // earlier (e.g. in `buildEnd`) would also reject imports that tree-shaking + // drops, such as an unused leaf of a `sideEffects: false` barrel package. + generateBundle(_options, bundle) { + // scan build strips modules down to their imports, so it doesn't + // reflect what the real build keeps. the real build does the check. + if (manager.isScanBuild) return + const retained = new Set() + for (const output of Object.values(bundle)) { + if (output.type === 'chunk') { + for (const id of output.moduleIds) { + retained.add(id) + } + } + } + for (const source of ['server-only', 'client-only']) { + const id = `\0virtual:vite-rsc/validate-imports/invalid/${source}` + if (retained.has(id)) { + validateImportChain( + getImportChainBuild(this, id, retained), + this.environment.name, + this.environment.config.root, + ) + } } }, } @@ -112,19 +121,47 @@ function getImportChainDev(environment: DevEnvironment, id: string) { return chain } -function getImportChainBuild(ctx: Rollup.PluginContext, id: string): string[] { - const chain: string[] = [] - const recurse = (id: string) => { - if (chain.includes(id)) return +function getImportChainBuild( + ctx: Rollup.PluginContext, + id: string, + retained: Set, +): string[] { + const getImporters = (id: string) => { const info = ctx.getModuleInfo(id) - if (!info) return - chain.push(id) - const next = info.importers[0] - if (next) { - recurse(next) + return info ? [...info.importers, ...info.dynamicImporters] : [] + } + // start with the path to the nearest module kept in the output, which is + // what kept the invalid module. its direct importers can all be dropped, + // e.g. rollup keeps the side-effect imports of a dropped module that + // re-exports a used binding. + let chain = [id] + const parents = new Map() + const queue = [id] + for (const current of queue) { + if (current !== id && retained.has(current)) { + chain = [current] + for (let next = parents.get(current); next; next = parents.get(next)) { + chain.unshift(next) + } + break + } + for (const importer of getImporters(current)) { + if (importer !== id && !parents.has(importer)) { + parents.set(importer, current) + queue.push(importer) + } } } - recurse(id) + // then continue through importers kept in the output where possible. + while (true) { + const importers = getImporters(chain.at(-1)!).filter( + (importer) => !chain.includes(importer), + ) + const next = + importers.find((importer) => retained.has(importer)) ?? importers[0] + if (!next) break + chain.push(next) + } return chain }