Fix broken package names being used in commands

leeyi45 · leeyi45 · commit 18fc8ef5a9a8 · 2025-11-07T13:59:21.000-05:00
diff --git a/.github/actions/src/info/__tests__/index.test.ts b/.github/actions/src/info/__tests__/index.test.ts
@@ -4,7 +4,7 @@ import pathlib from 'path';
 import * as core from '@actions/core';
 import { describe, expect, test, vi } from 'vitest';
 import * as git from '../../commons.js';
-import * as lockfiles from '../../lockfiles.js';
+import * as lockfiles from '../../lockfiles/index.js';
 import { getAllPackages, getRawPackages, main } from '../index.js';
 
 const mockedCheckChanges = vi.spyOn(git, 'checkDirForChanges');
diff --git a/.github/actions/src/info/index.ts b/.github/actions/src/info/index.ts
@@ -6,7 +6,7 @@ import type { SummaryTableRow } from '@actions/core/lib/summary.js';
 import packageJson from '../../../../package.json' with { type: 'json' };
 import { checkDirForChanges, type PackageRecord, type RawPackageRecord } from '../commons.js';
 import { gitRoot } from '../gitRoot.js';
-import { getPackagesWithResolutionChanges, hasLockFileChanged } from '../lockfiles.js';
+import { getPackagesWithResolutionChanges, hasLockFileChanged } from '../lockfiles/index.js';
 import { topoSortPackages } from './sorter.js';
 
 const packageNameRE = /^@sourceacademy\/(.+?)-(.+)$/u;
diff --git a/.github/actions/src/lockfiles/__tests__/lockfiles.test.ts b/.github/actions/src/lockfiles/__tests__/lockfiles.test.ts
@@ -0,0 +1,18 @@
+import { describe, expect, it, vi } from 'vitest';
+import * as utils from '../utils.js';
+
+vi.mock(import('../../gitRoot.js'), () => ({
+  gitRoot: 'root'
+}));
+
+describe(utils.extractPackageName, () => {
+  it('works with packages that start with @', () => {
+    expect(utils.extractPackageName('@sourceacademy/tab-Rune@workspace:^'))
+      .toEqual('@sourceacademy/tab-Rune');
+  });
+
+  it('works with regular package names', () => {
+    expect(utils.extractPackageName('lodash@npm:^4.17.20'))
+      .toEqual('lodash');
+  });
+});
diff --git a/.github/actions/src/lockfiles/index.ts b/.github/actions/src/lockfiles/index.ts
@@ -0,0 +1,31 @@
+import { getExecOutput } from '@actions/exec';
+import memoize from 'lodash/memoize.js';
+import uniq from 'lodash/uniq.js';
+import { getPackageDiffs, getPackageReason } from './utils.js';
+
+/**
+ * Returns `true` if there are changes present in the given directory relative to
+ * the master branch\
+ * Used to determine, particularly for libraries, if running tests and tsc are necessary
+ */
+export const hasLockFileChanged = memoize(async () => {
+  const { exitCode } = await getExecOutput(
+    'git --no-pager diff --quiet origin/master -- yarn.lock',
+    [],
+    {
+      failOnStdErr: false,
+      ignoreReturnCode: true
+    }
+  );
+  return exitCode !== 0;
+});
+
+/**
+ * Gets a list of packages that have had some dependency of theirs
+ * change according to the lockfile but not their package.json
+ */
+export async function getPackagesWithResolutionChanges() {
+  const packages = [...await getPackageDiffs()];
+  const workspaces = await Promise.all(packages.map(getPackageReason));
+  return uniq(workspaces.flat());
+}
diff --git a/.github/actions/src/lockfiles/utils.ts b/.github/actions/src/lockfiles/utils.ts
@@ -2,75 +2,61 @@ import fs from 'fs/promises';
 import pathlib from 'path';
 import * as core from '@actions/core';
 import { getExecOutput } from '@actions/exec';
-import memoize from 'lodash/memoize.js';
-import uniq from 'lodash/uniq.js';
 import { extractPkgsFromYarnLockV2 } from 'snyk-nodejs-lockfile-parser';
-import { gitRoot } from './gitRoot.js';
+import { gitRoot } from '../gitRoot.js';
+
+const packageNameRE = /^(.+)@.+$/;
 
 /**
- * Parses and lockfile's contents and extracts all the different dependencies and
- * versions
+ * Lockfile specifications come in the form of package_name@resolution, but
+ * we only want the package name. This function extracts that package name,
+ * accounting for the fact that package names might start with '@'
  */
