Skip to content

Commit

Permalink
fix(sourcemaps): Write package manager command instead of object to p…
Browse files Browse the repository at this point in the history
…ackage.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
  • Loading branch information
Lms24 authored Sep 13, 2023
1 parent 02de142 commit 06b067c
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand Down
25 changes: 16 additions & 9 deletions src/sourcemaps/tools/sentry-cli.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -194,7 +194,7 @@ async function askShouldAddToBuildCommand(): Promise<boolean> {
*
* @param packageDotJson The package.json which will be modified.
*/
async function addSentryCommandToBuildCommand(
export async function addSentryCommandToBuildCommand(
packageDotJson: PackageDotJson,
): Promise<void> {
// This usually shouldn't happen because earlier we added the
Expand All @@ -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.
Expand All @@ -221,15 +220,15 @@ async function addSentryCommandToBuildCommand(
(await abortIfCancelled(
clack.confirm({
message: `Is ${chalk.cyan(
`${packageManagerName} run ${buildCommand}`,
`${packageManager.runScriptCommand} ${buildCommand}`,
)} your production build command?`,
}),
));

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
Expand All @@ -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'),
Expand Down
18 changes: 12 additions & 6 deletions src/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions test/sourcemaps/tools/sentry-cli.test.ts
Original file line number Diff line number Diff line change
@@ -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`,
),
);
});
});

0 comments on commit 06b067c

Please sign in to comment.