From 06b067c4140c554a82851a10a55ebcb72ab2a5db Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Sep 2023 17:13:31 +0200 Subject: [PATCH] fix(sourcemaps): Write package manager command instead of object to package.json (#453) fix a bug in our Sentry-CLI flow when we printed the package manager object (introduced in #417) to the package.json instead of the package manager's run command --- CHANGELOG.md | 1 + src/sourcemaps/tools/sentry-cli.ts | 25 +++++++----- src/utils/package-manager.ts | 18 ++++++--- test/sourcemaps/tools/sentry-cli.test.ts | 51 ++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 test/sourcemaps/tools/sentry-cli.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e2029d..709178ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - feat(remix): Pass `org`, `project`, `url` to `upload-sourcemaps` script (#434) - fix(nextjs): Add selfhosted url in `next.config.js` (#438) - fix(nextjs): Create necessary directories in app router (#439) +- fix(sourcemaps): Write package manager command instead of object to package.json (#453) Work in this release contributed by @andreysam. Thank you for your contributions! diff --git a/src/sourcemaps/tools/sentry-cli.ts b/src/sourcemaps/tools/sentry-cli.ts index 100ca690..89192ee2 100644 --- a/src/sourcemaps/tools/sentry-cli.ts +++ b/src/sourcemaps/tools/sentry-cli.ts @@ -1,5 +1,5 @@ // @ts-ignore - clack is ESM and TS complains about that. It works though -import clack from '@clack/prompts'; +import * as clack from '@clack/prompts'; import chalk from 'chalk'; import * as Sentry from '@sentry/node'; import * as path from 'path'; @@ -14,7 +14,7 @@ import { import { SourceMapUploadToolConfigurationOptions } from './types'; import { hasPackageInstalled, PackageDotJson } from '../../utils/package-json'; import { traceStep } from '../../telemetry'; -import { detectPackageManger } from '../../utils/package-manager'; +import { detectPackageManger, NPM } from '../../utils/package-manager'; const SENTRY_NPM_SCRIPT_NAME = 'sentry:sourcemaps'; @@ -194,7 +194,7 @@ async function askShouldAddToBuildCommand(): Promise { * * @param packageDotJson The package.json which will be modified. */ -async function addSentryCommandToBuildCommand( +export async function addSentryCommandToBuildCommand( packageDotJson: PackageDotJson, ): Promise { // This usually shouldn't happen because earlier we added the @@ -205,8 +205,7 @@ async function addSentryCommandToBuildCommand( (s) => s !== SENTRY_NPM_SCRIPT_NAME, ); - const packageManager = detectPackageManger(); - const packageManagerName = packageManager?.name ?? 'npm'; + const packageManager = detectPackageManger() ?? NPM; // Heuristic to pre-select the build command: // Often, 'build' is the prod build command, so we favour it. @@ -221,7 +220,7 @@ async function addSentryCommandToBuildCommand( (await abortIfCancelled( clack.confirm({ message: `Is ${chalk.cyan( - `${packageManagerName} run ${buildCommand}`, + `${packageManager.runScriptCommand} ${buildCommand}`, )} your production build command?`, }), )); @@ -229,7 +228,7 @@ async function addSentryCommandToBuildCommand( if (allNpmScripts.length && (!buildCommand || !isProdBuildCommand)) { buildCommand = await abortIfCancelled( clack.select({ - message: `Which ${packageManagerName} command in your ${chalk.cyan( + message: `Which ${packageManager.name} command in your ${chalk.cyan( 'package.json', )} builds your application for production?`, options: allNpmScripts @@ -252,10 +251,18 @@ Please add it manually to your prod build command.`, return; } + const oldCommand = packageDotJson.scripts[buildCommand]; + if (!oldCommand) { + // very unlikely to happen but nevertheless + clack.log.warn( + `\`${buildCommand}\` doesn't seem to be part of your package.json scripts`, + ); + return; + } + packageDotJson.scripts[ buildCommand - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - ] = `${packageDotJson.scripts[buildCommand]} && ${packageManager} run ${SENTRY_NPM_SCRIPT_NAME}`; + ] = `${oldCommand} && ${packageManager.runScriptCommand} ${SENTRY_NPM_SCRIPT_NAME}`; await fs.promises.writeFile( path.join(process.cwd(), 'package.json'), diff --git a/src/utils/package-manager.ts b/src/utils/package-manager.ts index 5e19457f..ff793a4b 100644 --- a/src/utils/package-manager.ts +++ b/src/utils/package-manager.ts @@ -10,38 +10,44 @@ export interface PackageManager { lockFile: string; installCommand: string; buildCommand: string; + /* The command that the package manager uses to run a script from package.json */ + runScriptCommand: string; } -const bun: PackageManager = { +export const BUN: PackageManager = { name: 'bun', label: 'Bun', lockFile: 'bun.lockb', installCommand: 'bun add', - buildCommand: 'bun build', + buildCommand: 'bun run build', + runScriptCommand: 'bun run', }; -const yarn: PackageManager = { +export const YARN: PackageManager = { name: 'yarn', label: 'Yarn', lockFile: 'yarn.lock', installCommand: 'yarn add', buildCommand: 'yarn build', + runScriptCommand: 'yarn', }; -const pnpm: PackageManager = { +export const PNPM: PackageManager = { name: 'pnpm', label: 'PNPM', lockFile: 'pnpm-lock.yaml', installCommand: 'pnpm add', buildCommand: 'pnpm build', + runScriptCommand: 'pnpm', }; -const npm: PackageManager = { +export const NPM: PackageManager = { name: 'npm', label: 'NPM', lockFile: 'package-lock.json', installCommand: 'npm add', buildCommand: 'npm run build', + runScriptCommand: 'npm run', }; -export const packageManagers = [bun, yarn, pnpm, npm]; +export const packageManagers = [BUN, YARN, PNPM, NPM]; export function detectPackageManger(): PackageManager | null { for (const packageManager of packageManagers) { diff --git a/test/sourcemaps/tools/sentry-cli.test.ts b/test/sourcemaps/tools/sentry-cli.test.ts new file mode 100644 index 00000000..30deb5d1 --- /dev/null +++ b/test/sourcemaps/tools/sentry-cli.test.ts @@ -0,0 +1,51 @@ +import * as fs from 'fs'; + +import { addSentryCommandToBuildCommand } from '../../../src/sourcemaps/tools/sentry-cli'; + +import * as packageManagerHelpers from '../../../src/utils/package-manager'; + +const writeFileSpy = jest + .spyOn(fs.promises, 'writeFile') + .mockImplementation(() => Promise.resolve()); + +jest.mock('@clack/prompts', () => { + return { + log: { + info: jest.fn(), + success: jest.fn(), + }, + confirm: jest.fn().mockResolvedValue(true), + isCancel: jest.fn().mockReturnValue(false), + }; +}); + +describe('addSentryCommandToBuildCommand', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it.each([ + [ + packageManagerHelpers.NPM, + packageManagerHelpers.PNPM, + packageManagerHelpers.YARN, + packageManagerHelpers.BUN, + ], + ])('adds the cli command to the script command (%s)', async (_, pacMan) => { + jest + .spyOn(packageManagerHelpers, 'detectPackageManger') + .mockReturnValue(pacMan); + const packageJson = { + scripts: { + build: 'tsc', + }, + version: '1.0.0', + }; + await addSentryCommandToBuildCommand(packageJson); + expect(writeFileSpy).toHaveBeenCalledWith( + expect.stringContaining('package.json'), + expect.stringContaining( + `tsc && ${pacMan.runScriptCommand} sentry:sourcemaps`, + ), + ); + }); +});