From e2fe6d349f3db2db2af6e90fe5f056ca96499125 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:30:59 +0200 Subject: [PATCH 1/2] fix(server-utils): Bundle the CommonJS dependencies of instrumented packages on Vite 8 Rolldown emits a require() of an external package in bundled CommonJS as __require(), which a bundler that runs over the server output cannot follow. On Rolldown, the orchestrion Vite plugin now also force-bundles the dependencies that the CommonJS code of instrumented packages requires. Fixes #25470 --- .../tanstackstart-react/package.json | 5 ++ .../tests/server-build.test.ts | 22 +++++ .../bundler/commonJSDependencies.ts | 89 +++++++++++++++++++ .../src/orchestrion/bundler/vite.ts | 29 +++++- .../test/orchestrion/bundler.test.ts | 62 +++++++++++-- .../orchestrion/commonJSDependencies.test.ts | 75 ++++++++++++++++ 6 files changed, 271 insertions(+), 11 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts create mode 100644 packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts create mode 100644 packages/server-utils/test/orchestrion/commonJSDependencies.test.ts diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json b/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json index d080dc4e25fd..ed739895d0d9 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json @@ -14,6 +14,7 @@ "test:build:tunnel-static": "pnpm install && E2E_TEST_TUNNEL_ROUTE_MODE=static E2E_TEST_DSN=http://public@localhost:3031/1337 pnpm build", "test:build:tunnel-custom": "pnpm install && E2E_TEST_CUSTOM_TUNNEL_ROUTE=1 pnpm build", "test:build:tunnel-object": "pnpm install && E2E_TEST_TUNNEL_ROUTE_MODE=object E2E_TEST_DSN=http://public@localhost:3031/1337 pnpm build", + "test:build:hoisted": "pnpm install --config.node-linker=hoisted && pnpm build", "test:build-latest": "pnpm add @tanstack/react-start@latest @tanstack/react-router@latest nitro@latest vite@latest @vitejs/plugin-react-swc@latest vite-tsconfig-paths@latest && pnpm install && pnpm build", "test:assert:proxy": "pnpm test && TEST_ENV=development pnpm playwright test db-drivers", "test:assert": "pnpm test:assert:proxy", @@ -51,6 +52,10 @@ }, "sentryTest": { "variants": [ + { + "label": "tanstackstart-react (hoisted)", + "build-command": "pnpm test:build:hoisted" + }, { "label": "tanstackstart-react (tunnel-generated)", "build-command": "pnpm test:build:tunnel-generated", diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts new file mode 100644 index 000000000000..be86266cf2e8 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts @@ -0,0 +1,22 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { expect, test } from '@playwright/test'; +import { bundleReferencesModule } from '@sentry-internal/test-utils'; + +// Vite 8 emits a `require()` of an external package in bundled CommonJS as `__require()`, and Nitro's +// second bundling pass cannot follow that call. In a flat install (the hoisted variant), Vite keeps +// the dependencies of the instrumented drivers external unless the SDK bundles them, and the server +// then fails with `Cannot find module` outside the project. +test('bundles the CommonJS dependencies of the instrumented drivers', () => { + const root = path.join(process.cwd(), '.output/server'); + const serverBundle = readdirSync(root, { recursive: true, encoding: 'utf8' }) + .filter(file => /\.m?js$/.test(file) && !file.startsWith('node_modules')) + .map(file => readFileSync(path.join(root, file), 'utf8')) + .join('\n'); + const driverDependencies = ['sqlstring', 'bignumber.js', 'denque', 'cluster-key-slot', 'standard-as-callback']; + + const externalRequires = [...serverBundle.matchAll(/__require\(["']([^"']+)["']\)/g)].map(match => match[1]); + + expect(bundleReferencesModule(serverBundle, 'mysql'), 'mysql must be bundled to be instrumented').toBe(false); + expect(externalRequires.filter(name => driverDependencies.includes(name))).toEqual([]); +}); diff --git a/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts b/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts new file mode 100644 index 000000000000..527170c2e704 --- /dev/null +++ b/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts @@ -0,0 +1,89 @@ +import { isPlainObject } from '@sentry/core'; +import { readFileSync, realpathSync } from 'node:fs'; +import { dirname, join } from 'node:path'; + +interface InstalledPackage { + dir: string; + manifest: Record; +} + +/** + * Names of the installed packages that the CommonJS code of `moduleNames` `require()`s, directly + * or through other such packages. + * + * Only `dependencies` count. Optional and peer dependencies may be missing or native, and the app + * owns them. The walk stops at ESM packages, because a static `import` of an external package + * stays an `import` in the output. + */ +export function commonJSDependencyNames(root: string, moduleNames: readonly string[]): string[] { + const names = new Set(); + const visitedDirs = new Set(); + + const visit = (pkg: InstalledPackage): void => { + if (visitedDirs.has(pkg.dir) || isESMOnly(pkg.manifest)) { + return; + } + visitedDirs.add(pkg.dir); + + for (const name of requiredDependencyNames(pkg.manifest)) { + const dependency = findInstalledPackage(name, pkg.dir); + if (dependency) { + names.add(name); + visit(dependency); + } + } + }; + + for (const name of moduleNames) { + const pkg = findInstalledPackage(name, root); + if (pkg && !hasESMEntry(pkg.manifest)) { + visit(pkg); + } + } + + return [...names]; +} + +// A `require()` loads the CommonJS build of a package, unless the package is ESM-only. +function isESMOnly(manifest: Record): boolean { + return manifest.type === 'module'; +} + +// The app `import`s instrumented packages, so a package with an ESM entry loads no CommonJS. A +// `module` field is the common sign. A wrapper like pg's `exports.import` still loads CommonJS. +function hasESMEntry(manifest: Record): boolean { + return isESMOnly(manifest) || typeof manifest.module === 'string'; +} + +function requiredDependencyNames(manifest: Record): string[] { + const optional = keysOf(manifest.optionalDependencies); + return keysOf(manifest.dependencies).filter(name => !optional.includes(name)); +} + +function keysOf(value: unknown): string[] { + return isPlainObject(value) ? Object.keys(value) : []; +} + +// Looks up `node_modules` like Node does. The real path matters under isolated installs (pnpm), +// where a package's own dependencies are siblings of its real location, not of the symlink. +function findInstalledPackage(name: string, fromDir: string): InstalledPackage | undefined { + for (let dir = fromDir; ; dir = dirname(dir)) { + const packageDir = join(dir, 'node_modules', name); + const manifest = readManifest(join(packageDir, 'package.json')); + if (manifest) { + return { dir: realpathSync(packageDir), manifest }; + } + if (dirname(dir) === dir) { + return undefined; + } + } +} + +function readManifest(file: string): Record | undefined { + try { + const manifest: unknown = JSON.parse(readFileSync(file, 'utf8')); + return isPlainObject(manifest) ? manifest : undefined; + } catch { + return undefined; + } +} diff --git a/packages/server-utils/src/orchestrion/bundler/vite.ts b/packages/server-utils/src/orchestrion/bundler/vite.ts index c6a704bb1ca9..6a8cb2ca235d 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -1,8 +1,10 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/vite'; -import type { ConfigEnv, Plugin, ResolvedConfig, UserConfig } from 'vite'; +import { resolve } from 'node:path'; +import type { ConfigEnv, ConfigPluginContext, Plugin, ResolvedConfig, UserConfig } from 'vite'; export type { Plugin as VitePlugin } from 'vite'; import { instrumentedModuleNames } from '../config'; +import { commonJSDependencyNames } from './commonJSDependencies'; import type { PluginOptions } from './options'; import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './resolve'; @@ -69,9 +71,11 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { } const upstream = codeTransformer(orchestrionTransformOptions(options)); + let commonJSDependencies: string[] = []; const noExternalModules = (): string[] => [ ...instrumentedModuleNames(options.instrumentations), '@sentry/server-utils', + ...commonJSDependencies, ]; const instance = {}; @@ -116,14 +120,33 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { // ESM entry, which crashes at startup. // Not in `serve`: Vite's dev SSR runner has no CJS interop, so inlined `mysql`/`ioredis` throw // `exports is not defined`, and the runtime hook injects the same publishers instead. + // Rolldown (Vite 8) emits a `require()` of an external package in bundled CommonJS as + // `__require()`, which a bundler that runs over the server output cannot follow. Rollup turns it + // into an `import`. So on Rolldown, the dependencies that this CommonJS requires are bundled too. config: { // Runs after the framework plugins, so `build.ssr` set by their `config` hooks is visible. order: 'post', - handler(config: UserConfig, env?: ConfigEnv): { ssr: { noExternal: string[] } } | null { + handler( + this: ConfigPluginContext | undefined, + config: UserConfig, + env?: ConfigEnv, + ): { ssr: { noExternal: string[] } } | null { + if (env?.command === 'serve') { + return null; + } + + // Vite 5 calls `config` hooks without a `this` context. + if (this?.meta.rolldownVersion) { + commonJSDependencies = commonJSDependencyNames( + resolve(config.root ?? process.cwd()), + instrumentedModuleNames(options.instrumentations), + ); + } + // A top-level `ssr` key makes Vite 6+ add an `ssr` environment with no entry, which // `vite build --app` cannot build. Vite 5 has no `configEnvironment` and needs the key, and // its SSR builds always set `build.ssr`. - if (env?.command === 'serve' || !(config.ssr || config.build?.ssr)) { + if (!(config.ssr || config.build?.ssr)) { return null; } diff --git a/packages/server-utils/test/orchestrion/bundler.test.ts b/packages/server-utils/test/orchestrion/bundler.test.ts index d6cf125df97d..f5ae2c74e599 100644 --- a/packages/server-utils/test/orchestrion/bundler.test.ts +++ b/packages/server-utils/test/orchestrion/bundler.test.ts @@ -1,10 +1,11 @@ -import { existsSync } from 'node:fs'; -import { isAbsolute } from 'node:path'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { isAbsolute, join } from 'node:path'; import type { OnStartResult, PluginBuild } from 'esbuild'; import type { NormalizedInputOptions, PluginContext } from 'rollup'; -import type { ResolvedConfig } from 'vite'; +import type { Plugin, ResolvedConfig } from 'vite'; import type { Compiler } from 'webpack'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { sentryOrchestrionPlugin as bunPlugin } from '../../src/orchestrion/bundler/bun'; import { sentryOrchestrionPlugin as esbuildPlugin } from '../../src/orchestrion/bundler/esbuild'; import { orchestrionTransformOptions } from '../../src/orchestrion/bundler/options'; @@ -219,6 +220,9 @@ describe('sentryOrchestrionWebpackPlugin', () => { }); describe('sentryOrchestrionPlugin (vite)', () => { + const ROLLUP_CONTEXT = { meta: { rollupVersion: '4.0.0' } }; + const ROLLDOWN_CONTEXT = { meta: { rollupVersion: '4.0.0', rolldownVersion: '1.2.14' } }; + function runConfigResolved( ssrExternal: string[] | true | undefined, command: 'build' | 'serve' = 'build', @@ -234,23 +238,26 @@ describe('sentryOrchestrionPlugin (vite)', () => { return warn; } + // `context` is the `this` of the hook: Vite 6+ pass `meta`, Vite 5 passes nothing. function runConfig( command: 'build' | 'serve', userConfig: Record = { build: { ssr: true } }, + options: { plugin?: Plugin; context?: unknown } = {}, ): { ssr: { noExternal: string[] } } | null { - const plugin = vitePlugin(); + const { plugin = vitePlugin() } = options; + const context = 'context' in options ? options.context : ROLLUP_CONTEXT; const config = plugin.config as { - handler: (config: unknown, env: unknown) => { ssr: { noExternal: string[] } } | null; + handler: (this: unknown, config: unknown, env: unknown) => { ssr: { noExternal: string[] } } | null; }; - return config.handler.call(plugin, userConfig, { command, mode: 'production' }); + return config.handler.call(context, userConfig, { command, mode: 'production' }); } function runConfigEnvironment( name: string, environmentConfig: Record, command: 'build' | 'serve' = 'build', + plugin = vitePlugin(), ): { resolve: { noExternal: string[] } } | null { - const plugin = vitePlugin(); const configEnvironment = plugin.configEnvironment as ( name: string, config: unknown, @@ -283,6 +290,45 @@ describe('sentryOrchestrionPlugin (vite)', () => { expect(config?.ssr.noExternal).toContain('mysql'); }); + describe('dependencies of bundled CommonJS', () => { + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orch-vite-cjs-deps-')); + mkdirSync(join(root, 'node_modules/pg'), { recursive: true }); + writeFileSync(join(root, 'node_modules/pg/package.json'), JSON.stringify({ dependencies: { 'pg-types': '^2' } })); + mkdirSync(join(root, 'node_modules/pg-types'), { recursive: true }); + writeFileSync(join(root, 'node_modules/pg-types/package.json'), '{}'); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('force-bundles them on Rolldown, which emits their requires as `__require()`', () => { + const config = runConfig('build', { root, build: { ssr: true } }, { context: ROLLDOWN_CONTEXT }); + + expect(config?.ssr.noExternal).toContain('pg-types'); + }); + + it('force-bundles them in server environments on Rolldown', () => { + const plugin = vitePlugin(); + + runConfig('build', { root }, { plugin, context: ROLLDOWN_CONTEXT }); + + expect(runConfigEnvironment('ssr', {}, 'build', plugin)?.resolve.noExternal).toContain('pg-types'); + }); + + it.each([ + ['Rollup', ROLLUP_CONTEXT], + ['Vite 5, which calls config hooks without a context', undefined], + ])('leaves them external on %s', (_, context) => { + const config = runConfig('build', { root, build: { ssr: true } }, { context }); + + expect(config?.ssr.noExternal).not.toContain('pg-types'); + }); + }); + it('does not force-bundle instrumented modules on the dev server', () => { // Inlined in dev, the CommonJS drivers throw `exports is not defined` on import. expect(runConfig('serve')).toBeNull(); diff --git a/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts b/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts new file mode 100644 index 000000000000..4126a542c8ad --- /dev/null +++ b/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts @@ -0,0 +1,75 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { commonJSDependencyNames } from '../../src/orchestrion/bundler/commonJSDependencies'; + +function installPackage(parentDir: string, name: string, manifest: Record = {}): string { + const dir = join(parentDir, 'node_modules', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, ...manifest })); + return dir; +} + +describe('commonJSDependencyNames', () => { + let root: string; + + beforeEach(() => { + root = realpathSync(mkdtempSync(join(tmpdir(), 'orch-cjs-deps-'))); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('collects the dependencies of a CommonJS package transitively', () => { + installPackage(root, 'pg', { dependencies: { 'pg-types': '^2' } }); + installPackage(root, 'pg-types', { dependencies: { 'postgres-array': '~2' } }); + installPackage(root, 'postgres-array'); + + expect(commonJSDependencyNames(root, ['pg']).sort()).toEqual(['pg-types', 'postgres-array']); + }); + + it.each([ + ['an optional', { dependencies: { dep: '^1' }, optionalDependencies: { dep: '^1' } }], + ['a peer', { peerDependencies: { dep: '^1' } }], + ])('skips %s dependency', (_, manifest) => { + installPackage(root, 'pg', manifest); + installPackage(root, 'dep'); + + expect(commonJSDependencyNames(root, ['pg'])).toEqual([]); + }); + + it('skips a dependency that is not installed', () => { + installPackage(root, 'pg', { dependencies: { 'not-installed': '^1' } }); + + expect(commonJSDependencyNames(root, ['pg'])).toEqual([]); + }); + + it('includes an ESM dependency but not its own dependencies', () => { + installPackage(root, 'mysql', { dependencies: { 'esm-only': '^1' } }); + installPackage(root, 'esm-only', { type: 'module', dependencies: { 'esm-dep': '^1' } }); + installPackage(root, 'esm-dep'); + + expect(commonJSDependencyNames(root, ['mysql'])).toEqual(['esm-only']); + }); + + it.each([{ module: './esm/index.js' }, { type: 'module' }])( + 'skips an instrumented package with an ESM entry (%o)', + manifest => { + installPackage(root, 'graphql', { ...manifest, dependencies: { dep: '^1' } }); + installPackage(root, 'dep'); + + expect(commonJSDependencyNames(root, ['graphql'])).toEqual([]); + }, + ); + + it('resolves dependencies from the real location of a symlinked package', () => { + const store = join(root, 'node_modules/.pnpm/ioredis@5.0.0'); + const realDir = installPackage(store, 'ioredis', { dependencies: { denque: '^2' } }); + installPackage(store, 'denque'); + symlinkSync(realDir, join(root, 'node_modules/ioredis'), 'dir'); + + expect(commonJSDependencyNames(root, ['ioredis'])).toEqual(['denque']); + }); +}); From 9d08cb462417fa63dbccb601dc0b63e1ba9a3fbb Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:58:47 +0200 Subject: [PATCH 2/2] fix(server-utils): Bundle the external requires of instrumented CommonJS on Rolldown Resolve each require() call from a package that orchestrion bundled instead of walking package.json files. Only calls that Vite would make external are bundled, and the user's ssr.external entries and optional dependencies stay external. --- .../tests/server-build.test.ts | 8 +- .../bundler/commonJSDependencies.ts | 89 --------- .../orchestrion/bundler/externalRequire.ts | 125 +++++++++++++ .../src/orchestrion/bundler/vite.ts | 36 +--- .../test/orchestrion/bundler.test.ts | 83 +++------ .../orchestrion/commonJSDependencies.test.ts | 75 -------- .../test/orchestrion/externalRequire.test.ts | 173 ++++++++++++++++++ 7 files changed, 340 insertions(+), 249 deletions(-) delete mode 100644 packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts create mode 100644 packages/server-utils/src/orchestrion/bundler/externalRequire.ts delete mode 100644 packages/server-utils/test/orchestrion/commonJSDependencies.test.ts create mode 100644 packages/server-utils/test/orchestrion/externalRequire.test.ts diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts index be86266cf2e8..57bf54397164 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts @@ -3,10 +3,10 @@ import path from 'node:path'; import { expect, test } from '@playwright/test'; import { bundleReferencesModule } from '@sentry-internal/test-utils'; -// Vite 8 emits a `require()` of an external package in bundled CommonJS as `__require()`, and Nitro's -// second bundling pass cannot follow that call. In a flat install (the hoisted variant), Vite keeps -// the dependencies of the instrumented drivers external unless the SDK bundles them, and the server -// then fails with `Cannot find module` outside the project. +// Nitro can't follow the `__require()` calls that Vite 8 emits for external requires in bundled +// CommonJS, so the deployed server fails with `Cannot find module`. Only flat installs (the hoisted +// variant) keep the drivers' dependencies external. +// https://vite.dev/guide/migration#require-calls-for-externalized-modules test('bundles the CommonJS dependencies of the instrumented drivers', () => { const root = path.join(process.cwd(), '.output/server'); const serverBundle = readdirSync(root, { recursive: true, encoding: 'utf8' }) diff --git a/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts b/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts deleted file mode 100644 index 527170c2e704..000000000000 --- a/packages/server-utils/src/orchestrion/bundler/commonJSDependencies.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { isPlainObject } from '@sentry/core'; -import { readFileSync, realpathSync } from 'node:fs'; -import { dirname, join } from 'node:path'; - -interface InstalledPackage { - dir: string; - manifest: Record; -} - -/** - * Names of the installed packages that the CommonJS code of `moduleNames` `require()`s, directly - * or through other such packages. - * - * Only `dependencies` count. Optional and peer dependencies may be missing or native, and the app - * owns them. The walk stops at ESM packages, because a static `import` of an external package - * stays an `import` in the output. - */ -export function commonJSDependencyNames(root: string, moduleNames: readonly string[]): string[] { - const names = new Set(); - const visitedDirs = new Set(); - - const visit = (pkg: InstalledPackage): void => { - if (visitedDirs.has(pkg.dir) || isESMOnly(pkg.manifest)) { - return; - } - visitedDirs.add(pkg.dir); - - for (const name of requiredDependencyNames(pkg.manifest)) { - const dependency = findInstalledPackage(name, pkg.dir); - if (dependency) { - names.add(name); - visit(dependency); - } - } - }; - - for (const name of moduleNames) { - const pkg = findInstalledPackage(name, root); - if (pkg && !hasESMEntry(pkg.manifest)) { - visit(pkg); - } - } - - return [...names]; -} - -// A `require()` loads the CommonJS build of a package, unless the package is ESM-only. -function isESMOnly(manifest: Record): boolean { - return manifest.type === 'module'; -} - -// The app `import`s instrumented packages, so a package with an ESM entry loads no CommonJS. A -// `module` field is the common sign. A wrapper like pg's `exports.import` still loads CommonJS. -function hasESMEntry(manifest: Record): boolean { - return isESMOnly(manifest) || typeof manifest.module === 'string'; -} - -function requiredDependencyNames(manifest: Record): string[] { - const optional = keysOf(manifest.optionalDependencies); - return keysOf(manifest.dependencies).filter(name => !optional.includes(name)); -} - -function keysOf(value: unknown): string[] { - return isPlainObject(value) ? Object.keys(value) : []; -} - -// Looks up `node_modules` like Node does. The real path matters under isolated installs (pnpm), -// where a package's own dependencies are siblings of its real location, not of the symlink. -function findInstalledPackage(name: string, fromDir: string): InstalledPackage | undefined { - for (let dir = fromDir; ; dir = dirname(dir)) { - const packageDir = join(dir, 'node_modules', name); - const manifest = readManifest(join(packageDir, 'package.json')); - if (manifest) { - return { dir: realpathSync(packageDir), manifest }; - } - if (dirname(dir) === dir) { - return undefined; - } - } -} - -function readManifest(file: string): Record | undefined { - try { - const manifest: unknown = JSON.parse(readFileSync(file, 'utf8')); - return isPlainObject(manifest) ? manifest : undefined; - } catch { - return undefined; - } -} diff --git a/packages/server-utils/src/orchestrion/bundler/externalRequire.ts b/packages/server-utils/src/orchestrion/bundler/externalRequire.ts new file mode 100644 index 000000000000..971f19c322d6 --- /dev/null +++ b/packages/server-utils/src/orchestrion/bundler/externalRequire.ts @@ -0,0 +1,125 @@ +import { isPlainObject } from '@sentry/core'; +import { readFileSync } from 'node:fs'; +import { createRequire, isBuiltin } from 'node:module'; +import { isAbsolute } from 'node:path'; +import type { Plugin } from 'vite'; + +type ResolveIdHandler = Extract, (...args: never[]) => unknown>; +type ResolveIdParameters = Parameters; + +const NODE_MODULES = '/node_modules/'; + +/** + * Creates a `resolveId` handler that bundles the external packages that bundled CommonJS `require()`s. + * + * Rolldown keeps these calls as `__require()`, which a second bundler over the server output can't follow: + * - https://vite.dev/guide/migration#require-calls-for-externalized-modules + * - https://rolldown.rs/in-depth/bundling-cjs#require-external-modules + * + * Only handles requires from packages bundled for orchestrion, and from packages bundled because of them. + */ +export function createExternalRequireResolver( + moduleNames: readonly string[], +): (this: ThisParameterType, ...args: ResolveIdParameters) => Promise { + const bundledPackagesByEnvironment = new Map>(); + + return async function resolveExternalRequire(source, importer, options) { + if ( + !this.meta.rolldownVersion || + this.environment.mode !== 'build' || + options.kind !== 'require-call' || + !importer || + !isPackageSpecifier(source) + ) { + return null; + } + + const bundledPackages = bundledPackagesByEnvironment.get(this.environment.name) ?? new Set(moduleNames); + bundledPackagesByEnvironment.set(this.environment.name, bundledPackages); + + const owner = ownerPackage(importer); + const name = packageName(source); + if ( + !owner || + !bundledPackages.has(owner.name) || + isUserExternal(this.environment.config.resolve.external, source, name) || + isOptionalDependency(owner.dir, name) + ) { + return null; + } + + const resolved = await this.resolve(source, importer, { ...options, skipSelf: true }); + if (!resolved) { + return null; + } + // Vite bundles it anyway (e.g. under pnpm), so its own requires count too. + if (!resolved.external) { + bundledPackages.add(name); + return null; + } + + const file = resolveWithNode(source, importer); + if (file) { + bundledPackages.add(name); + } + return file ?? null; + }; +} + +function isPackageSpecifier(source: string): boolean { + return !isBuiltin(source) && !isAbsolute(source) && !/^[.\0]/.test(source); +} + +// `pg-types/lib/x` → `pg-types`, `@scope/pkg/x` → `@scope/pkg`. +function packageName(specifier: string): string { + const [first = '', second] = specifier.split('/'); + return first.startsWith('@') && second ? `${first}/${second}` : first; +} + +// Uses the last `node_modules` segment, so nested and pnpm store paths work. +function ownerPackage(file: string): { name: string; dir: string } | undefined { + const path = file.replace(/\\/g, '/'); + const start = path.lastIndexOf(NODE_MODULES); + if (start === -1) { + return undefined; + } + const name = packageName(path.slice(start + NODE_MODULES.length)); + return { name, dir: `${path.slice(0, start)}${NODE_MODULES}${name}` }; +} + +// Matches like Vite. `external: true` is Vite's SSR default, not a choice for this package. +function isUserExternal(external: readonly string[] | true, source: string, name: string): boolean { + return Array.isArray(external) && external.some(entry => entry === source || entry === name); +} + +// These can be missing or native, like `pg-native`. +function isOptionalDependency(packageDir: string, name: string): boolean { + const manifest = readManifest(`${packageDir}/package.json`); + if (!manifest) { + return false; + } + const { peerDependenciesMeta, optionalDependencies } = manifest; + const peerMeta = isPlainObject(peerDependenciesMeta) ? peerDependenciesMeta[name] : undefined; + return ( + (isPlainObject(peerMeta) && peerMeta.optional === true) || + (isPlainObject(optionalDependencies) && name in optionalDependencies) + ); +} + +// The file that `require()` loads at runtime. +function resolveWithNode(source: string, importer: string): string | undefined { + try { + return createRequire(importer).resolve(source); + } catch { + return undefined; + } +} + +function readManifest(file: string): Record | undefined { + try { + const manifest: unknown = JSON.parse(readFileSync(file, 'utf8')); + return isPlainObject(manifest) ? manifest : undefined; + } catch { + return undefined; + } +} diff --git a/packages/server-utils/src/orchestrion/bundler/vite.ts b/packages/server-utils/src/orchestrion/bundler/vite.ts index 6a8cb2ca235d..16e642dc76fd 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -1,12 +1,11 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/vite'; -import { resolve } from 'node:path'; -import type { ConfigEnv, ConfigPluginContext, Plugin, ResolvedConfig, UserConfig } from 'vite'; +import type { ConfigEnv, Plugin, ResolvedConfig, UserConfig } from 'vite'; export type { Plugin as VitePlugin } from 'vite'; import { instrumentedModuleNames } from '../config'; -import { commonJSDependencyNames } from './commonJSDependencies'; import type { PluginOptions } from './options'; import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; +import { createExternalRequireResolver } from './externalRequire'; import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './resolve'; type TransformHandler = (this: unknown, code: string, id: string, opts?: { ssr?: boolean }) => unknown; @@ -71,12 +70,11 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { } const upstream = codeTransformer(orchestrionTransformOptions(options)); - let commonJSDependencies: string[] = []; const noExternalModules = (): string[] => [ ...instrumentedModuleNames(options.instrumentations), '@sentry/server-utils', - ...commonJSDependencies, ]; + const resolveExternalRequire = createExternalRequireResolver(noExternalModules()); const instance = {}; // `true` when an earlier instance of this plugin is in the same build, which then transforms the modules alone. @@ -98,7 +96,10 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { // packages into default imports, and `@sentry/core` has no default export, so the server // crashes at startup. Vite 8 keeps the `require()`s, which loads a second copy of `@sentry/core`. async resolveId(source, importer, resolveOptions) { - if (source !== SNIPPET_IMPORT_SPECIFIER || !resolveOptions?.ssr) { + if (source !== SNIPPET_IMPORT_SPECIFIER) { + return resolveExternalRequire.call(this, source, importer, resolveOptions); + } + if (!resolveOptions?.ssr) { return null; } const resolved = await this.resolve(source, importer, { ...resolveOptions, skipSelf: true }); @@ -120,33 +121,14 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { // ESM entry, which crashes at startup. // Not in `serve`: Vite's dev SSR runner has no CJS interop, so inlined `mysql`/`ioredis` throw // `exports is not defined`, and the runtime hook injects the same publishers instead. - // Rolldown (Vite 8) emits a `require()` of an external package in bundled CommonJS as - // `__require()`, which a bundler that runs over the server output cannot follow. Rollup turns it - // into an `import`. So on Rolldown, the dependencies that this CommonJS requires are bundled too. config: { // Runs after the framework plugins, so `build.ssr` set by their `config` hooks is visible. order: 'post', - handler( - this: ConfigPluginContext | undefined, - config: UserConfig, - env?: ConfigEnv, - ): { ssr: { noExternal: string[] } } | null { - if (env?.command === 'serve') { - return null; - } - - // Vite 5 calls `config` hooks without a `this` context. - if (this?.meta.rolldownVersion) { - commonJSDependencies = commonJSDependencyNames( - resolve(config.root ?? process.cwd()), - instrumentedModuleNames(options.instrumentations), - ); - } - + handler(config: UserConfig, env?: ConfigEnv): { ssr: { noExternal: string[] } } | null { // A top-level `ssr` key makes Vite 6+ add an `ssr` environment with no entry, which // `vite build --app` cannot build. Vite 5 has no `configEnvironment` and needs the key, and // its SSR builds always set `build.ssr`. - if (!(config.ssr || config.build?.ssr)) { + if (env?.command === 'serve' || !(config.ssr || config.build?.ssr)) { return null; } diff --git a/packages/server-utils/test/orchestrion/bundler.test.ts b/packages/server-utils/test/orchestrion/bundler.test.ts index f5ae2c74e599..b0690ab74903 100644 --- a/packages/server-utils/test/orchestrion/bundler.test.ts +++ b/packages/server-utils/test/orchestrion/bundler.test.ts @@ -1,11 +1,10 @@ -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { isAbsolute, join } from 'node:path'; +import { existsSync } from 'node:fs'; +import { isAbsolute } from 'node:path'; import type { OnStartResult, PluginBuild } from 'esbuild'; import type { NormalizedInputOptions, PluginContext } from 'rollup'; -import type { Plugin, ResolvedConfig } from 'vite'; +import type { ResolvedConfig } from 'vite'; import type { Compiler } from 'webpack'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { sentryOrchestrionPlugin as bunPlugin } from '../../src/orchestrion/bundler/bun'; import { sentryOrchestrionPlugin as esbuildPlugin } from '../../src/orchestrion/bundler/esbuild'; import { orchestrionTransformOptions } from '../../src/orchestrion/bundler/options'; @@ -220,9 +219,6 @@ describe('sentryOrchestrionWebpackPlugin', () => { }); describe('sentryOrchestrionPlugin (vite)', () => { - const ROLLUP_CONTEXT = { meta: { rollupVersion: '4.0.0' } }; - const ROLLDOWN_CONTEXT = { meta: { rollupVersion: '4.0.0', rolldownVersion: '1.2.14' } }; - function runConfigResolved( ssrExternal: string[] | true | undefined, command: 'build' | 'serve' = 'build', @@ -238,26 +234,23 @@ describe('sentryOrchestrionPlugin (vite)', () => { return warn; } - // `context` is the `this` of the hook: Vite 6+ pass `meta`, Vite 5 passes nothing. function runConfig( command: 'build' | 'serve', userConfig: Record = { build: { ssr: true } }, - options: { plugin?: Plugin; context?: unknown } = {}, ): { ssr: { noExternal: string[] } } | null { - const { plugin = vitePlugin() } = options; - const context = 'context' in options ? options.context : ROLLUP_CONTEXT; + const plugin = vitePlugin(); const config = plugin.config as { - handler: (this: unknown, config: unknown, env: unknown) => { ssr: { noExternal: string[] } } | null; + handler: (config: unknown, env: unknown) => { ssr: { noExternal: string[] } } | null; }; - return config.handler.call(context, userConfig, { command, mode: 'production' }); + return config.handler.call(plugin, userConfig, { command, mode: 'production' }); } function runConfigEnvironment( name: string, environmentConfig: Record, command: 'build' | 'serve' = 'build', - plugin = vitePlugin(), ): { resolve: { noExternal: string[] } } | null { + const plugin = vitePlugin(); const configEnvironment = plugin.configEnvironment as ( name: string, config: unknown, @@ -290,45 +283,6 @@ describe('sentryOrchestrionPlugin (vite)', () => { expect(config?.ssr.noExternal).toContain('mysql'); }); - describe('dependencies of bundled CommonJS', () => { - let root: string; - - beforeEach(() => { - root = mkdtempSync(join(tmpdir(), 'orch-vite-cjs-deps-')); - mkdirSync(join(root, 'node_modules/pg'), { recursive: true }); - writeFileSync(join(root, 'node_modules/pg/package.json'), JSON.stringify({ dependencies: { 'pg-types': '^2' } })); - mkdirSync(join(root, 'node_modules/pg-types'), { recursive: true }); - writeFileSync(join(root, 'node_modules/pg-types/package.json'), '{}'); - }); - - afterEach(() => { - rmSync(root, { recursive: true, force: true }); - }); - - it('force-bundles them on Rolldown, which emits their requires as `__require()`', () => { - const config = runConfig('build', { root, build: { ssr: true } }, { context: ROLLDOWN_CONTEXT }); - - expect(config?.ssr.noExternal).toContain('pg-types'); - }); - - it('force-bundles them in server environments on Rolldown', () => { - const plugin = vitePlugin(); - - runConfig('build', { root }, { plugin, context: ROLLDOWN_CONTEXT }); - - expect(runConfigEnvironment('ssr', {}, 'build', plugin)?.resolve.noExternal).toContain('pg-types'); - }); - - it.each([ - ['Rollup', ROLLUP_CONTEXT], - ['Vite 5, which calls config hooks without a context', undefined], - ])('leaves them external on %s', (_, context) => { - const config = runConfig('build', { root, build: { ssr: true } }, { context }); - - expect(config?.ssr.noExternal).not.toContain('pg-types'); - }); - }); - it('does not force-bundle instrumented modules on the dev server', () => { // Inlined in dev, the CommonJS drivers throw `exports is not defined` on import. expect(runConfig('serve')).toBeNull(); @@ -492,6 +446,27 @@ describe('sentryOrchestrionPlugin (vite)', () => { expect(fallback).toMatch(/[/\\]build[/\\]esm[/\\]index\.js$/); expect(existsSync(fallback as string)).toBe(true); }); + + it('resolves the snippet require() to the ESM entry before it handles other requires on Rolldown', async () => { + const resolveId = vitePlugin().resolveId as ( + this: unknown, + source: string, + importer: string, + opts: { ssr: boolean; kind: string }, + ) => Promise; + const context = { + meta: { rolldownVersion: '1.2.14' }, + environment: { name: 'ssr', mode: 'build', config: { resolve: { external: [] } } }, + resolve: vi.fn().mockResolvedValue({ id: '/app/node_modules/@sentry/server-utils/build/cjs/index.js' }), + }; + + const resolved = await resolveId.call(context, '@sentry/server-utils', '/app/node_modules/pg/lib/client.js', { + ssr: true, + kind: 'require-call', + }); + + expect(resolved).toEqual({ id: '/app/node_modules/@sentry/server-utils/build/esm/index.js' }); + }); }); describe('sentryOrchestrionPlugin (bun)', () => { diff --git a/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts b/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts deleted file mode 100644 index 4126a542c8ad..000000000000 --- a/packages/server-utils/test/orchestrion/commonJSDependencies.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { commonJSDependencyNames } from '../../src/orchestrion/bundler/commonJSDependencies'; - -function installPackage(parentDir: string, name: string, manifest: Record = {}): string { - const dir = join(parentDir, 'node_modules', name); - mkdirSync(dir, { recursive: true }); - writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, ...manifest })); - return dir; -} - -describe('commonJSDependencyNames', () => { - let root: string; - - beforeEach(() => { - root = realpathSync(mkdtempSync(join(tmpdir(), 'orch-cjs-deps-'))); - }); - - afterEach(() => { - rmSync(root, { recursive: true, force: true }); - }); - - it('collects the dependencies of a CommonJS package transitively', () => { - installPackage(root, 'pg', { dependencies: { 'pg-types': '^2' } }); - installPackage(root, 'pg-types', { dependencies: { 'postgres-array': '~2' } }); - installPackage(root, 'postgres-array'); - - expect(commonJSDependencyNames(root, ['pg']).sort()).toEqual(['pg-types', 'postgres-array']); - }); - - it.each([ - ['an optional', { dependencies: { dep: '^1' }, optionalDependencies: { dep: '^1' } }], - ['a peer', { peerDependencies: { dep: '^1' } }], - ])('skips %s dependency', (_, manifest) => { - installPackage(root, 'pg', manifest); - installPackage(root, 'dep'); - - expect(commonJSDependencyNames(root, ['pg'])).toEqual([]); - }); - - it('skips a dependency that is not installed', () => { - installPackage(root, 'pg', { dependencies: { 'not-installed': '^1' } }); - - expect(commonJSDependencyNames(root, ['pg'])).toEqual([]); - }); - - it('includes an ESM dependency but not its own dependencies', () => { - installPackage(root, 'mysql', { dependencies: { 'esm-only': '^1' } }); - installPackage(root, 'esm-only', { type: 'module', dependencies: { 'esm-dep': '^1' } }); - installPackage(root, 'esm-dep'); - - expect(commonJSDependencyNames(root, ['mysql'])).toEqual(['esm-only']); - }); - - it.each([{ module: './esm/index.js' }, { type: 'module' }])( - 'skips an instrumented package with an ESM entry (%o)', - manifest => { - installPackage(root, 'graphql', { ...manifest, dependencies: { dep: '^1' } }); - installPackage(root, 'dep'); - - expect(commonJSDependencyNames(root, ['graphql'])).toEqual([]); - }, - ); - - it('resolves dependencies from the real location of a symlinked package', () => { - const store = join(root, 'node_modules/.pnpm/ioredis@5.0.0'); - const realDir = installPackage(store, 'ioredis', { dependencies: { denque: '^2' } }); - installPackage(store, 'denque'); - symlinkSync(realDir, join(root, 'node_modules/ioredis'), 'dir'); - - expect(commonJSDependencyNames(root, ['ioredis'])).toEqual(['denque']); - }); -}); diff --git a/packages/server-utils/test/orchestrion/externalRequire.test.ts b/packages/server-utils/test/orchestrion/externalRequire.test.ts new file mode 100644 index 000000000000..73951a3b971e --- /dev/null +++ b/packages/server-utils/test/orchestrion/externalRequire.test.ts @@ -0,0 +1,173 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createExternalRequireResolver } from '../../src/orchestrion/bundler/externalRequire'; + +interface ContextOptions { + environment?: string; + meta?: Record; + mode?: string; + external?: string[] | true; + resolved?: { id: string; external?: boolean } | null; +} + +describe('createExternalRequireResolver', () => { + let root: string; + let pgFile: string; + + function installPackage(name: string, manifest: Record = {}, parentDir = root): string { + const dir = join(parentDir, 'node_modules', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, main: 'index.js', ...manifest })); + writeFileSync(join(dir, 'index.js'), ''); + return join(dir, 'index.js'); + } + + // By default, a Rolldown build in which Vite keeps the required package external. + function context({ + environment = 'ssr', + meta = { rolldownVersion: '1.2.14' }, + mode = 'build', + external = [], + resolved = { id: 'external', external: true }, + }: ContextOptions = {}): never { + return { + meta, + environment: { name: environment, mode, config: { resolve: { external } } }, + resolve: vi.fn().mockResolvedValue(resolved), + } as never; + } + + function resolveRequire( + resolver: ReturnType, + source: string, + importer: string, + contextOptions?: ContextOptions, + kind = 'require-call', + ): Promise { + return resolver.call(context(contextOptions), source, importer, { kind, isEntry: false } as never); + } + + beforeEach(() => { + root = realpathSync(mkdtempSync(join(tmpdir(), 'orch-external-require-'))); + pgFile = join(root, 'node_modules/pg/lib/client.js'); + installPackage('pg'); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('bundles an external package that an instrumented package requires', async () => { + const pgTypes = installPackage('pg-types'); + + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile)).resolves.toBe(pgTypes); + }); + + it.each([ + ['it bundled', { id: 'external', external: true }], + ['Vite bundles itself', { id: 'pg-types/index.js' }], + ])('follows the requires of a package that %s', async (_, resolved) => { + const pgTypes = installPackage('pg-types'); + const postgresArray = installPackage('postgres-array'); + const resolver = createExternalRequireResolver(['pg']); + + await resolveRequire(resolver, 'pg-types', pgFile, { resolved }); + + await expect(resolveRequire(resolver, 'postgres-array', pgTypes)).resolves.toBe(postgresArray); + }); + + // Every require() here resolves to a real file, so only the filter can return `null`. + it.each([ + ['app code', 'pg-types', 'src/db.js'], + ['a package that orchestrion does not bundle', 'pg-types', 'node_modules/other/index.js'], + ['a builtin', 'node:net', 'node_modules/pg/lib/client.js'], + ['a relative path', './connection', 'node_modules/pg/lib/client.js'], + ])('ignores a require() of %s', async (_, source, importer) => { + installPackage('pg-types'); + installPackage('other'); + mkdirSync(join(root, 'node_modules/pg/lib'), { recursive: true }); + writeFileSync(join(root, 'node_modules/pg/lib/connection.js'), ''); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), source, join(root, importer)), + ).resolves.toBeNull(); + }); + + it('bundles a require() from a pnpm store path', async () => { + const store = join(root, 'node_modules/.pnpm/pg@8.23.1'); + installPackage('pg', {}, store); + const pgTypes = installPackage('pg-types', {}, store); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', join(store, 'node_modules/pg/lib/client.js')), + ).resolves.toBe(pgTypes); + }); + + it('tracks bundled packages for each environment separately', async () => { + const pgTypes = installPackage('pg-types'); + installPackage('postgres-array'); + const resolver = createExternalRequireResolver(['pg']); + + await resolveRequire(resolver, 'pg-types', pgFile, { environment: 'ssr' }); + + await expect(resolveRequire(resolver, 'postgres-array', pgTypes, { environment: 'nitro' })).resolves.toBeNull(); + }); + + it.each([ + ['on Rollup', { meta: { rollupVersion: '4.0.0' } }, 'require-call'], + ['on the dev server', { mode: 'dev' }, 'require-call'], + ['for an import', {}, 'import-statement'], + ])('leaves resolution to the bundler %s', async (_, contextOptions, kind) => { + installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, contextOptions, kind), + ).resolves.toBeNull(); + }); + + it.each([ + ['an optional peer', { peerDependenciesMeta: { 'pg-native': { optional: true } } }], + ['an optional dependency', { optionalDependencies: { 'pg-native': '^3' } }], + ])('keeps %s external', async (_, manifest) => { + installPackage('pg', manifest); + installPackage('pg-native'); + + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-native', pgFile)).resolves.toBeNull(); + }); + + it('keeps a scoped subpath external when the user lists its package', async () => { + installPackage('@scope/pkg'); + writeFileSync(join(root, 'node_modules/@scope/pkg/sub.js'), ''); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), '@scope/pkg/sub', pgFile, { external: ['@scope/pkg'] }), + ).resolves.toBeNull(); + }); + + it('keeps a package external that the user lists in `external`', async () => { + installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, { external: ['pg-types'] }), + ).resolves.toBeNull(); + }); + + it.each([ + ['only a subpath of the package', ['pg-types/lib/arrays']], + ['`true`, the SSR default', true], + ] as const)('bundles a package despite an external entry that names %s', async (_, external) => { + const pgTypes = installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, { + external: external === true ? true : [...external], + }), + ).resolves.toBe(pgTypes); + }); + + it('keeps a package external that is not installed', async () => { + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile)).resolves.toBeNull(); + }); +});