Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(cli): show warning when using deprecated version #102

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion packages/aws-cdk/lib/cli/util/npm.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { exec as _exec } from 'child_process';
import { promisify } from 'util';
import * as semver from 'semver';
import { promisify } from 'util';
import { debug } from '../../logging';
import { ToolkitError } from '../../toolkit/error';

Expand All @@ -19,3 +19,17 @@ export async function getLatestVersionFromNpm(): Promise<string> {

return latestVersion;
}

export async function checkIfDeprecated(version: string): Promise<string | null> {
try {
const { stdout, stderr } = await exec(`npm view aws-cdk@${version} deprecated --silent`, { timeout: 3000 });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you work this into the existing version check in some way? (it may be that you will need to change that existing check to return information about more than 1 version at a time).

I'd like to avoid doing unnecessary calls to NPM.

Copy link
Contributor Author

@tttol tttol Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rix0rrr
It's true that my implementation results in unnecessary NPM calls. But I couldn't find the logic of version check in existing source code. Which logic/file are you referring when you said work this into the existing version check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@tttol tttol Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rix0rrr
Do you mean that calling checkIfDeprecated out of if (laterVersion) block is better?
Ref: https://github.com/aws/aws-cdk-cli/blob/main/packages/aws-cdk/lib/cli/version.ts#L125

    try {
      const laterVersion = await latestVersionIfHigher(currentVersion, versionCheckCache ?? new VersionCheckTTL());
+    const deprecationMessage = await checkIfDeprecated(currentVersion);
+    if (deprecationMessage) {
+      info(`You are using deprecated version(aws-cdk@${currentVersion}): ${deprecationMessage}`);
+    }
      if (laterVersion) {
        const versionMessage = await getVersionMessage(currentVersion, laterVersion);

if (stderr) {
debug(`The 'npm view aws-cdk@${version} deprecated --silent' command generated an error stream with content [${stderr.trim()}]`);
}

return stdout ? stdout.trim() : null;
} catch (err) {
debug(`Failed to check package deprecation status - ${err}`);
return null;
}
}
14 changes: 9 additions & 5 deletions packages/aws-cdk/lib/cli/version.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* istanbul ignore file */
import * as path from 'path';
import * as chalk from 'chalk';
import * as fs from 'fs-extra';
import * as path from 'path';
import * as semver from 'semver';
import { debug, info } from '../logging';
import { ToolkitError } from '../toolkit/error';
import { formatAsBanner } from './util/console-formatters';
import { cdkCacheDir, rootDir } from '../util/directories';
import { getLatestVersionFromNpm } from './util/npm';
import { formatAsBanner } from './util/console-formatters';
import { checkIfDeprecated, getLatestVersionFromNpm } from './util/npm';

const ONE_DAY_IN_SECONDS = 1 * 24 * 60 * 60;

Expand Down Expand Up @@ -106,8 +106,11 @@ function getMajorVersionUpgradeMessage(currentVersion: string): string | void {
}
}

function getVersionMessage(currentVersion: string, laterVersion: string): string[] {
async function getVersionMessage(currentVersion: string, laterVersion: string): Promise<string[]> {
const deprecationMessage = await checkIfDeprecated(currentVersion);

return [
deprecationMessage ? `You are using deprecated version(aws-cdk@${currentVersion}): ${deprecationMessage}` : '',
`Newer version of CDK is available [${chalk.green(laterVersion as string)}]`,
getMajorVersionUpgradeMessage(currentVersion),
'Upgrade recommended (npm install -g aws-cdk)',
Expand All @@ -122,7 +125,8 @@ export async function displayVersionMessage(currentVersion = versionNumber(), ve
try {
const laterVersion = await latestVersionIfHigher(currentVersion, versionCheckCache ?? new VersionCheckTTL());
if (laterVersion) {
const bannerMsg = formatAsBanner(getVersionMessage(currentVersion, laterVersion));
const versionMessage = await getVersionMessage(currentVersion, laterVersion);
const bannerMsg = formatAsBanner(versionMessage);
bannerMsg.forEach((e) => info(e));
}
} catch (err: any) {
Expand Down
55 changes: 55 additions & 0 deletions packages/aws-cdk/test/cli/util/npm.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { checkIfDeprecated } from '../../../lib/cli/util/npm';

jest.mock('util', () => {
const mockExec = jest.fn();
const format = jest.fn((fmt, ...args) => {
return [fmt, ...args].join(' ');
});
return {
promisify: jest.fn(() => mockExec),
__mockExec: mockExec,
format,
};
});

const { __mockExec: mockedExec } = jest.requireMock('util');

describe('npm.ts', () => {
beforeEach(() => {
jest.resetAllMocks();
});

describe('checkIfDeprecated', () => {
test('returns null for non-deprecated version', async () => {
mockedExec.mockResolvedValue({
stdout: '',
stderr: '',
});

const result = await checkIfDeprecated('2.1.0');

expect(result).toBeNull();
expect(mockedExec).toHaveBeenCalledWith('npm view aws-cdk@2.1.0 deprecated --silent', { timeout: 3000 });
});

test('returns deprecation message for deprecated version', async () => {
const deprecationMessage = 'This version has been deprecated';
mockedExec.mockImplementation(() => Promise.resolve({
stdout: deprecationMessage,
stderr: '',
}));

const result = await checkIfDeprecated('1.0.0');

expect(result).toBe(deprecationMessage);
});

test('returns null when error occurs', async () => {
mockedExec.mockImplementation(() => Promise.reject(new Error('npm error')));

const result = await checkIfDeprecated('2.1.0');

expect(result).toBeNull();
});
});
});
25 changes: 20 additions & 5 deletions packages/aws-cdk/test/cli/version.test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
/* eslint-disable import/order */
import * as path from 'path';
import { setTimeout as _setTimeout } from 'timers';
import { promisify } from 'util';
import * as fs from 'fs-extra';
import * as os from 'os';
import * as path from 'path';
import * as sinon from 'sinon';
import * as logging from '../../lib/logging';
import { setTimeout as _setTimeout } from 'timers';
import { promisify } from 'util';
import * as npm from '../../lib/cli/util/npm';
import { latestVersionIfHigher, VersionCheckTTL, displayVersionMessage, isDeveloperBuild } from '../../lib/cli/version';
import { displayVersionMessage, isDeveloperBuild, latestVersionIfHigher, VersionCheckTTL } from '../../lib/cli/version';
import * as logging from '../../lib/logging';

jest.setTimeout(10_000);


const setTimeout = promisify(_setTimeout);

function tmpfile(): string {
Expand Down Expand Up @@ -143,6 +144,20 @@ describe('version message', () => {
expect(printSpy).toHaveBeenCalledWith(expect.stringContaining('100.0.0'));
expect(printSpy).not.toHaveBeenCalledWith(expect.stringContaining('Information about upgrading from 99.x to 100.x'));
});

test('Prints a message when a deprecated version is used', async () => {
// Given the current version is 1.0.0 and the latest version is 1.1.0
const currentVersion = '1.0.0';
jest.spyOn(npm, 'getLatestVersionFromNpm').mockResolvedValueOnce('1.1.0');
jest.spyOn(npm, 'checkIfDeprecated').mockResolvedValueOnce('aws-cdk@1.0.0 has been deprecated.');
const printSpy = jest.spyOn(logging, 'info');

// When displayVersionMessage is called
await displayVersionMessage(currentVersion, new VersionCheckTTL(tmpfile(), 0));

// Then the deprecated version message is printed
expect(printSpy).toHaveBeenCalledWith(expect.stringContaining('You are using deprecated version(aws-cdk@1.0.0):'));
});
});

test('isDeveloperBuild call does not throw an error', () => {
Expand Down
Loading