diff --git a/packages/plugin-rsc/e2e/linked-package.test.ts b/packages/plugin-rsc/e2e/linked-package.test.ts new file mode 100644 index 000000000..30ef14453 --- /dev/null +++ b/packages/plugin-rsc/e2e/linked-package.test.ts @@ -0,0 +1,216 @@ +import fs from 'node:fs' +import path from 'node:path' +import { expect, test } from '@playwright/test' +import { setupInlineFixture, useFixture } from './fixture' +import { waitForHydration } from './helper' + +// "isolated" installs each package's dependencies next to it, like pnpm. +// "hoisted" installs every dependency in the app's node_modules, like npm. +for (const layout of ['isolated', 'hoisted'] as const) { + test.describe(layout, () => { + defineLinkedPackageTests(layout) + }) +} + +function defineLinkedPackageTests(layout: 'isolated' | 'hoisted') { + const root = `examples/e2e/temp/linked-package-${layout}` + // the node_modules that the dependencies of the package at `dir` go in + const nodeModules = (dir: string) => + layout === 'hoisted' ? 'node_modules' : path.posix.join(dir, 'node_modules') + + test.beforeAll(async () => { + // Dependency and import graph: + // + // app + // `-- @vitejs/test-workspace-ui (linked, "react" in "dependencies") + // |-- @vitejs/test-client-lib ("react" in "peerDependencies") + // `-- @vitejs/test-workspace-nested (linked, "react" in "dependencies") + // `-- @vitejs/test-nested-client-lib ("react" in "peerDependencies") + // + // Linked packages resolve outside node_modules, so Vite treats them as + // source. The client libraries are only reachable through the linked + // packages, and their "use client" directive sits on an internal module. + // Unless they're in "noExternal" and excluded from the server optimizers, + // dev externalizes them, or pre-bundles them and drops the directive when + // dependency discovery is enabled. Builds only externalize dependencies + // that resolve from the app, so they fail in the hoisted layout. + const json = (value: unknown) => JSON.stringify(value, null, 2) + // a React library whose "use client" directive is on an internal module + const clientLibrary = (parent: string, name: string, label: string) => { + const dir = `${nodeModules(parent)}/${name}` + return { + [`${dir}/package.json`]: json({ + name, + version: '1.0.0', + type: 'module', + exports: './index.js', + peerDependencies: { + react: '*', + }, + }), + [`${dir}/index.js`]: /* js */ ` + export { TestClient } from './client.js' + `, + [`${dir}/client.js`]: /* js */ ` + 'use client' + + import React from 'react' + + const CountContext = React.createContext(0) + + function Count() { + return ${JSON.stringify(label + ': ')} + React.useContext(CountContext) + } + + export function TestClient() { + const [count, setCount] = React.useState(0) + return React.createElement( + CountContext.Provider, + { value: count }, + React.createElement( + 'button', + { onClick: () => setCount((value) => value + 1) }, + React.createElement(Count), + ), + ) + } + `, + } + } + await setupInlineFixture({ + src: 'examples/starter-extra', + dest: root, + files: { + 'package.json': { + edit: (source) => { + const packageJson = JSON.parse(source) + packageJson.dependencies = { + ...packageJson.dependencies, + '@vitejs/test-workspace-ui': '0.0.0', + } + return JSON.stringify(packageJson, null, 2) + '\n' + }, + }, + 'src/root.tsx': /* tsx */ ` + import { TestWorkspace } from '@vitejs/test-workspace-ui/server' + + export function Root() { + return ( + + + + + + ) + } + `, + 'packages/ui/package.json': json({ + name: '@vitejs/test-workspace-ui', + version: '0.0.0', + private: true, + type: 'module', + exports: { + './server': './server.js', + }, + dependencies: { + '@vitejs/test-client-lib': '1.0.0', + '@vitejs/test-workspace-nested': '0.0.0', + react: '*', + }, + }), + 'packages/ui/server.js': /* js */ ` + import { TestClient } from '@vitejs/test-client-lib' + import { TestNested } from '@vitejs/test-workspace-nested/server' + import React from 'react' + + export function TestWorkspace() { + return React.createElement( + 'div', + null, + React.createElement(TestClient), + React.createElement(TestNested), + ) + } + `, + ...clientLibrary( + 'packages/ui', + '@vitejs/test-client-lib', + 'Workspace client', + ), + 'packages/nested/package.json': json({ + name: '@vitejs/test-workspace-nested', + version: '0.0.0', + private: true, + type: 'module', + exports: { + './server': './server.js', + }, + dependencies: { + '@vitejs/test-nested-client-lib': '1.0.0', + react: '*', + }, + }), + 'packages/nested/server.js': /* js */ ` + import { TestClient } from '@vitejs/test-nested-client-lib' + import React from 'react' + + export function TestNested() { + return React.createElement(TestClient) + } + `, + ...clientLibrary( + 'packages/nested', + '@vitejs/test-nested-client-lib', + 'Nested workspace client', + ), + }, + }) + // link workspace packages like a package manager would + const link = (target: string, linkPath: string) => { + const absoluteLinkPath = path.resolve(root, linkPath) + fs.mkdirSync(path.dirname(absoluteLinkPath), { recursive: true }) + // a junction doesn't need extra privileges on Windows (ignored elsewhere) + fs.symlinkSync(path.resolve(root, target), absoluteLinkPath, 'junction') + } + link('packages/ui', `${nodeModules('.')}/@vitejs/test-workspace-ui`) + link( + 'packages/nested', + `${nodeModules('packages/ui')}/@vitejs/test-workspace-nested`, + ) + }) + + function defineTest(f: ReturnType) { + test('client libraries imported by linked packages', async ({ page }) => { + await page.goto(f.url()) + await waitForHydration(page) + for (const label of ['Workspace client', 'Nested workspace client']) { + const button = (count: number) => + page.getByRole('button', { name: `${label}: ${count}`, exact: true }) + await button(0).click() + await expect(button(1)).toBeVisible() + } + }) + } + + for (const mode of ['dev', 'build'] as const) { + test.describe(mode, () => { + defineTest(useFixture({ root, mode })) + }) + } + + // `@cloudflare/vite-plugin` enables dependency discovery in the `rsc` environment + test.describe('dev-cloudflare', () => { + defineTest(useFixture({ root, mode: 'dev', command: 'pnpm cf:dev' })) + }) + + test.describe('build-cloudflare', () => { + defineTest( + useFixture({ + root, + mode: 'build', + buildCommand: 'pnpm cf:build', + command: 'pnpm cf:preview', + }), + ) + }) +} diff --git a/packages/plugin-rsc/src/plugin.ts b/packages/plugin-rsc/src/plugin.ts index 6cb24453e..b4d25be72 100644 --- a/packages/plugin-rsc/src/plugin.ts +++ b/packages/plugin-rsc/src/plugin.ts @@ -25,9 +25,9 @@ import { normalizePath, parseAstAsync, } from 'vite' -import { crawlFrameworkPkgs } from 'vitefu' import vitePluginRscCore from './core/plugin' import { cjsModuleRunnerPlugin } from './plugins/cjs' +import { crawlFrameworkPackages } from './plugins/crawl-framework-packages' import { vitePluginFindSourceMapURL } from './plugins/find-source-map-url' import { ensureEnvironmentImportsEntryFallback, @@ -511,8 +511,7 @@ export default function vitePluginRsc( } // crawl packages with "react" in "peerDependencies" to bundle react deps on server // see https://github.com/svitejs/vitefu/blob/d8d82fa121e3b2215ba437107093c77bde51b63b/src/index.js#L95-L101 - const result = await crawlFrameworkPkgs({ - root: process.cwd(), + const frameworkPackages = await crawlFrameworkPackages(process.cwd(), { isBuild: env.command === 'build', isFrameworkPkgByJson(pkgJson) { if ([PKG_NAME, 'react-dom'].includes(pkgJson.name)) { @@ -528,13 +527,13 @@ export default function vitePluginRsc( 'server-only', 'client-only', PKG_NAME, - ...result.ssr.noExternal.sort(), + ...frameworkPackages.sort(), ] const optimizeDepsExclude = noExternal.filter( (pkg) => !['react', 'react-dom', 'react-server-dom-webpack'].includes(pkg), ) - hasReactServerDomWebpack = result.ssr.noExternal.includes( + hasReactServerDomWebpack = frameworkPackages.includes( 'react-server-dom-webpack', ) const reactServerDomPackageName = hasReactServerDomWebpack diff --git a/packages/plugin-rsc/src/plugins/crawl-framework-packages.test.ts b/packages/plugin-rsc/src/plugins/crawl-framework-packages.test.ts new file mode 100644 index 000000000..5d5e93bb7 --- /dev/null +++ b/packages/plugin-rsc/src/plugins/crawl-framework-packages.test.ts @@ -0,0 +1,183 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { crawlFrameworkPackages } from './crawl-framework-packages' + +describe(crawlFrameworkPackages, () => { + let root: string + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'rsc-crawl-')) + }) + + afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }) + }) + + function writePackage(dir: string, pkgJson: Record) { + fs.mkdirSync(path.join(root, dir), { recursive: true }) + fs.writeFileSync( + path.join(root, dir, 'package.json'), + JSON.stringify({ version: '1.0.0', ...pkgJson }), + ) + } + + // install a package into `node_modules` of `parent` + function installPackage( + parent: string, + pkgJson: { name: string } & Record, + ) { + writePackage(path.join(parent, 'node_modules', pkgJson.name), pkgJson) + } + + // link a package outside `node_modules` into `node_modules` of `parent`, like a workspace package + function linkPackage(parent: string, name: string, target: string) { + const linkPath = path.join(root, parent, 'node_modules', name) + fs.mkdirSync(path.dirname(linkPath), { recursive: true }) + // a junction doesn't need extra privileges on Windows (ignored elsewhere) + fs.symlinkSync(path.join(root, target), linkPath, 'junction') + } + + function crawl() { + return crawlFrameworkPackages(path.join(root, 'app'), { + isBuild: false, + isFrameworkPkgByJson(pkgJson) { + const deps = pkgJson['peerDependencies'] + return deps && 'react' in deps + }, + }) + } + + const reactLibrary = (name: string, extra?: Record) => ({ + name, + peerDependencies: { react: '*' }, + ...extra, + }) + + it('framework packages of the app', async () => { + writePackage('app', { name: 'app', dependencies: { 'react-lib': '*' } }) + installPackage('app', reactLibrary('react-lib')) + expect(await crawl()).toEqual(['react-lib']) + }) + + it('framework packages imported only by a linked package', async () => { + writePackage('app', { name: 'app', dependencies: { ui: '*' } }) + writePackage('packages/ui', { + name: 'ui', + dependencies: { react: '*', 'react-lib': '*' }, + }) + linkPackage('app', 'ui', 'packages/ui') + installPackage('packages/ui', reactLibrary('react-lib')) + installPackage('packages/ui', { name: 'react' }) + expect(await crawl()).toEqual(['react-lib']) + }) + + it('linked packages of linked packages', async () => { + writePackage('app', { name: 'app', dependencies: { ui: '*' } }) + writePackage('packages/ui', { + name: 'ui', + dependencies: { primitives: '*' }, + }) + writePackage('packages/primitives', { + name: 'primitives', + dependencies: { 'react-lib': '*' }, + }) + linkPackage('app', 'ui', 'packages/ui') + linkPackage('packages/ui', 'primitives', 'packages/primitives') + installPackage('packages/primitives', reactLibrary('react-lib')) + expect(await crawl()).toEqual(['react-lib']) + }) + + it('devDependencies of linked packages', async () => { + writePackage('app', { name: 'app', devDependencies: { ui: '*' } }) + writePackage('packages/ui', { + name: 'ui', + peerDependencies: { 'react-lib': '*' }, + devDependencies: { 'react-lib': '*' }, + }) + linkPackage('app', 'ui', 'packages/ui') + installPackage('packages/ui', reactLibrary('react-lib')) + expect(await crawl()).toEqual(['react-lib']) + }) + + it('dependencies of framework packages reached through a linked package', async () => { + writePackage('app', { name: 'app', dependencies: { ui: '*' } }) + writePackage('packages/ui', { + name: 'ui', + dependencies: { 'react-lib': '*' }, + }) + linkPackage('app', 'ui', 'packages/ui') + installPackage( + 'packages/ui', + reactLibrary('react-lib', { dependencies: { 'react-lib-dep': '*' } }), + ) + installPackage( + 'packages/ui/node_modules/react-lib', + reactLibrary('react-lib-dep'), + ) + expect((await crawl()).sort()).toEqual(['react-lib', 'react-lib-dep']) + }) + + it('framework packages installed under an alias elsewhere', async () => { + writePackage('app', { + name: 'app', + dependencies: { 'react-lib-v2': 'npm:react-lib@2', ui: '*' }, + }) + writePackage( + 'app/node_modules/react-lib-v2', + reactLibrary('react-lib', { version: '2.0.0' }), + ) + writePackage('packages/ui', { + name: 'ui', + dependencies: { 'react-lib': '*' }, + }) + linkPackage('app', 'ui', 'packages/ui') + installPackage('packages/ui', reactLibrary('react-lib')) + expect((await crawl()).sort()).toEqual(['react-lib', 'react-lib-v2']) + }) + + it('skips non-framework packages installed in node_modules', async () => { + // not linked, so vitefu's rule still applies: Vite pre-bundles it as a + // whole, including the React libraries it imports + writePackage('app', { name: 'app', dependencies: { 'non-react': '*' } }) + installPackage('app', { + name: 'non-react', + dependencies: { 'react-lib': '*' }, + }) + installPackage('app/node_modules/non-react', reactLibrary('react-lib')) + expect(await crawl()).toEqual([]) + }) + + it('skips linked packages without a valid package.json', async () => { + writePackage('app', { + name: 'app', + dependencies: { broken: '*', ui: '*' }, + }) + writePackage('packages/ui', { + name: 'ui', + dependencies: { 'react-lib': '*' }, + }) + fs.mkdirSync(path.join(root, 'packages/broken'), { recursive: true }) + fs.writeFileSync(path.join(root, 'packages/broken/package.json'), '{') + linkPackage('app', 'broken', 'packages/broken') + linkPackage('app', 'ui', 'packages/ui') + installPackage('packages/ui', reactLibrary('react-lib')) + expect(await crawl()).toEqual(['react-lib']) + }) + + it('handles linked packages that depend on each other', async () => { + writePackage('app', { name: 'app', dependencies: { a: '*' } }) + writePackage('packages/a', { name: 'a', dependencies: { b: '*' } }) + writePackage('packages/b', { + name: 'b', + dependencies: { a: '*', app: '*', 'react-lib': '*' }, + }) + linkPackage('app', 'a', 'packages/a') + linkPackage('packages/a', 'b', 'packages/b') + linkPackage('packages/b', 'a', 'packages/a') + linkPackage('packages/b', 'app', 'app') + installPackage('packages/b', reactLibrary('react-lib')) + expect(await crawl()).toEqual(['react-lib']) + }) +}) diff --git a/packages/plugin-rsc/src/plugins/crawl-framework-packages.ts b/packages/plugin-rsc/src/plugins/crawl-framework-packages.ts new file mode 100644 index 000000000..c42807486 --- /dev/null +++ b/packages/plugin-rsc/src/plugins/crawl-framework-packages.ts @@ -0,0 +1,123 @@ +import fs from 'node:fs' +import path from 'node:path' +import { + type CrawlFrameworkPkgsOptions, + crawlFrameworkPkgs, + findClosestPkgJsonPath, + findDepPkgJsonPath, +} from 'vitefu' + +type CrawlOptions = Required< + Pick +> + +type PackageJson = { + dependencies?: Record + devDependencies?: Record +} + +/** + * Crawl the framework packages of the app at `root` and of every linked + * package it depends on, returning their names for `ssr.noExternal`. + * + * vitefu only descends into framework packages, so it misses the React + * libraries imported by a workspace package that lists "react" in + * "dependencies". Linked packages resolve outside node_modules and Vite treats + * them as source, so the libraries they import need the same handling as the + * app's own. + */ +export async function crawlFrameworkPackages( + root: string, + options: CrawlOptions, +): Promise { + const frameworkNames = new Set() + // names of the packages checked by earlier crawls + const checked = new Set() + // crawl one at a time so each crawl skips the packages checked by earlier + // ones, along with their dependencies. vitefu only passes the dependency + // name, so if linked packages install different versions of a package, only + // the first one found is checked. + for (const dir of [root, ...(await findLinkedPackageDirs(root))]) { + // package names, which differ from dependency names for npm aliases + const otherNames: string[] = [] + const result = await crawlFrameworkPkgs({ + root: dir, + isBuild: options.isBuild, + isFrameworkPkgByName: (name) => (checked.has(name) ? false : undefined), + isFrameworkPkgByJson(pkgJson) { + const isFrameworkPkg = options.isFrameworkPkgByJson(pkgJson) + if (!isFrameworkPkg) { + otherNames.push(pkgJson.name) + } + return isFrameworkPkg + }, + }) + for (const name of result.ssr.noExternal) { + frameworkNames.add(name) + checked.add(name) + } + for (const name of otherNames) { + checked.add(name) + } + } + return [...frameworkNames] +} + +/** + * Find the linked packages that `root` depends on, directly or through other + * linked packages: workspace packages and `link:`/`file:` dependencies, which + * resolve outside node_modules. + */ +async function findLinkedPackageDirs(root: string): Promise { + const rootPkgJsonPath = await findClosestPkgJsonPath(root) + const rootPkgJson = rootPkgJsonPath && (await readPkgJson(rootPkgJsonPath)) + if (!rootPkgJsonPath || !rootPkgJson) { + return [] + } + const visited = new Set([fs.realpathSync(rootPkgJsonPath)]) + const dirs: string[] = [] + + // visit the dependencies of the package at `dir` + async function visitDependencies(dir: string, pkgJson: PackageJson) { + const deps = [ + ...Object.keys(pkgJson.dependencies ?? {}), + ...Object.keys(pkgJson.devDependencies ?? {}), + ] + await Promise.all( + deps.map(async (dep) => { + // the real path, so linked packages point outside node_modules + const depPkgJsonPath = await findDepPkgJsonPath(dep, dir) + if ( + !depPkgJsonPath || + isInNodeModules(depPkgJsonPath) || + visited.has(depPkgJsonPath) + ) { + return + } + visited.add(depPkgJsonPath) + const depPkgJson = await readPkgJson(depPkgJsonPath) + if (depPkgJson) { + const depDir = path.dirname(depPkgJsonPath) + dirs.push(depDir) + await visitDependencies(depDir, depPkgJson) + } + }), + ) + } + + await visitDependencies(path.dirname(rootPkgJsonPath), rootPkgJson) + // sort for a stable crawl order, since dependencies resolve concurrently + return dirs.sort() +} + +function isInNodeModules(file: string): boolean { + return /[/\\]node_modules[/\\]/.test(file) +} + +async function readPkgJson(file: string): Promise { + try { + return JSON.parse(await fs.promises.readFile(file, 'utf-8')) + } catch { + return undefined + } +}