Skip to content

Commit

Permalink
Rewrite logic to just add overrides and not install deps
Browse files Browse the repository at this point in the history
  • Loading branch information
andreiborza committed Nov 29, 2024
1 parent 970fcb8 commit 18c622a
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 145 deletions.
29 changes: 26 additions & 3 deletions e2e-tests/tests/nuxt-3.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,36 @@ async function runWizardOnNuxtProject(projectDir: string): Promise<void> {
'Please select your package manager.',
);

const tracingOptionPrompted =
const nitropackOverridePrompted =
packageManagerPrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
// Selecting `yarn` as the package manager
[KEYS.DOWN, KEYS.ENTER],
// Do you want to install version 2.9.7 of nitropack and add an override to package.json?
'Do you want to add an override for nitropack version 2.9.7?',
{
timeout: 240_000,
},
));

const nftOverridePrompted =
nitropackOverridePrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
// Selecting `yes` to downgrade nitropack
KEYS.ENTER,
'Do you want to add an override for @vercel/nft version ^0.27.4?',
// 'Do you want to install version',
{
timeout: 240_000,
},
));

const tracingOptionPrompted =
nftOverridePrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
KEYS.ENTER,
// "Do you want to enable Tracing", sometimes doesn't work as `Tracing` can be printed in bold.
'to track the performance of your application?',
'Do you want to enable',
{
timeout: 240_000,
},
Expand All @@ -59,7 +82,7 @@ async function runWizardOnNuxtProject(projectDir: string): Promise<void> {
const replayOptionPrompted =
tracingOptionPrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
[KEYS.ENTER],
KEYS.ENTER,
// "Do you want to enable Sentry Session Replay", sometimes doesn't work as `Sentry Session Replay` can be printed in bold.
'to get a video-like reproduction of errors during a user session?',
));
Expand Down
27 changes: 25 additions & 2 deletions e2e-tests/tests/nuxt-4.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,36 @@ async function runWizardOnNuxtProject(projectDir: string): Promise<void> {
'Please select your package manager.',
);

const tracingOptionPrompted =
const nitropackOverridePrompted =
packageManagerPrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
// Selecting `yarn` as the package manager
[KEYS.DOWN, KEYS.ENTER],
// Do you want to install version 2.9.7 of nitropack and add an override to package.json?
'Do you want to add an override for nitropack version 2.9.7?',
{
timeout: 240_000,
},
));

const nftOverridePrompted =
nitropackOverridePrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
// Selecting `yes` to downgrade nitropack
KEYS.ENTER,
'Do you want to add an override for @vercel/nft version ^0.27.4?',
// 'Do you want to install version',
{
timeout: 240_000,
},
));

