diff --git a/.changeset/pr-1237.md b/.changeset/pr-1237.md new file mode 100644 index 0000000000..805084eff5 --- /dev/null +++ b/.changeset/pr-1237.md @@ -0,0 +1,6 @@ + +--- +'@sanity/cli': patch +--- + +fix: don't treat ignored build scripts as failure during dep install \ No newline at end of file diff --git a/packages/@sanity/cli/src/util/packageManager/__tests__/installPackages.test.ts b/packages/@sanity/cli/src/util/packageManager/__tests__/installPackages.test.ts index a1589652d6..1ac829d272 100644 --- a/packages/@sanity/cli/src/util/packageManager/__tests__/installPackages.test.ts +++ b/packages/@sanity/cli/src/util/packageManager/__tests__/installPackages.test.ts @@ -67,6 +67,7 @@ describe('installDeclaredPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.start).toHaveBeenCalled() @@ -88,6 +89,7 @@ describe('installDeclaredPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -107,6 +109,7 @@ describe('installDeclaredPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -126,6 +129,7 @@ describe('installDeclaredPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -190,6 +194,7 @@ describe('installNewPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.start).toHaveBeenCalled() @@ -214,6 +219,7 @@ describe('installNewPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -234,6 +240,7 @@ describe('installNewPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -254,6 +261,7 @@ describe('installNewPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() @@ -317,12 +325,193 @@ describe('installNewPackages', () => { cwd: workDir, encoding: 'utf8', env: {PATH: '/mock/path'}, + reject: false, stdio: 'pipe', }) expect(mockSpinnerInstance.succeed).toHaveBeenCalled() }) }) +describe('pnpm ignored build scripts', () => { + const workDir = '/test/project' + const context = {output: mockOutput, workDir} + + const approveBuildsNotice = + 'pnpm skipped build scripts for some dependencies. Run "pnpm approve-builds" in the project directory to pick which dependencies should be allowed to run scripts.' + + test('treats ignored esbuild build script as success without notice', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: [ + ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.28.0', + '', + 'Run "pnpm approve-builds" to pick which dependencies should be allowed to run scripts.', + ].join('\n'), + stdout: 'Packages: +123', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockSpinnerInstance.fail).not.toHaveBeenCalled() + expect(mockOutput.warn).not.toHaveBeenCalled() + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('treats ignored builds as success but prints notice for non-esbuild packages', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: [ + ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.28.0, sharp@0.34.1.', + '', + 'Run "pnpm approve-builds" to pick which dependencies should be allowed to run scripts.', + ].join('\n'), + stdout: 'Packages: +123', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockSpinnerInstance.fail).not.toHaveBeenCalled() + expect(mockOutput.warn).toHaveBeenCalledWith(approveBuildsNotice) + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('prints notice for scoped packages with ignored build scripts', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stdout: ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: @tailwindcss/oxide@4.0.0', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockOutput.warn).toHaveBeenCalledWith(approveBuildsNotice) + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('handles line-wrapped error output', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: [ + ' ERR_PNPM_IGNORED_BUILDS Ignored build', + 'scripts: esbuild@0.28.0,', + 'sharp@0.34.1.', + '', + 'Run "pnpm approve-builds" to pick which dependencies should be allowed to run scripts.', + ].join('\n'), + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockSpinnerInstance.fail).not.toHaveBeenCalled() + expect(mockOutput.warn).toHaveBeenCalledWith(approveBuildsNotice) + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('does not print notice when wrapped output only skips esbuild', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: [ + ' ERR_PNPM_IGNORED_BUILDS Ignored build', + 'scripts: esbuild@0.28.0', + '', + 'Run "pnpm approve-builds" to pick which dependencies should be allowed to run scripts.', + ].join('\n'), + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockOutput.warn).not.toHaveBeenCalled() + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('applies to installNewPackages as well', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.28.0', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installNewPackages({packageManager: 'pnpm', packages: ['lodash']}, context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockOutput.warn).not.toHaveBeenCalled() + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('does not apply ignored builds handling for other package managers', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stdout: ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.28.0', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'npm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.fail).toHaveBeenCalled() + expect(mockOutput.error).toHaveBeenCalledWith('Dependency installation failed', {exit: 1}) + }) + + test('prints notice for whitespace-separated package list', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stdout: ' ERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.28.0 sharp@0.34.1', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.succeed).toHaveBeenCalled() + expect(mockOutput.warn).toHaveBeenCalledWith(approveBuildsNotice) + expect(mockOutput.error).not.toHaveBeenCalled() + }) + + test('still fails pnpm installs without the ignored builds marker', async () => { + const mockResult: Partial = { + exitCode: 1, + failed: true, + stderr: ' ERR_PNPM_FETCH_404 GET https://registry.npmjs.org/nope: Not Found - 404', + stdout: 'Packages: +0', + } + mockExeca.mockResolvedValueOnce(mockResult as Result) + + await installDeclaredPackages(workDir, 'pnpm', context) + + expect(mockExeca).toHaveBeenCalledTimes(1) + expect(mockSpinnerInstance.fail).toHaveBeenCalled() + expect(mockOutput.warn).not.toHaveBeenCalled() + // The actionable error lives on stderr, so it must be surfaced to the user. + expect(mockOutput.log).toHaveBeenCalledWith( + expect.stringContaining('ERR_PNPM_FETCH_404 GET https://registry.npmjs.org/nope'), + ) + expect(mockOutput.error).toHaveBeenCalledWith('Dependency installation failed', {exit: 1}) + }) +}) + describe('error handling edge cases', () => { const workDir = '/test/project' const context = {output: mockOutput, workDir} diff --git a/packages/@sanity/cli/src/util/packageManager/installPackages.ts b/packages/@sanity/cli/src/util/packageManager/installPackages.ts index 98a6b99e6c..5d5d572b49 100644 --- a/packages/@sanity/cli/src/util/packageManager/installPackages.ts +++ b/packages/@sanity/cli/src/util/packageManager/installPackages.ts @@ -34,6 +34,36 @@ const PACKAGE_MANAGER_COMMANDS: PackageManagerCommands = { }, } +const IGNORED_BUILDS_NOTICE = + 'pnpm skipped build scripts for some dependencies. Run "pnpm approve-builds" in the project directory to pick which dependencies should be allowed to run scripts.' + +// Matches pnpm's `ERR_PNPM_IGNORED_BUILDS` error against whitespace-normalized +// output (pnpm may wrap the message), capturing the list of skipped packages, +// eg `esbuild@0.25.0, sharp@0.34.0.` - the `@` token sequence +// ends the capture where the trailing `Run "pnpm approve-builds"…` hint starts +const IGNORED_BUILDS_PATTERN = + /ERR_PNPM_IGNORED_BUILDS.*?Ignored build scripts: ?((?:[^\s,]+@[^\s,]+[, ]*)+)/ + +function getIgnoredBuildScripts(commandOutput: string): string[] | undefined { + const match = commandOutput.replaceAll(/\s+/g, ' ').match(IGNORED_BUILDS_PATTERN) + if (!match) { + return undefined + } + + // The capture allows both comma and whitespace separators (pnpm may print + // either, and wrapping can drop the comma), so split on both. + return match[1] + .split(/[\s,]+/) + .map((entry) => entry.replace(/\.$/, '')) + .filter(Boolean) +} + +function isEsbuild(ignoredEntry: string): boolean { + // Entries are on the form `@` - strip the version, + // keeping in mind that scoped package names also start with `@` + return ignoredEntry.replace(/@[^@]+$/, '') === 'esbuild' +} + async function executePackageManagerCommand( packageManager: PackageManagerLibs, args: string[], @@ -46,8 +76,28 @@ async function executePackageManagerCommand( const result = await execa(packageManager, args, execOptions) if (result?.exitCode || result?.failed) { + // pnpm exits non-zero if dependency build scripts were skipped, even though + // the install itself succeeded. Treat it as a success, but point to + // `pnpm approve-builds` if anything other than esbuild was skipped + // (esbuild works without its build script through a JS fallback). + const commandOutput = [result.stdout, result.stderr] + .filter((chunk): chunk is string => typeof chunk === 'string') + .join('\n') + const ignoredBuilds = + packageManager === 'pnpm' ? getIgnoredBuildScripts(commandOutput) : undefined + + if (ignoredBuilds) { + progress.succeed() + if (ignoredBuilds.some((entry) => !isEsbuild(entry))) { + output.warn(IGNORED_BUILDS_NOTICE) + } + return + } + progress.fail() - output.log(String(result.stdout)) + // Log both streams - package managers often print the actionable error + // details to stderr, so logging stdout alone can hide the failure reason. + output.log(commandOutput) output.error(errorMessage, {exit: 1}) } else { progress.succeed() @@ -64,6 +114,7 @@ export async function installDeclaredPackages( cwd, encoding: 'utf8', env: getPartialEnvWithNpmPath(cwd), + reject: false, stdio: 'pipe', } @@ -92,6 +143,7 @@ export async function installNewPackages( cwd: workDir, encoding: 'utf8', env: getPartialEnvWithNpmPath(workDir), + reject: false, stdio: 'pipe', }