PR feedback

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(

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: [] });

packages/cli-core/src/package-manager-controller/lock-file-reader/npm_lock_file_reader.ts

@@ -1,51 +1,53 @@
-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';
 
 /**
  * NpmLockFileReader is an abstraction around the logic used to read and parse lock file contents
  */
 export class NpmLockFileReader implements LockFileReader {
-  getLockFileContentsFromCwd = async (): Promise<LockFileContents> => {
+  getLockFileContentsFromCwd = async (): Promise<
+    LockFileContents | undefined
+  > => {
     const dependencies: Array<Dependency> = [];
     const packageLockJsonPath = path.resolve(
       process.cwd(),
       'package-lock.json'
     );
-
-    let jsonLockParsedValue: Record<string, unknown>;
+    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(),

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/[email protected])(@aws-sdk/[email protected])(@aws-sdk/[email protected](@aws-sdk/[email protected]))(@aws-sdk/[email protected])([email protected]([email protected]))([email protected])([email protected])
+        '@aws-amplify/backend-cli':
+          specifier: ^1.4.5
+          version: 1.4.6(@aws-sdk/[email protected](@aws-sdk/[email protected]))(@aws-sdk/[email protected])(@aws-sdk/[email protected])([email protected]([email protected]))([email protected])([email protected])([email protected]([email protected]))([email protected])([email protected])
+        aws-cdk:
+          specifier: ^2.173.4
+          version: 2.174.1
+        aws-cdk-lib:
+          specifier: ^2.173.4
+          version: 2.174.1([email protected])
+        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: [] });

packages/cli-core/src/package-manager-controller/lock-file-reader/pnpm_lock_file_reader.ts

@@ -1,18 +1,17 @@
-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';
 
 /**
  * PnpmLockFileReader is an abstraction around the logic used to read and parse lock file contents
  */
 export class PnpmLockFileReader implements LockFileReader {
-  getLockFileContentsFromCwd = async (): Promise<LockFileContents> => {
+  getLockFileContentsFromCwd = async (): Promise<
+    LockFileContents | undefined
+  > => {
     const eolRegex = '[\r\n]';
     const dependencies: Array<Dependency> = [];
     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 };

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<Dependency>;
+};
+
+export type LockFileReader = {
+  getLockFileContentsFromCwd: () => Promise<LockFileContents | undefined>;
+};

packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.test.ts

@@ -56,10 +56,25 @@ [email protected]:
     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: [] });

packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_classic_lock_file_reader.ts

@@ -1,27 +1,26 @@
-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';
 
 /**
  * YarnClassicLockFileReader is an abstraction around the logic used to read and parse lock file contents
  */
 export class YarnClassicLockFileReader implements LockFileReader {
-  getLockFileContentsFromCwd = async (): Promise<LockFileContents> => {
+  getLockFileContentsFromCwd = async (): Promise<
+    LockFileContents | undefined
+  > => {
     const eolRegex = '[\r\n]';
     const dependencies: Array<Dependency> = [];
     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 };

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: [] });

packages/cli-core/src/package-manager-controller/lock-file-reader/yarn_modern_lock_file_reader.ts

@@ -1,18 +1,17 @@
-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';
 
 /**
  * YarnModernLockFileReader is an abstraction around the logic used to read and parse lock file contents
  */
 export class YarnModernLockFileReader implements LockFileReader {
-  getLockFileContentsFromCwd = async (): Promise<LockFileContents> => {
+  getLockFileContentsFromCwd = async (): Promise<
+    LockFileContents | undefined
+  > => {
     const eolRegex = '[\r\n]';
     const dependencies: Array<Dependency> = [];
     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 };