const tracingOptionPrompted =
nftOverridePrompted &&
(await wizardInstance.sendStdinAndWaitForOutput(
KEYS.ENTER,
// "Do you want to enable Tracing", sometimes doesn't work as `Tracing` can be printed in bold.
'to track the performance of your application?',
'Do you want to enable',
{
timeout: 240_000,
},
Expand Down
5 changes: 4 additions & 1 deletion e2e-tests/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,10 @@ export function createFile(filePath: string, content?: string) {
* @param oldContent
* @param newContent
*/
export function modifyFile(filePath: string, replaceMap: Record<string, string>) {
export function modifyFile(
filePath: string,
replaceMap: Record<string, string>,
) {
const fileContent = fs.readFileSync(filePath, 'utf-8');
let newFileContent = fileContent;

Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"r2": "^2.0.1",
"read-env": "^1.3.0",
"recast": "^0.23.3",
"resolve": "^1.22.8",
"semver": "^7.5.3",
"xcode": "3.0.1",
"xml-js": "^1.6.11",
Expand Down
14 changes: 9 additions & 5 deletions src/nuxt/nuxt-wizard.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// @ts-ignore - clack is ESM and TS complains about that. It works though
import * as clack from '@clack/prompts';
import * as Sentry from '@sentry/node';
import { gte, lt, lte, minVersion } from 'semver';
import chalk from 'chalk';
import { lt, minVersion } from 'semver';
import type { WizardOptions } from '../utils/types';
import { traceStep, withTelemetry } from '../telemetry';
import {
Expand All @@ -14,6 +15,7 @@ import {
ensurePackageIsInstalled,
getOrAskForProjectData,
getPackageDotJson,
getPackageManager,
installPackage,
printWelcome,
runPrettierIfInstalled,
Expand All @@ -23,15 +25,14 @@ import {
addSDKModule,
getNuxtConfig,
createConfigFiles,
installExtraDepsIfNeeded,
addNuxtOverrides,
} from './sdk-setup';
import {
createExampleComponent,
createExamplePage,
supportsExamplePage,
} from './sdk-example';
import { isNuxtV4 } from './utils';
import chalk from 'chalk';

export function runNuxtWizard(options: WizardOptions) {
return withTelemetry(
Expand Down Expand Up @@ -90,16 +91,19 @@ export async function runNuxtWizardWithTelemetry(
const { authToken, selectedProject, selfHosted, sentryUrl } =
await getOrAskForProjectData(options, 'javascript-nuxt');

const packageManager = await getPackageManager();

await addNuxtOverrides(packageManager, minVer);

const sdkAlreadyInstalled = hasPackageInstalled('@sentry/nuxt', packageJson);
Sentry.setTag('sdk-already-installed', sdkAlreadyInstalled);

await installPackage({
packageName: '@sentry/nuxt',
alreadyInstalled: sdkAlreadyInstalled,
packageManager,
});

await installExtraDepsIfNeeded(minVer);

await addDotEnvSentryBuildPluginFile(authToken);

const nuxtConfig = await traceStep('load-nuxt-config', getNuxtConfig);
Expand Down
78 changes: 31 additions & 47 deletions src/nuxt/sdk-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ import {
import {
abort,
abortIfCancelled,
askShouldInstallPackage,
askShouldAddNuxtOverride,
featureSelectionPrompt,
installPackage,
isUsingTypeScript,
} from '../utils/clack-utils';
import { traceStep } from '../telemetry';
import { getInstalledPackageVersion } from '../utils/package-version';
import { gte, lt, minVersion, SemVer } from 'semver';
import { detectPackageManger, PackageManager } from '../utils/package-manager';
import { lt, SemVer } from 'semver';
import { PackageManager } from '../utils/package-manager';

const possibleNuxtConfig = [
'nuxt.config.js',
Expand Down Expand Up @@ -213,52 +211,38 @@ export async function createConfigFiles(dsn: string) {
}
}

export async function installExtraDepsIfNeeded(nuxtMinVer: SemVer | null) {
// We currently have some restrictions on the dependencies nuxt ships with
// and try to resolve these here for users if they agree.
// See: https://github.com/getsentry/sentry-javascript/issues/14514
await installExtraDepIfNeeded('nitropack', '2.9.7', (minVer) =>
gte(minVer, '2.10.0'),
);

await installExtraDepIfNeeded('@vercel/nft', '^0.27.4', (minVer) =>
lt(minVer, '0.27.4'),
);

if (nuxtMinVer && lt(nuxtMinVer, '3.14.0')) {
await installExtraDepIfNeeded('ofetch', '^1.4.0', (minVer) =>
lt(minVer, '1.4.0'),
);
}
}

export async function installExtraDepIfNeeded(
pkgName: string,
pkgVersion: string,
isNeeded: (minVer: SemVer) => boolean,
export async function addNuxtOverrides(
packageManager: PackageManager,
nuxtMinVer: SemVer | null,
) {
const installedVersion =
(await getInstalledPackageVersion(pkgName)) || '0.0.0';
const minVer = minVersion(installedVersion);
const overrides = [
{
pkgName: 'nitropack',
pkgVersion: '2.9.7',
},
{
pkgName: '@vercel/nft',
pkgVersion: '^0.27.4',
},
...(nuxtMinVer && lt(nuxtMinVer, '3.14.0')
? [{ pkgName: 'ofetch', pkgVersion: '^1.4.0' }]
: []),
];

clack.log.warn(
`To ensure Sentry can properly instrument your code it needs to add version overrides for some Nuxt dependencies.\n\nFor more info see: ${chalk.cyan(
'https://github.com/getsentry/sentry-javascript/issues/14514',
)}`,
);

if (!minVer || isNeeded(minVer)) {
clack.log.warn(
`You have version ${chalk.cyan(installedVersion)} of ${chalk.cyan(
pkgName,
)} installed.\nCurrently, we do not properly support this version (see https://github.com/getsentry/sentry-javascript/issues/14514).`,
for (const { pkgName, pkgVersion } of overrides) {
const shouldAddOverride = await askShouldAddNuxtOverride(
pkgName,
pkgVersion,
);

const shouldInstall = await askShouldInstallPackage(pkgName, pkgVersion);

if (shouldInstall) {
const { packageManager } = await installPackage({
packageName: `${pkgName}@${pkgVersion}`,
alreadyInstalled: false,
askBeforeUpdating: false,
packageNameDisplayLabel: `${pkgName}@${pkgVersion}`,
});

await packageManager?.addOverride(pkgName, pkgVersion);
if (shouldAddOverride) {
await packageManager.addOverride(pkgName, pkgVersion);
}
}
}
24 changes: 12 additions & 12 deletions src/utils/clack-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,15 @@ export async function installPackage({
alreadyInstalled,
askBeforeUpdating = true,
packageNameDisplayLabel,
packageManager,
}: {
/** The string that is passed to the package manager CLI as identifier to install (e.g. `@sentry/nextjs`, or `@sentry/nextjs@^8`) */
packageName: string;
alreadyInstalled: boolean;
askBeforeUpdating?: boolean;
/** Overrides what is shown in the installation logs in place of the `packageName` option. Useful if the `packageName` is ugly (e.g. `@sentry/nextjs@^8`) */
packageNameDisplayLabel?: string;
packageManager?: PackageManager;
}): Promise<{ packageManager?: PackageManager }> {
return traceStep('install-package', async () => {
if (alreadyInstalled && askBeforeUpdating) {
Expand All @@ -380,18 +382,18 @@ export async function installPackage({

const sdkInstallSpinner = clack.spinner();

const packageManager = await getPackageManager();
const pkgManager = packageManager || (await getPackageManager());

sdkInstallSpinner.start(
`${alreadyInstalled ? 'Updating' : 'Installing'} ${chalk.bold.cyan(
packageNameDisplayLabel ?? packageName,
)} with ${chalk.bold(packageManager.label)}.`,
)} with ${chalk.bold(pkgManager.label)}.`,
);

try {
await new Promise<void>((resolve, reject) => {
childProcess.exec(
`${packageManager.installCommand} ${packageName} ${packageManager.flags}`,
`${pkgManager.installCommand} ${packageName} ${pkgManager.flags}`,
(err, stdout, stderr) => {
if (err) {
// Write a log file so we can better troubleshoot issues
Expand Down Expand Up @@ -430,10 +432,10 @@ export async function installPackage({
sdkInstallSpinner.stop(
`${alreadyInstalled ? 'Updated' : 'Installed'} ${chalk.bold.cyan(
packageNameDisplayLabel ?? packageName,
)} with ${chalk.bold(packageManager.label)}.`,
)} with ${chalk.bold(pkgManager.label)}.`,
);

return { packageManager };
return { packageManager: pkgManager };
});
}

Expand Down Expand Up @@ -1490,18 +1492,16 @@ export async function featureSelectionPrompt<F extends ReadonlyArray<Feature>>(
});
}

export async function askShouldInstallPackage(
export async function askShouldAddNuxtOverride(
pkgName: string,
pkgVersion: string,
): Promise<boolean> {
return traceStep(`ask-install-package-${pkgName}`, () =>
return traceStep(`ask-add-nuxt-overrides`, () =>
abortIfCancelled(
clack.confirm({
message: `Do you want to install version ${chalk.cyan(
pkgVersion,
)} of ${chalk.cyan(pkgName)} and add an override to ${chalk.cyan(
'package.json',
)}?`,
message: `Do you want to add an override for ${chalk.cyan(
pkgName,
)} version ${chalk.cyan(pkgVersion)}?`,
}),
),
);
Expand Down
46 changes: 0 additions & 46 deletions src/utils/package-version.ts

This file was deleted.

Loading

0 comments on commit 18c622a

Please sign in to comment.