-function processLockFileText(contents: string) {
-  const lockFile = extractPkgsFromYarnLockV2(contents);
-  return Object.entries(lockFile).reduce<Record<string, Set<string>>>((res, [key, { version }]) => {
-    const newKey = key.split('@')[0];
-
-    if (!(newKey in res)) {
-      res[newKey] = new Set();
-    }
+export function extractPackageName(raw: string) {
+  const match = packageNameRE.exec(raw);
+  if (!match) {
+    throw new Error(`Invalid package name: ${raw}`);
+  }
 
-    res[newKey].add(version);
-    return res;
-  }, {});
+  return match[1];
 }
 
 /**
- * Retrieves the contents of the lockfile on the master branch
+ * Using `yarn why`, determine why the given package is present in the
+ * lockfile.
  */
-async function getMasterLockFile() {
-  const { stdout, exitCode } = await getExecOutput(
-    'git',
-    [
-      '--no-pager',
-      'show',
-      'origin/master:yarn.lock'
-    ],
-  );
-
+export async function getPackageReason(pkg: string) {
+  const { stdout, stderr, exitCode } = await getExecOutput('yarn why', [pkg, '-R', '--json'], { silent: true });
   if (exitCode !== 0) {
-    throw new Error('git show exited with non-zero error-code');
+    core.error(stderr);
+    throw new Error(`yarn why for '${pkg}' exited with non-zero exit code!`);
   }
 
-  return processLockFileText(stdout);
-}
-
-/**
- * Retrieves the contents of the lockfile in the repo
- */
-async function getCurrentLockFile() {
-  const lockFilePath = pathlib.join(gitRoot, 'yarn.lock');
-  const contents = await fs.readFile(lockFilePath, 'utf-8');
-  return processLockFileText(contents);
+  return stdout.trim().split('\n').map(each => {
+    const entry = JSON.parse(each).value;
+    return extractPackageName(entry);
+  });
 }
 
 /**
  * Determines the names of the packages that have changed versions
  */
-async function getPackageDiffs() {
+export async function getPackageDiffs() {
   const [currentLockFile, masterLockFile] = await Promise.all([
     getCurrentLockFile(),
     getMasterLockFile()
   ]);
 
-  const packages = [];
+  const packages = new Set<string>();
+
   for (const [depName, versions] of Object.entries(currentLockFile)) {
+    if (packages.has(depName)) continue;
+
     if (!(depName in masterLockFile)) {
       core.info(`${depName} in current lockfile, not in master lock file`);
       continue;
     }
 
-    const args = depName.split('@');
     let needToAdd = false;
     for (const version of versions) {
       if (!masterLockFile[depName].has(version)) {
@@ -86,65 +72,56 @@ async function getPackageDiffs() {
       }
     }
 
-    if (needToAdd) {
-      if (depName.startsWith('@')) {
-        packages.push(args[1]);
-      } else {
-        packages.push(args[0]);
-      }
-    }
+    if (needToAdd) packages.add(depName);
   }
 
-  return uniq(packages);
+  return packages;
 }
 
 /**
- * Using `yarn why`, determine why the given package is present in the
- * lockfile.
+ * Retrieves the contents of the lockfile in the repo
  */
-async function getPackageReason(pkg: string) {
-  const packageNameRE = /^(@?sourceacademy\/.+)@.+$/;
+export async function getCurrentLockFile() {
+  const lockFilePath = pathlib.join(gitRoot, 'yarn.lock');
+  const contents = await fs.readFile(lockFilePath, 'utf-8');
+  return processLockFileText(contents);
+}
+
+/**
+ * Retrieves the contents of the lockfile on the master branch
+ */
+export async function getMasterLockFile() {
+  const { stdout, exitCode } = await getExecOutput(
+    'git',
+    [
+      '--no-pager',
+      'show',
+      'origin/master:yarn.lock'
+    ],
+    { silent: true }
+  );
 
-  const { stdout, stderr, exitCode } = await getExecOutput('yarn why', [pkg, '-R', '--json'], { silent: true });
   if (exitCode !== 0) {
-    core.error(stderr);
-    throw new Error(`yarn why for ${pkg} exited with non-zero exit code!`);
+    throw new Error('git show exited with non-zero error-code');
   }
 
-  return stdout.trim().split('\n').map(each => {
-    const entry = JSON.parse(each).value;
-    const match = packageNameRE.exec(entry);
-    if (!match) {
-      throw new Error('failed to identify a package!');
-    }
-
-    return match[1];
-  });
+  return processLockFileText(stdout);
 }
 
 /**
- * Returns `true` if there are changes present in the given directory relative to
- * the master branch\
- * Used to determine, particularly for libraries, if running tests and tsc are necessary
+ * Parses and lockfile's contents and extracts all the different dependencies and
+ * versions
  */
-export const hasLockFileChanged = memoize(async () => {
-  const { exitCode } = await getExecOutput(
-    'git',
-    ['--no-pager', 'diff', '--quiet', 'origin/master', '--', 'yarn.lock'],
-    {
-      failOnStdErr: false,
-      ignoreReturnCode: true
+export function processLockFileText(contents: string) {
+  const lockFile = extractPkgsFromYarnLockV2(contents);
+  return Object.entries(lockFile).reduce<Record<string, Set<string>>>((res, [key, { version }]) => {
+    const newKey = extractPackageName(key);
+
+    if (!(newKey in res)) {
+      res[newKey] = new Set();
     }
-  );
-  return exitCode !== 0;
-});
 
-/**
- * Gets a list of packages that have had some dependency of theirs
- * change according to the lockfile but not their package.json
- */
-export async function getPackagesWithResolutionChanges() {
-  const packages = await getPackageDiffs();
-  const workspaces = await Promise.all(packages.map(getPackageReason));
-  return uniq(workspaces.flat());
+    res[newKey].add(version);
+    return res;
+  }, {});
 }
diff --git a/.github/actions/tsconfig.json b/.github/actions/tsconfig.json
@@ -1,8 +1,13 @@
 {
-  "include": ["src/**/*.ts", "vitest.setup.ts"],
+  "include": [
+    "src/**/*.ts",
+    "vitest.setup.ts"
+  ],
   "compilerOptions": {
     "forceConsistentCasingInFileNames": true,
-    "lib": ["ES2020"],
+    "lib": [
+      "ES2020"
+    ],
     "module": "esnext",
     "moduleResolution": "bundler",
     "noEmit": true,
