From 2d42a8459d629626fd66558838b7d131a09efc56 Mon Sep 17 00:00:00 2001 From: Roshane Pascual Date: Thu, 9 Jan 2025 13:21:51 -0800 Subject: [PATCH] PR feedback --- .../backend-deployer/src/cdk_deployer.test.ts | 2 +- .../npm_lock_file_reader.test.ts | 14 +++++- .../lock-file-reader/npm_lock_file_reader.ts | 42 ++++++++-------- .../pnpm_lock_file_reader.test.ts | 48 +++++++++++++++++++ .../lock-file-reader/pnpm_lock_file_reader.ts | 16 ++++--- .../lock-file-reader/types.ts | 9 ++++ .../yarn_classic_lock_file_reader.test.ts | 17 ++++++- .../yarn_classic_lock_file_reader.ts | 19 ++++---- .../yarn_modern_lock_file_reader.test.ts | 25 +++++++++- .../yarn_modern_lock_file_reader.ts | 18 ++++--- .../npm_package_manager_controller.test.ts | 46 +++++------------- .../package_manager_controller_base.ts | 17 ++----- .../pnpm_package_manager_controller.test.ts | 46 +++++------------- ...classic_package_manager_controller.test.ts | 47 +++++------------- ..._modern_package_manager_controller.test.ts | 48 +++++-------------- packages/cli/src/ampx.ts | 2 +- .../sandbox/sandbox_command_factory.ts | 2 +- .../src/amplify_project_creator.test.ts | 2 +- .../initial_project_file_generator.test.ts | 2 +- packages/platform-core/API.md | 2 +- .../src/usage-data/usage_data_emitter.test.ts | 25 +++++++++- .../src/usage-data/usage_data_emitter.ts | 13 +++-- .../usage-data/usage_data_emitter_factory.ts | 2 +- packages/plugin-types/API.md | 12 +---- .../src/package_manager_controller.ts | 10 +--- 25 files changed, 251 insertions(+), 235 deletions(-) create mode 100644 packages/cli-core/src/package-manager-controller/lock-file-reader/types.ts diff --git a/packages/backend-deployer/src/cdk_deployer.test.ts b/packages/backend-deployer/src/cdk_deployer.test.ts index 39af9db733..254cb0ff3d 100644 --- a/packages/backend-deployer/src/cdk_deployer.test.ts +++ b/packages/backend-deployer/src/cdk_deployer.test.ts @@ -46,7 +46,7 @@ void describe('invokeCDKCommand', () => { runWithPackageManager: mock.fn(() => Promise.resolve() as never), getCommand: (args: string[]) => `'npx ${args.join(' ')}'`, allowsSignalPropagation: () => true, - getDependencies: mock.fn(() => Promise.resolve([])), + tryGetDependencies: mock.fn(() => Promise.resolve([])), }; const invoker = new CDKDeployer( diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.test.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.test.ts index fc1eadbec1..fff5b1c46b 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.test.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.test.ts @@ -52,10 +52,22 @@ void describe('NpmLockFileReader', () => { assert.strictEqual(fspReadFileMock.mock.callCount(), 1); }); - void it('returns empty lock file contents when package-lock.json is not present or parse-able', async () => { + void it('returns undefined when package-lock.json is not present or parse-able', async () => { fspReadFileMock.mock.mockImplementationOnce(() => Promise.reject(new Error()) ); + const lockFileContents = + await npmLockFileReader.getLockFileContentsFromCwd(); + assert.deepEqual(lockFileContents, undefined); + }); + + void it('returns empty dependency array when package-lock.json does not have dependencies', async () => { + fspReadFileMock.mock.mockImplementationOnce(() => + JSON.stringify({ + name: 'test_project', + version: '1.0.0', + }) + ); const lockFileContents = await npmLockFileReader.getLockFileContentsFromCwd(); assert.deepEqual(lockFileContents, { dependencies: [] }); diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.ts index 4aa3c81143..6f412085b5 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.ts @@ -1,11 +1,8 @@ -import { - Dependency, - LockFileContents, - LockFileReader, -} from '@aws-amplify/plugin-types'; +import { Dependency } from '@aws-amplify/plugin-types'; import fsp from 'fs/promises'; import path from 'path'; import z from 'zod'; +import { LockFileContents, LockFileReader } from './types.js'; import { printer } from '../../printer.js'; import { LogLevel } from '../../printer/printer.js'; @@ -13,39 +10,44 @@ import { LogLevel } from '../../printer/printer.js'; * NpmLockFileReader is an abstraction around the logic used to read and parse lock file contents */ export class NpmLockFileReader implements LockFileReader { - getLockFileContentsFromCwd = async (): Promise => { + getLockFileContentsFromCwd = async (): Promise< + LockFileContents | undefined + > => { const dependencies: Array = []; const packageLockJsonPath = path.resolve( process.cwd(), 'package-lock.json' ); - - let jsonLockParsedValue: Record; + let packageLockJson; try { const jsonLockContents = await fsp.readFile(packageLockJsonPath, 'utf-8'); - jsonLockParsedValue = JSON.parse(jsonLockContents); + const jsonLockParsedValue = JSON.parse(jsonLockContents); + // This will strip fields that are not part of the package lock schema + packageLockJson = packageLockJsonSchema.parse(jsonLockParsedValue); } catch (error) { printer.log( `Failed to get lock file contents because ${packageLockJsonPath} does not exist or is not parse-able`, LogLevel.DEBUG ); - return { dependencies }; + return; } - // This will strip fields that are not part of the package lock schema - const packageLockJson = packageLockJsonSchema.parse(jsonLockParsedValue); - for (const key in packageLockJson.packages) { if (key === '') { // Skip root project in packages continue; } - // Remove "node_modules/" prefix - const dependencyName = key.replace(/^node_modules\//, ''); - dependencies.push({ - name: dependencyName, - version: packageLockJson.packages[key].version, - }); + const dependencyVersion = packageLockJson.packages[key].version; + + // Version may not exist if package is a symbolic link + if (dependencyVersion) { + // Remove "node_modules/" prefix + const dependencyName = key.replace(/^node_modules\//, ''); + dependencies.push({ + name: dependencyName, + version: dependencyVersion, + }); + } } return { dependencies }; @@ -57,7 +59,7 @@ const packageLockJsonSchema = z.object({ .record( z.string(), z.object({ - version: z.string(), + version: z.string().optional(), }) ) .optional(), diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.test.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.test.ts index 2c99265ff4..443a5f6e5c 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.test.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.test.ts @@ -90,6 +90,54 @@ packages: fspReadFileMock.mock.mockImplementationOnce(() => Promise.reject(new Error()) ); + const lockFileContents = + await pnpmLockFileReader.getLockFileContentsFromCwd(); + assert.deepEqual(lockFileContents, undefined); + }); + + void it('returns empty dependency array when pnpm-lock.yaml does not have dependencies', async () => { + mock.method( + fsp, + 'readFile', + () => `lockfileVersion: '9.0' + + settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + + importers: + + .: + dependencies: + aws-amplify: + specifier: ^6.12.0 + version: 6.12.0 + devDependencies: + '@aws-amplify/backend': + specifier: ^1.11.0 + version: 1.12.0(@aws-sdk/client-cloudformation@3.723.0)(@aws-sdk/client-s3@3.723.0)(@aws-sdk/client-sso-oidc@3.621.0(@aws-sdk/client-sts@3.621.0))(@aws-sdk/types@3.723.0)(aws-cdk-lib@2.174.1(constructs@10.4.2))(constructs@10.4.2)(zod@3.24.1) + '@aws-amplify/backend-cli': + specifier: ^1.4.5 + version: 1.4.6(@aws-sdk/client-sso-oidc@3.621.0(@aws-sdk/client-sts@3.621.0))(@aws-sdk/client-sts@3.621.0)(@aws-sdk/types@3.723.0)(aws-cdk-lib@2.174.1(constructs@10.4.2))(aws-cdk@2.174.1)(constructs@10.4.2)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)(typescript@5.7.2) + aws-cdk: + specifier: ^2.173.4 + version: 2.174.1 + aws-cdk-lib: + specifier: ^2.173.4 + version: 2.174.1(constructs@10.4.2) + constructs: + specifier: ^10.4.2 + version: 10.4.2 + esbuild: + specifier: ^0.24.2 + version: 0.24.2 + tsx: + specifier: ^4.19.2 + version: 4.19.2 + typescript: + specifier: ^5.7.2 + version: 5.7.2` + ); const lockFileContents = await pnpmLockFileReader.getLockFileContentsFromCwd(); assert.deepEqual(lockFileContents, { dependencies: [] }); diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.ts index 86e1478881..caed7ecaf6 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.ts @@ -1,10 +1,7 @@ -import { - Dependency, - LockFileContents, - LockFileReader, -} from '@aws-amplify/plugin-types'; +import { Dependency } from '@aws-amplify/plugin-types'; import fsp from 'fs/promises'; import path from 'path'; +import { LockFileContents, LockFileReader } from './types.js'; import { printer } from '../../printer.js'; import { LogLevel } from '../../printer/printer.js'; @@ -12,7 +9,9 @@ import { LogLevel } from '../../printer/printer.js'; * PnpmLockFileReader is an abstraction around the logic used to read and parse lock file contents */ export class PnpmLockFileReader implements LockFileReader { - getLockFileContentsFromCwd = async (): Promise => { + getLockFileContentsFromCwd = async (): Promise< + LockFileContents | undefined + > => { const eolRegex = '[\r\n]'; const dependencies: Array = []; const pnpmLockPath = path.resolve(process.cwd(), 'pnpm-lock.yaml'); @@ -24,6 +23,9 @@ export class PnpmLockFileReader implements LockFileReader { ); const startOfPackagesIndex = pnpmLockContentsArray.indexOf('packages:'); + if (startOfPackagesIndex === -1) { + return { dependencies }; + } const pnpmLockPackages = pnpmLockContentsArray.slice( startOfPackagesIndex + 1 ); @@ -49,7 +51,7 @@ export class PnpmLockFileReader implements LockFileReader { `Failed to get lock file contents because ${pnpmLockPath} does not exist or is not parse-able`, LogLevel.DEBUG ); - return { dependencies }; + return; } return { dependencies }; diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/types.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/types.ts new file mode 100644 index 0000000000..6762c4a30d --- /dev/null +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/types.ts @@ -0,0 +1,9 @@ +import { Dependency } from '@aws-amplify/plugin-types'; + +export type LockFileContents = { + dependencies: Array; +}; + +export type LockFileReader = { + getLockFileContentsFromCwd: () => Promise; +}; diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.test.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.test.ts index fb3585106e..38afb7860b 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.test.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.test.ts @@ -56,10 +56,25 @@ some_other_dep@12.13.14: assert.strictEqual(fspReadFileMock.mock.callCount(), 1); }); - void it('returns empty lock file contents when yarn.lock is not present or parse-able', async () => { + void it('returns undefined when yarn.lock is not present or parse-able', async () => { fspReadFileMock.mock.mockImplementationOnce(() => Promise.reject(new Error()) ); + const lockFileContents = + await yarnClassicLockFileReader.getLockFileContentsFromCwd(); + assert.deepEqual(lockFileContents, undefined); + }); + + void it('returns empty dependency array when yarn.lock does not have dependencies', async () => { + mock.method( + fsp, + 'readFile', + () => `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +` + ); const lockFileContents = await yarnClassicLockFileReader.getLockFileContentsFromCwd(); assert.deepEqual(lockFileContents, { dependencies: [] }); diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.ts index 8abc4779b1..cd12059485 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.ts @@ -1,10 +1,7 @@ -import { - Dependency, - LockFileContents, - LockFileReader, -} from '@aws-amplify/plugin-types'; +import { Dependency } from '@aws-amplify/plugin-types'; import fsp from 'fs/promises'; import path from 'path'; +import { LockFileContents, LockFileReader } from './types.js'; import { printer } from '../../printer.js'; import { LogLevel } from '../../printer/printer.js'; @@ -12,16 +9,18 @@ import { LogLevel } from '../../printer/printer.js'; * YarnClassicLockFileReader is an abstraction around the logic used to read and parse lock file contents */ export class YarnClassicLockFileReader implements LockFileReader { - getLockFileContentsFromCwd = async (): Promise => { + getLockFileContentsFromCwd = async (): Promise< + LockFileContents | undefined + > => { const eolRegex = '[\r\n]'; const dependencies: Array = []; const yarnLockPath = path.resolve(process.cwd(), 'yarn.lock'); try { const yarnLockContents = await fsp.readFile(yarnLockPath, 'utf-8'); - const yarnLockContentsArray = yarnLockContents.split( - new RegExp(`${eolRegex}${eolRegex}`) - ); + const yarnLockContentsArray = yarnLockContents + .trim() + .split(new RegExp(`${eolRegex}${eolRegex}`)); // Slice to remove comment block at the start of the lock file for (const yarnDependencyBlock of yarnLockContentsArray.slice(1)) { @@ -45,7 +44,7 @@ export class YarnClassicLockFileReader implements LockFileReader { `Failed to get lock file contents because ${yarnLockPath} does not exist or is not parse-able`, LogLevel.DEBUG ); - return { dependencies }; + return; } return { dependencies }; diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.test.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.test.ts index 1b1a5213a5..461b88f66a 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.test.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.test.ts @@ -63,10 +63,33 @@ __metadata: assert.strictEqual(fspReadFileMock.mock.callCount(), 1); }); - void it('returns empty lock file contents when yarn.lock is not present or parse-able', async () => { + void it('returns undefined when yarn.lock is not present or parse-able', async () => { fspReadFileMock.mock.mockImplementationOnce(() => Promise.reject(new Error()) ); + const lockFileContents = + await yarnModernLockFileReader.getLockFileContentsFromCwd(); + assert.deepEqual(lockFileContents, undefined); + }); + + void it('returns empty dependency array when yarn.lock does not have dependencies', async () => { + mock.method( + fsp, + 'readFile', + () => `# This file is generated by running "yarn install" inside your project. +# Manual changes might be lost - proceed with caution! + +__metadata: + version: 8 + cacheKey: 10c0 + +"testapp@workspace:.": + version: 0.0.0-use.local + resolution: "testapp@workspace:." + languageName: unknown + linkType: soft +` + ); const lockFileContents = await yarnModernLockFileReader.getLockFileContentsFromCwd(); assert.deepEqual(lockFileContents, { dependencies: [] }); diff --git a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.ts b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.ts index 3c8423f5e9..868c6dc8d0 100644 --- a/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.ts +++ b/packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.ts @@ -1,10 +1,7 @@ -import { - Dependency, - LockFileContents, - LockFileReader, -} from '@aws-amplify/plugin-types'; +import { Dependency } from '@aws-amplify/plugin-types'; import fsp from 'fs/promises'; import path from 'path'; +import { LockFileContents, LockFileReader } from './types.js'; import { printer } from '../../printer.js'; import { LogLevel } from '../../printer/printer.js'; @@ -12,7 +9,9 @@ import { LogLevel } from '../../printer/printer.js'; * YarnModernLockFileReader is an abstraction around the logic used to read and parse lock file contents */ export class YarnModernLockFileReader implements LockFileReader { - getLockFileContentsFromCwd = async (): Promise => { + getLockFileContentsFromCwd = async (): Promise< + LockFileContents | undefined + > => { const eolRegex = '[\r\n]'; const dependencies: Array = []; const yarnLockPath = path.resolve(process.cwd(), 'yarn.lock'); @@ -23,6 +22,11 @@ export class YarnModernLockFileReader implements LockFileReader { new RegExp(`${eolRegex}${eolRegex}`) ); + if (yarnLockContentsArray.length === 3) { + // Contents are only comment block, metadata, and workspace info + return { dependencies }; + } + // Slice to remove comment block and metadata at the start of the lock file for (const yarnDependencyBlock of yarnLockContentsArray.slice(2)) { const yarnDependencyLines = yarnDependencyBlock @@ -45,7 +49,7 @@ export class YarnModernLockFileReader implements LockFileReader { `Failed to get lock file contents because ${yarnLockPath} does not exist or is not parse-able`, LogLevel.DEBUG ); - return { dependencies }; + return; } return { dependencies }; diff --git a/packages/cli-core/src/package-manager-controller/npm_package_manager_controller.test.ts b/packages/cli-core/src/package-manager-controller/npm_package_manager_controller.test.ts index 072bd1c14c..4f7aa2d9db 100644 --- a/packages/cli-core/src/package-manager-controller/npm_package_manager_controller.test.ts +++ b/packages/cli-core/src/package-manager-controller/npm_package_manager_controller.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { execa } from 'execa'; import { NpmPackageManagerController } from './npm_package_manager_controller.js'; import { executeWithDebugLogger } from './execute_with_debugger_logger.js'; -import { LockFileReader } from '@aws-amplify/plugin-types'; +import { LockFileReader } from './lock-file-reader/types.js'; void describe('NpmPackageManagerController', () => { const fspMock = { @@ -127,9 +127,8 @@ void describe('NpmPackageManagerController', () => { }); void describe('getDependencies', () => { - const existsSyncMock = mock.fn(() => true); - void it('successfully returns dependency versions', async () => { + const existsSyncMock = mock.fn(() => true); const lockFileReaderMock = { getLockFileContentsFromCwd: async () => Promise.resolve({ @@ -163,7 +162,7 @@ void describe('NpmPackageManagerController', () => { lockFileReaderMock ); const dependencyVersions = - await npmPackageManagerController.getDependencies(); + await npmPackageManagerController.tryGetDependencies(); const expectedVersions = [ { name: 'aws-cdk', @@ -173,40 +172,17 @@ void describe('NpmPackageManagerController', () => { name: 'aws-cdk-lib', version: '12.13.14', }, + { + name: 'test_dep', + version: '1.23.45', + }, + { + name: 'some_other_dep', + version: '12.1.3', + }, ]; assert.deepEqual(dependencyVersions, expectedVersions); }); - - void it('returns empty array if there are no matching dependencies', async () => { - const lockFileReaderMock = { - getLockFileContentsFromCwd: async () => - Promise.resolve({ - dependencies: [ - { - name: 'test_dep', - version: '1.23.45', - }, - { - name: 'some_other_dep', - version: '12.1.3', - }, - ], - }), - } as LockFileReader; - const npmPackageManagerController = new NpmPackageManagerController( - '/testProjectRoot', - fspMock as unknown as typeof fsp, - pathMock as unknown as typeof path, - execaMock as unknown as typeof execa, - executeWithDebugLoggerMock as unknown as typeof executeWithDebugLogger, - existsSyncMock, - lockFileReaderMock - ); - const dependencyVersions = - await npmPackageManagerController.getDependencies(); - - assert.deepEqual(dependencyVersions, []); - }); }); }); diff --git a/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts b/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts index 8946fec30e..be83839c87 100644 --- a/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts +++ b/packages/cli-core/src/package-manager-controller/package_manager_controller_base.ts @@ -6,13 +6,13 @@ import { Dependency, ExecaChildProcess, ExecaOptions, - LockFileReader, type PackageManagerController, } from '@aws-amplify/plugin-types'; import { LogLevel } from '../printer/printer.js'; import { printer } from '../printer.js'; import { executeWithDebugLogger as _executeWithDebugLogger } from './execute_with_debugger_logger.js'; import { getPackageManagerRunnerName } from './get_package_manager_name.js'; +import { LockFileReader } from './lock-file-reader/types.js'; /** * PackageManagerController is an abstraction around package manager commands that are needed to initialize a project and install dependencies @@ -149,22 +149,13 @@ export abstract class PackageManagerControllerBase allowsSignalPropagation = () => true; /** - * getDependencies - Retrieves dependency versions from the lock file in the project root + * tryGetDependencies - Tries to retrieve dependency versions from the lock file in the project root */ - getDependencies = async (): Promise> => { + tryGetDependencies = async (): Promise | undefined> => { const lockFileContents = await this.lockFileReader.getLockFileContentsFromCwd(); - const targetDependencies = ['aws-cdk', 'aws-cdk-lib']; - const dependencyVersions: Array = []; - - for (const { name, version } of lockFileContents.dependencies) { - if (targetDependencies.includes(name)) { - dependencyVersions.push({ name, version }); - } - } - - return dependencyVersions; + return lockFileContents?.dependencies; }; /** diff --git a/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.test.ts b/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.test.ts index 83b631390f..3e3c7bc342 100644 --- a/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.test.ts +++ b/packages/cli-core/src/package-manager-controller/pnpm_package_manager_controller.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { execa } from 'execa'; import { PnpmPackageManagerController } from './pnpm_package_manager_controller.js'; import { executeWithDebugLogger } from './execute_with_debugger_logger.js'; -import { LockFileReader } from '@aws-amplify/plugin-types'; +import { LockFileReader } from './lock-file-reader/types.js'; void describe('PnpmPackageManagerController', () => { const fspMock = { @@ -127,9 +127,8 @@ void describe('PnpmPackageManagerController', () => { }); void describe('getDependencies', () => { - const existsSyncMock = mock.fn(() => true); - void it('successfully returns dependency versions', async () => { + const existsSyncMock = mock.fn(() => true); const lockFileReaderMock = { getLockFileContentsFromCwd: async () => Promise.resolve({ @@ -163,7 +162,7 @@ void describe('PnpmPackageManagerController', () => { lockFileReaderMock ); const dependencyVersions = - await pnpmPackageManagerController.getDependencies(); + await pnpmPackageManagerController.tryGetDependencies(); const expectedVersions = [ { name: 'aws-cdk', @@ -173,40 +172,17 @@ void describe('PnpmPackageManagerController', () => { name: 'aws-cdk-lib', version: '12.13.14', }, + { + name: 'test_dep', + version: '1.23.45', + }, + { + name: 'some_other_dep', + version: '12.1.3', + }, ]; assert.deepEqual(dependencyVersions, expectedVersions); }); - - void it('returns empty array if there are no matching dependencies', async () => { - const lockFileReaderMock = { - getLockFileContentsFromCwd: async () => - Promise.resolve({ - dependencies: [ - { - name: 'test_dep', - version: '1.23.45', - }, - { - name: 'some_other_dep', - version: '12.1.3', - }, - ], - }), - } as LockFileReader; - const pnpmPackageManagerController = new PnpmPackageManagerController( - '/testProjectRoot', - fspMock as unknown as typeof fsp, - pathMock as unknown as typeof path, - execaMock as unknown as typeof execa, - executeWithDebugLoggerMock as unknown as typeof executeWithDebugLogger, - existsSyncMock, - lockFileReaderMock - ); - const dependencyVersions = - await pnpmPackageManagerController.getDependencies(); - - assert.deepEqual(dependencyVersions, []); - }); }); }); diff --git a/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.test.ts b/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.test.ts index 00ac91fc6e..4c35e60e77 100644 --- a/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.test.ts +++ b/packages/cli-core/src/package-manager-controller/yarn_classic_package_manager_controller.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { execa } from 'execa'; import { YarnClassicPackageManagerController } from './yarn_classic_package_manager_controller.js'; import { executeWithDebugLogger } from './execute_with_debugger_logger.js'; -import { LockFileReader } from '@aws-amplify/plugin-types'; +import { LockFileReader } from './lock-file-reader/types.js'; void describe('YarnClassicPackageManagerController', () => { const fspMock = { @@ -131,9 +131,8 @@ void describe('YarnClassicPackageManagerController', () => { }); void describe('getDependencies', () => { - const existsSyncMock = mock.fn(() => true); - void it('successfully returns dependency versions', async () => { + const existsSyncMock = mock.fn(() => true); const lockFileReaderMock = { getLockFileContentsFromCwd: async () => Promise.resolve({ @@ -168,7 +167,7 @@ void describe('YarnClassicPackageManagerController', () => { lockFileReaderMock ); const dependencyVersions = - await yarnClassicPackageManagerController.getDependencies(); + await yarnClassicPackageManagerController.tryGetDependencies(); const expectedVersions = [ { name: 'aws-cdk', @@ -178,41 +177,17 @@ void describe('YarnClassicPackageManagerController', () => { name: 'aws-cdk-lib', version: '12.13.14', }, + { + name: 'test_dep', + version: '1.23.45', + }, + { + name: 'some_other_dep', + version: '12.1.3', + }, ]; assert.deepEqual(dependencyVersions, expectedVersions); }); - - void it('returns empty array if there are no matching dependencies', async () => { - const lockFileReaderMock = { - getLockFileContentsFromCwd: async () => - Promise.resolve({ - dependencies: [ - { - name: 'test_dep', - version: '1.23.45', - }, - { - name: 'some_other_dep', - version: '12.1.3', - }, - ], - }), - } as LockFileReader; - const yarnClassicPackageManagerController = - new YarnClassicPackageManagerController( - '/testProjectRoot', - fspMock as unknown as typeof fsp, - pathMock as unknown as typeof path, - execaMock as unknown as typeof execa, - executeWithDebugLoggerMock as unknown as typeof executeWithDebugLogger, - existsSyncMock, - lockFileReaderMock - ); - const dependencyVersions = - await yarnClassicPackageManagerController.getDependencies(); - - assert.deepEqual(dependencyVersions, []); - }); }); }); diff --git a/packages/cli-core/src/package-manager-controller/yarn_modern_package_manager_controller.test.ts b/packages/cli-core/src/package-manager-controller/yarn_modern_package_manager_controller.test.ts index 215c573948..83119e1aef 100644 --- a/packages/cli-core/src/package-manager-controller/yarn_modern_package_manager_controller.test.ts +++ b/packages/cli-core/src/package-manager-controller/yarn_modern_package_manager_controller.test.ts @@ -6,7 +6,7 @@ import { execa } from 'execa'; import { Printer } from '../printer/printer.js'; import { YarnModernPackageManagerController } from './yarn_modern_package_manager_controller.js'; import { executeWithDebugLogger } from './execute_with_debugger_logger.js'; -import { LockFileReader } from '@aws-amplify/plugin-types'; +import { LockFileReader } from './lock-file-reader/types.js'; void describe('YarnModernPackageManagerController', () => { const fspMock = { @@ -126,9 +126,8 @@ void describe('YarnModernPackageManagerController', () => { }); void describe('getDependencies', () => { - const existsSyncMock = mock.fn(() => true); - void it('successfully returns dependency versions', async () => { + const existsSyncMock = mock.fn(() => true); const lockFileReaderMock = { getLockFileContentsFromCwd: async () => Promise.resolve({ @@ -164,7 +163,7 @@ void describe('YarnModernPackageManagerController', () => { lockFileReaderMock ); const dependencyVersions = - await yarnModernPackageManagerController.getDependencies(); + await yarnModernPackageManagerController.tryGetDependencies(); const expectedVersions = [ { name: 'aws-cdk', @@ -174,42 +173,17 @@ void describe('YarnModernPackageManagerController', () => { name: 'aws-cdk-lib', version: '12.13.14', }, + { + name: 'test_dep', + version: '1.23.45', + }, + { + name: 'some_other_dep', + version: '12.1.3', + }, ]; assert.deepEqual(dependencyVersions, expectedVersions); }); - - void it('returns empty array if there are no matching dependencies', async () => { - const lockFileReaderMock = { - getLockFileContentsFromCwd: async () => - Promise.resolve({ - dependencies: [ - { - name: 'test_dep', - version: '1.23.45', - }, - { - name: 'some_other_dep', - version: '12.1.3', - }, - ], - }), - } as LockFileReader; - const yarnModernPackageManagerController = - new YarnModernPackageManagerController( - '/testProjectRoot', - printerMock as unknown as Printer, - fspMock as unknown as typeof fsp, - pathMock as unknown as typeof path, - execaMock as unknown as typeof execa, - executeWithDebugLoggerMock as unknown as typeof executeWithDebugLogger, - existsSyncMock, - lockFileReaderMock - ); - const dependencyVersions = - await yarnModernPackageManagerController.getDependencies(); - - assert.deepEqual(dependencyVersions, []); - }); }); }); diff --git a/packages/cli/src/ampx.ts b/packages/cli/src/ampx.ts index 8d97e526a1..c9448a58d1 100755 --- a/packages/cli/src/ampx.ts +++ b/packages/cli/src/ampx.ts @@ -29,7 +29,7 @@ if (libraryVersion == undefined) { const dependencies = await new PackageManagerControllerFactory() .getPackageManagerController() - .getDependencies(); + .tryGetDependencies(); const usageDataEmitter = await new UsageDataEmitterFactory().getInstance( libraryVersion, diff --git a/packages/cli/src/commands/sandbox/sandbox_command_factory.ts b/packages/cli/src/commands/sandbox/sandbox_command_factory.ts index c63403f947..eace366cd2 100644 --- a/packages/cli/src/commands/sandbox/sandbox_command_factory.ts +++ b/packages/cli/src/commands/sandbox/sandbox_command_factory.ts @@ -64,7 +64,7 @@ export const createSandboxCommand = (): CommandModule< async () => { const dependencies = await new PackageManagerControllerFactory() .getPackageManagerController() - .getDependencies(); + .tryGetDependencies(); return await new UsageDataEmitterFactory().getInstance( libraryVersion, dependencies diff --git a/packages/create-amplify/src/amplify_project_creator.test.ts b/packages/create-amplify/src/amplify_project_creator.test.ts index a88c08efcb..cf4675f9b1 100644 --- a/packages/create-amplify/src/amplify_project_creator.test.ts +++ b/packages/create-amplify/src/amplify_project_creator.test.ts @@ -109,7 +109,7 @@ void describe('AmplifyProjectCreator', () => { runWithPackageManager: mock.fn(() => Promise.resolve() as never), getCommand: (args: string[]) => `'npx ${args.join(' ')}'`, allowsSignalPropagation: () => true, - getDependencies: mock.fn(() => Promise.resolve([])), + tryGetDependencies: mock.fn(() => Promise.resolve([])), }; const projectRootValidatorMock = { validate: mock.fn() }; const gitIgnoreInitializerMock = { ensureInitialized: mock.fn() }; diff --git a/packages/create-amplify/src/initial_project_file_generator.test.ts b/packages/create-amplify/src/initial_project_file_generator.test.ts index 4d5314e680..39bed0b488 100644 --- a/packages/create-amplify/src/initial_project_file_generator.test.ts +++ b/packages/create-amplify/src/initial_project_file_generator.test.ts @@ -17,7 +17,7 @@ void describe('InitialProjectFileGenerator', () => { runWithPackageManager: mock.fn(() => Promise.resolve() as never), getCommand: (args: string[]) => `'npx ${args.join(' ')}'`, allowsSignalPropagation: () => true, - getDependencies: mock.fn(() => Promise.resolve([])), + tryGetDependencies: mock.fn(() => Promise.resolve([])), }; beforeEach(() => { executeWithDebugLoggerMock.mock.resetCalls(); diff --git a/packages/platform-core/API.md b/packages/platform-core/API.md index 092d8aee7f..4ef316c349 100644 --- a/packages/platform-core/API.md +++ b/packages/platform-core/API.md @@ -231,7 +231,7 @@ export type UsageDataEmitter = { // @public export class UsageDataEmitterFactory { - getInstance: (libraryVersion: string, dependencies: Array) => Promise; + getInstance: (libraryVersion: string, dependencies?: Array) => Promise; } // (No @packageDocumentation comment for this package) diff --git a/packages/platform-core/src/usage-data/usage_data_emitter.test.ts b/packages/platform-core/src/usage-data/usage_data_emitter.test.ts index 281fa69142..189c9b57d7 100644 --- a/packages/platform-core/src/usage-data/usage_data_emitter.test.ts +++ b/packages/platform-core/src/usage-data/usage_data_emitter.test.ts @@ -21,13 +21,21 @@ void describe('UsageDataEmitter', () => { const testLibraryVersion = '1.2.3'; const testDependencies = [ { - name: 'test-dep', + name: 'aws-cdk', version: '1.2.3', }, { - name: 'some_other_dep', + name: 'aws-cdk-lib', version: '12.13.14', }, + { + name: 'test-dep', + version: '1.2.4', + }, + { + name: 'some_other_dep', + version: '12.12.14', + }, ]; const testURL = url.parse('https://aws.amazon.com/amplify/'); const onReqEndMock = mock.fn(); @@ -101,6 +109,19 @@ void describe('UsageDataEmitter', () => { assert.ok(validate(usageDataSent.installationUuid)); assert.ok(usageDataSent.error == undefined); assert.ok(usageDataSent.downstreamException == undefined); + assert.deepStrictEqual( + usageDataSent.projectSetting.details, + JSON.stringify([ + { + name: 'aws-cdk', + version: '1.2.3', + }, + { + name: 'aws-cdk-lib', + version: '12.13.14', + }, + ]) + ); }); void test('happy case, emitFailure generates and send correct usage data', async () => { diff --git a/packages/platform-core/src/usage-data/usage_data_emitter.ts b/packages/platform-core/src/usage-data/usage_data_emitter.ts index 653fb4f513..82616760aa 100644 --- a/packages/platform-core/src/usage-data/usage_data_emitter.ts +++ b/packages/platform-core/src/usage-data/usage_data_emitter.ts @@ -16,16 +16,23 @@ import { Dependency } from '@aws-amplify/plugin-types'; * Entry point for sending usage data metrics */ export class DefaultUsageDataEmitter implements UsageDataEmitter { + private dependenciesToReport?: Array; /** * Constructor for UsageDataEmitter */ constructor( private readonly libraryVersion: string, - private readonly dependencies: Array, + private readonly dependencies?: Array, private readonly sessionUuid = uuid(), private readonly url = getUrl(), private readonly accountIdFetcher = new AccountIdFetcher() - ) {} + ) { + const targetDependencies = ['aws-cdk', 'aws-cdk-lib']; + + this.dependenciesToReport = this.dependencies?.filter((dependency) => + targetDependencies.includes(dependency.name) + ); + } emitSuccess = async ( metrics?: Record, @@ -90,7 +97,7 @@ export class DefaultUsageDataEmitter implements UsageDataEmitter { isCi: isCI, projectSetting: { editor: process.env.npm_config_user_agent, - details: JSON.stringify(this.dependencies), + details: JSON.stringify(this.dependenciesToReport), }, }; }; diff --git a/packages/platform-core/src/usage-data/usage_data_emitter_factory.ts b/packages/platform-core/src/usage-data/usage_data_emitter_factory.ts index f4466f1e83..c8134c0e09 100644 --- a/packages/platform-core/src/usage-data/usage_data_emitter_factory.ts +++ b/packages/platform-core/src/usage-data/usage_data_emitter_factory.ts @@ -25,7 +25,7 @@ export class UsageDataEmitterFactory { */ getInstance = async ( libraryVersion: string, - dependencies: Array + dependencies?: Array ): Promise => { const configController = configControllerFactory.getInstance( 'usage_data_preferences.json' diff --git a/packages/plugin-types/API.md b/packages/plugin-types/API.md index e6ecbae1b5..b289307f05 100644 --- a/packages/plugin-types/API.md +++ b/packages/plugin-types/API.md @@ -202,16 +202,6 @@ export type ImportPathVerifier = { verify: (importStack: string | undefined, expectedImportingFile: string, errorMessage: string) => void; }; -// @public (undocumented) -export type LockFileContents = { - dependencies: Array; -}; - -// @public (undocumented) -export type LockFileReader = { - getLockFileContentsFromCwd: () => Promise; -}; - // @public (undocumented) export type LogLevel = 'all' | 'debug' | 'error' | 'fatal' | 'info' | 'none' | 'trace' | 'warn'; @@ -236,7 +226,7 @@ export type PackageManagerController = { runWithPackageManager: (args: string[] | undefined, dir: string, options?: ExecaOptions) => ExecaChildProcess; getCommand: (args: string[]) => string; allowsSignalPropagation: () => boolean; - getDependencies: () => Promise>; + tryGetDependencies: () => Promise | undefined>; }; // @public (undocumented) diff --git a/packages/plugin-types/src/package_manager_controller.ts b/packages/plugin-types/src/package_manager_controller.ts index 9eed9e1e80..d583960c43 100644 --- a/packages/plugin-types/src/package_manager_controller.ts +++ b/packages/plugin-types/src/package_manager_controller.ts @@ -25,14 +25,6 @@ export type ExecaChildProcess = { export type Dependency = { name: string; version: string }; -export type LockFileContents = { - dependencies: Array; -}; - -export type LockFileReader = { - getLockFileContentsFromCwd: () => Promise; -}; - export type PackageManagerController = { initializeProject: () => Promise; initializeTsConfig: (targetDir: string) => Promise; @@ -47,5 +39,5 @@ export type PackageManagerController = { ) => ExecaChildProcess; getCommand: (args: string[]) => string; allowsSignalPropagation: () => boolean; - getDependencies: () => Promise>; + tryGetDependencies: () => Promise | undefined>; };