From a01d82993a4f745b0b53aecc94de6aa0d6fe9d75 Mon Sep 17 00:00:00 2001 From: Christopher Pruijsen Date: Fri, 11 Sep 2026 09:08:26 +0100 Subject: [PATCH] fix(sv-utils): detect pnpm version from the target project, not the invoker cwd --- .changeset/sharp-pianos-detect.md | 6 +++ documentation/docs/50-api/20-sv-utils.md | 6 ++- packages/sv-utils/api-surface.md | 10 +++- packages/sv-utils/src/pnpm-internals.ts | 11 +++- packages/sv-utils/src/pnpm.ts | 50 +++++++++++++++-- packages/sv-utils/src/tests/pnpm.ts | 60 ++++++++++++++++----- packages/sv/src/addons/sveltekit-adapter.ts | 2 +- packages/sv/src/core/package-manager.ts | 2 +- 8 files changed, 124 insertions(+), 23 deletions(-) create mode 100644 .changeset/sharp-pianos-detect.md diff --git a/.changeset/sharp-pianos-detect.md b/.changeset/sharp-pianos-detect.md new file mode 100644 index 000000000..f26a63d2c --- /dev/null +++ b/.changeset/sharp-pianos-detect.md @@ -0,0 +1,6 @@ +--- +'@sveltejs/sv-utils': patch +'sv': patch +--- + +fix(sv-utils): detect pnpm version from the target project, not the invoker cwd diff --git a/documentation/docs/50-api/20-sv-utils.md b/documentation/docs/50-api/20-sv-utils.md index 2450af76a..f5934b399 100644 --- a/documentation/docs/50-api/20-sv-utils.md +++ b/documentation/docs/50-api/20-sv-utils.md @@ -275,16 +275,18 @@ Lower-level building blocks, both reading candidate files through an injected `r Returns a transform for `pnpm-workspace.yaml` that adds packages to the pnpm "allow builds" config. Use with `sv.file` when the project uses pnpm. -The helper detects the installed pnpm version via `pnpm --version`: +The helper detects the pnpm version via `pnpm --version` in the given `cwd` (or `os.tmpdir()` when omitted, so the invoker's `packageManager` pin is not used): - pnpm `>= 11`: writes to the unified `allowBuilds` map (`{ pkg: true }`), migrating any legacy `onlyBuiltDependencies` list into the map. - pnpm `< 11`: writes to the legacy `onlyBuiltDependencies` list. +Pass `{ cwd }` so detection matches the target project. Pass `{ pnpmVersion }` to skip detection. + ```js // @noErrors import { pnpm } from '@sveltejs/sv-utils'; if (packageManager === 'pnpm') { - sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('my-native-dep')); + sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('my-native-dep', { cwd })); } ``` diff --git a/packages/sv-utils/api-surface.md b/packages/sv-utils/api-surface.md index 800791e10..533892a2c 100644 --- a/packages/sv-utils/api-surface.md +++ b/packages/sv-utils/api-surface.md @@ -824,10 +824,18 @@ declare const transforms: { text(cb: (file: { content: string; text: typeof text_d_exports }) => string | false): TransformFn; }; declare namespace pnpm_d_exports { - export { allowBuilds }; + export { AllowBuildsOptions, allowBuilds }; } +type AllowBuildsOptions = { + cwd?: string; + pnpmVersion?: string | number; +}; declare function allowBuilds(...packages: string[]): TransformFn; +declare function allowBuilds( + ...args: [...packages: string[], options: AllowBuildsOptions] +): TransformFn; +declare function allowBuilds(packages: string[], options?: AllowBuildsOptions): TransformFn; type Version = { major?: number; minor?: number; diff --git a/packages/sv-utils/src/pnpm-internals.ts b/packages/sv-utils/src/pnpm-internals.ts index a36f3697c..b92f332c5 100644 --- a/packages/sv-utils/src/pnpm-internals.ts +++ b/packages/sv-utils/src/pnpm-internals.ts @@ -1,11 +1,18 @@ import { execSync } from 'node:child_process'; +import os from 'node:os'; import { coerceVersion } from './semver.ts'; -export function detectPnpmMajor(): number | undefined { +/** + * Detects the major version of pnpm that would run in `cwd`. + * Defaults to `os.tmpdir()` so detection is not pinned by the invoker's + * `packageManager` / `devEngines.packageManager` field. + */ +export function detectPnpmMajor(cwd = os.tmpdir()): number | undefined { try { const out = execSync('pnpm --version', { encoding: 'utf-8', - stdio: ['ignore', 'pipe', 'ignore'] + stdio: ['ignore', 'pipe', 'ignore'], + cwd }); return coerceVersion(out.trim()).major; } catch { diff --git a/packages/sv-utils/src/pnpm.ts b/packages/sv-utils/src/pnpm.ts index cd5e6566f..8b3ff59dd 100644 --- a/packages/sv-utils/src/pnpm.ts +++ b/packages/sv-utils/src/pnpm.ts @@ -1,4 +1,5 @@ import { detectPnpmMajor } from './pnpm-internals.ts'; +import { coerceVersion } from './semver.ts'; import { transforms, type TransformFn } from './tooling/transforms.ts'; type YamlMap = { @@ -17,6 +18,24 @@ type YamlDoc = { createNode(value: unknown, options?: { flow?: boolean }): unknown; }; +export type AllowBuildsOptions = { + /** Directory whose pnpm version should be detected. */ + cwd?: string; + /** Explicit pnpm version; skips `pnpm --version` detection. */ + pnpmVersion?: string | number; +}; + +function isAllowBuildsOptions(value: unknown): value is AllowBuildsOptions { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function resolvePnpmMajor(options?: AllowBuildsOptions): number | undefined { + if (options?.pnpmVersion !== undefined) { + return coerceVersion(String(options.pnpmVersion)).major; + } + return detectPnpmMajor(options?.cwd); +} + /** * Returns a TransformFn for `pnpm-workspace.yaml` that adds packages to the * pnpm "allow builds" config. @@ -26,14 +45,39 @@ type YamlDoc = { * migrating any legacy `onlyBuiltDependencies` list into the map; * - on pnpm `< 11` writes to the legacy `onlyBuiltDependencies` list. * + * Pass `{ cwd }` so detection uses the target project rather than the + * invoker's working directory. Pass `{ pnpmVersion }` to skip detection. + * * ```ts * if (packageManager === 'pnpm') { - * sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('my-native-dep')); + * sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('my-native-dep', { cwd })); * } * ``` */ -export function allowBuilds(...packages: string[]): TransformFn { - const major = detectPnpmMajor(); +export function allowBuilds(...packages: string[]): TransformFn; +export function allowBuilds( + ...args: [...packages: string[], options: AllowBuildsOptions] +): TransformFn; +export function allowBuilds(packages: string[], options?: AllowBuildsOptions): TransformFn; +export function allowBuilds( + first?: string | string[] | AllowBuildsOptions, + second?: string | AllowBuildsOptions, + ...rest: Array +): TransformFn { + const args: Array = []; + if (first !== undefined) args.push(first); + if (second !== undefined) args.push(second); + args.push(...rest); + + let options: AllowBuildsOptions | undefined; + const last = args.at(-1); + if (isAllowBuildsOptions(last)) { + options = last; + args.pop(); + } + + const packages = args.length === 1 && Array.isArray(args[0]) ? args[0] : (args as string[]); + const major = resolvePnpmMajor(options); if (major !== undefined && major < 11) return writeLegacy(packages); return writeAllowBuilds(packages); } diff --git a/packages/sv-utils/src/tests/pnpm.ts b/packages/sv-utils/src/tests/pnpm.ts index e6d29d70e..8d55c8d83 100644 --- a/packages/sv-utils/src/tests/pnpm.ts +++ b/packages/sv-utils/src/tests/pnpm.ts @@ -1,13 +1,15 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { describe, expect, it } from 'vitest'; import { detectPnpmMajor } from '../pnpm-internals.ts'; import { allowBuilds } from '../pnpm.ts'; -const major = detectPnpmMajor(); -const isPnpm11 = major === undefined || major >= 11; +describe('allowBuilds (pnpm >= 11: writes allowBuilds map)', () => { + const transform = (pkg: string) => allowBuilds(pkg, { pnpmVersion: 11 }); -describe.runIf(isPnpm11)('allowBuilds (pnpm >= 11: writes allowBuilds map)', () => { it('creates allowBuilds map in empty file', () => { - expect(allowBuilds('esbuild')('')).toBe('allowBuilds:\n esbuild: true\n'); + expect(transform('esbuild')('')).toBe('allowBuilds:\n esbuild: true\n'); }); it('appends to existing allowBuilds map', () => { @@ -16,7 +18,7 @@ describe.runIf(isPnpm11)('allowBuilds (pnpm >= 11: writes allowBuilds map)', () allowBuilds: bar: true `; - expect(allowBuilds('esbuild')(input)).toBe(`packages: + expect(transform('esbuild')(input)).toBe(`packages: - 'packages/*' allowBuilds: bar: true @@ -28,7 +30,7 @@ allowBuilds: const input = `allowBuilds: core-js: false `; - expect(allowBuilds('esbuild')(input)).toBe(`allowBuilds: + expect(transform('esbuild')(input)).toBe(`allowBuilds: core-js: false esbuild: true `); @@ -41,7 +43,7 @@ onlyBuiltDependencies: - foo - bar `; - expect(allowBuilds('esbuild')(input)).toBe(`packages: + expect(transform('esbuild')(input)).toBe(`packages: - 'packages/*' allowBuilds: foo: true @@ -56,7 +58,7 @@ allowBuilds: allowBuilds: shared: false `; - expect(allowBuilds('newone')(input)).toBe(`allowBuilds: + expect(transform('newone')(input)).toBe(`allowBuilds: shared: false newone: true `); @@ -66,20 +68,22 @@ allowBuilds: const input = `allowBuilds: esbuild: true `; - expect(allowBuilds('esbuild')(input)).toBe(input); + expect(transform('esbuild')(input)).toBe(input); }); }); -describe.runIf(!isPnpm11)('allowBuilds (pnpm < 11: writes onlyBuiltDependencies list)', () => { +describe('allowBuilds (pnpm < 11: writes onlyBuiltDependencies list)', () => { + const transform = (pkg: string) => allowBuilds(pkg, { pnpmVersion: 10 }); + it('creates onlyBuiltDependencies list in empty file', () => { - expect(allowBuilds('esbuild')('')).toBe('onlyBuiltDependencies:\n - esbuild\n'); + expect(transform('esbuild')('')).toBe('onlyBuiltDependencies:\n - esbuild\n'); }); it('appends to existing onlyBuiltDependencies list', () => { const input = `onlyBuiltDependencies: - foo `; - expect(allowBuilds('esbuild')(input)).toBe(`onlyBuiltDependencies: + expect(transform('esbuild')(input)).toBe(`onlyBuiltDependencies: - foo - esbuild `); @@ -89,6 +93,36 @@ describe.runIf(!isPnpm11)('allowBuilds (pnpm < 11: writes onlyBuiltDependencies const input = `onlyBuiltDependencies: - esbuild `; - expect(allowBuilds('esbuild')(input)).toBe(input); + expect(transform('esbuild')(input)).toBe(input); + }); +}); + +describe('allowBuilds version detection', () => { + it('accepts an array of packages plus options', () => { + expect(allowBuilds(['esbuild', 'workerd'], { pnpmVersion: 10 })('')).toBe( + 'onlyBuiltDependencies:\n - esbuild\n - workerd\n' + ); + }); + + it('accepts trailing options after rest package names', () => { + expect(allowBuilds('esbuild', 'workerd', { pnpmVersion: 11 })('')).toBe( + 'allowBuilds:\n esbuild: true\n workerd: true\n' + ); + }); + + it('detects pnpm from the target cwd, not process.cwd()', { timeout: 30_000 }, () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sv-pnpm-')); + try { + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ name: 'pin11', packageManager: 'pnpm@11.0.0' }) + ); + + expect(detectPnpmMajor(process.cwd())).toBe(10); + expect(detectPnpmMajor(dir)).toBe(11); + expect(allowBuilds('esbuild', { cwd: dir })('')).toBe('allowBuilds:\n esbuild: true\n'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } }); }); diff --git a/packages/sv/src/addons/sveltekit-adapter.ts b/packages/sv/src/addons/sveltekit-adapter.ts index 76e4d8589..128985677 100644 --- a/packages/sv/src/addons/sveltekit-adapter.ts +++ b/packages/sv/src/addons/sveltekit-adapter.ts @@ -113,7 +113,7 @@ export default defineAddon({ sv.devDependency('wrangler', '^4.97.0'); if (packageManager === 'pnpm') { - sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('workerd')); + sv.file(file.findUp('pnpm-workspace.yaml'), pnpm.allowBuilds('workerd', { cwd })); } // default to jsonc diff --git a/packages/sv/src/core/package-manager.ts b/packages/sv/src/core/package-manager.ts index aaa02837c..c7548fc47 100644 --- a/packages/sv/src/core/package-manager.ts +++ b/packages/sv/src/core/package-manager.ts @@ -135,6 +135,6 @@ export function addPnpmAllowBuilds( const found = find.up('pnpm-workspace.yaml', { cwd }); const filePath = found ?? path.join(cwd, 'pnpm-workspace.yaml'); const content = found ? fs.readFileSync(found, 'utf-8') : ''; - const newContent = pnpm.allowBuilds(...packages)(content); + const newContent = pnpm.allowBuilds(packages, { cwd })(content); if (newContent && newContent !== content) fs.writeFileSync(filePath, newContent, 'utf-8'); }