Merge pull request #4 from stixx/optimizations

Optimize diff generation and add transitive dependency toggle

Author: stixx

README.md

@@ -134,10 +134,11 @@ jobs:
 
 ## Inputs
 
-| Input      | Description                                                    | Required | Default             |
-| ---------- | -------------------------------------------------------------- | -------- | ------------------- |
-| `base-ref` | Base branch reference for comparison (e.g., `main`, `develop`) | No       | `main`              |
-| `path`     | Path to package-lock.json file                                 | No       | `package-lock.json` |
+| Input                | Description                                                    | Required | Default             |
+| -------------------- | -------------------------------------------------------------- | -------- | ------------------- |
+| `base-ref`           | Base branch reference for comparison (e.g., `main`, `develop`) | No       | `main`              |
+| `path`               | Path to package-lock.json file                                 | No       | `package-lock.json` |
+| `include-transitive` | Include transitive dependencies in the diff                    | No       | `false`             |
 
 ## Outputs
 
@@ -216,6 +217,16 @@ To compare against a branch other than `main`:
     base-ref: develop
 ```
 
+### Include Transitive Dependencies
+
+By default, the action only shows changes for direct dependencies to keep the diff clean. To see all changes (including transitive dependencies):
+
+```yaml
+- uses: stixx/npm-diff@v1
+  with:
+    include-transitive: true
+```
+
 ## Contributing
 
 Contributions are welcome! Please feel free to submit a Pull Request.

action.yml

@@ -14,6 +14,10 @@ inputs:
     description: 'Path to package-lock.json file'
     required: false
     default: 'package-lock.json'
+  include-transitive:
+    description: 'Include transitive dependencies in the diff'
+    required: false
+    default: 'false'
 
 outputs:
   has_changes:

dist/index.js

@@ -1,6 +1,6 @@
 {
   "name": "npm-diff",
-  "version": "1.0.3",
+  "version": "1.1.0",
   "description": "Generate a formatted diff of npm package-lock.json changes for pull requests",
   "main": "dist/index.js",
   "scripts": {

package.json

@@ -28,15 +28,36 @@ export interface Changes {
 
 export function parseLockfile(
   path: string,
-  contentStr: string | null = null
+  contentStr: string | null = null,
+  includeTransitive: boolean = false
 ): Record<string, PackageInfo> {
   try {
     const content = contentStr || fs.readFileSync(path, 'utf8');
     const data: LockfileData = JSON.parse(content);
     let packages: Record<string, PackageInfo | string> = {};
 
     if (data.packages) {
-      packages = data.packages;
+      if (includeTransitive) {
+        packages = { ...data.packages };
+      } else {
+        // For lockfile v3, we only want to compare the direct dependencies
+        // to keep the diff clean and avoid GitHub PR comment limits.
+        // Direct dependencies are listed in the root package ('')
+        const rootPackage = data.packages[''] as LockfileData | undefined;
+        const directDeps = new Set([
+          ...Object.keys(rootPackage?.dependencies || {}),
+          ...Object.keys(rootPackage?.devDependencies || {}),
+          ...Object.keys(rootPackage?.optionalDependencies || {}),
+        ]);
+
+        for (const [key, info] of Object.entries(data.packages)) {
+          if (key === '') continue;
+          const name = key.replace(/^node_modules\//, '');
+          if (directDeps.has(name) && !key.includes('/node_modules/')) {
+            packages[key] = info;
+          }
+        }
+      }
     } else {
       packages = {
         ...(data.dependencies || {}),
@@ -77,7 +98,7 @@ export function comparePackages(
   for (const pkg of allPackages) {
     const inBase = basePackages.has(pkg);
     const inHead = headPackages.has(pkg);
-    const name = pkg.replace('node_modules/', '');
+    const name = pkg.replace(/^node_modules\//, '');
 
     // Skip packages that have no version (e.g., aliases or metadata without version)
     if (inHead && !head[pkg].version) continue;
@@ -100,7 +121,9 @@ export function comparePackages(
 }
 
 export function getCompareLink(packageName: string): string {
-  return `[Compare](https://www.npmjs.com/package/${packageName}?activeTab=versions)`;
+  // Use only the package name without node_modules path
+  const name = packageName.split('node_modules/').pop() || packageName;
+  return `[Compare](https://www.npmjs.com/package/${name}?activeTab=versions)`;
 }
 
 export function formatMarkdown(changes: Changes): string {
@@ -145,8 +168,15 @@ export function formatMarkdown(changes: Changes): string {
 
 // Main execution
 export function run(): void {
-  const baseRef = core.getInput('base-ref') || 'main';
-  const lockfilePath = core.getInput('path') || 'package-lock.json';
+  const getCustomInput = (name: string) => {
+    return (
+      core.getInput(name) || process.env[`INPUT_${name.toUpperCase().replace(/-/g, '_')}`] || ''
+    );
+  };
+
+  const baseRef = getCustomInput('base-ref') || 'main';
+  const lockfilePath = getCustomInput('path') || 'package-lock.json';
+  const includeTransitive = getCustomInput('include-transitive') === 'true';
 
   // Check if lockfile exists
   if (!fs.existsSync(lockfilePath)) {
@@ -161,7 +191,11 @@ export function run(): void {
   // Get the merge base to ensure we only see changes from the current branch
   let baseRevision = `origin/${baseRef}`;
   try {
-    const mergeBase = execSync(`git merge-base "origin/${baseRef}" HEAD`).toString().trim();
+    const mergeBase = execSync(`git merge-base "origin/${baseRef}" HEAD`, {
+      maxBuffer: 50 * 1024 * 1024,
+    })
+      .toString()
+      .trim();
     if (mergeBase) {
       baseRevision = mergeBase;
     }
@@ -173,7 +207,9 @@ export function run(): void {
 
   // Check if lockfile changed
   try {
-    const diff = execSync(`git diff --name-only "${baseRevision}" HEAD`).toString();
+    const diff = execSync(`git diff --name-only "${baseRevision}" HEAD`, {
+      maxBuffer: 50 * 1024 * 1024,
+    }).toString();
     const changedFiles = diff
       .split('\n')
       .map((line) => line.trim())
@@ -196,6 +232,17 @@ export function run(): void {
     if (err instanceof Error) {
       console.error('Error running git diff:', err.message);
     }
+    // If git diff fails (e.g. shallow clone), we don't want to proceed with an empty base
+    // as it would report all dependencies as added.
+    core.setOutput('has_changes', 'false');
+    core.setOutput(
+      'diff',
+      `_Error comparing branches. This may be due to a shallow clone. Try setting fetch-depth: 0 in actions/checkout._`
+    );
+    core.setOutput('added_count', '0');
+    core.setOutput('removed_count', '0');
+    core.setOutput('updated_count', '0');
+    return;
   }
 
   core.setOutput('has_changes', 'true');
@@ -205,13 +252,15 @@ export function run(): void {
   try {
     baseContent = execSync(`git show "${baseRevision}:${lockfilePath}"`, {
       stdio: ['pipe', 'pipe', 'ignore'],
+      maxBuffer: 50 * 1024 * 1024,
     }).toString();
   } catch {
     // If it fails, baseContent remains '{}'
+    // This could happen if the file didn't exist in the base branch
   }
 
-  const base = parseLockfile(lockfilePath, baseContent);
-  const head = parseLockfile(lockfilePath);
+  const base = parseLockfile(lockfilePath, baseContent, includeTransitive);
+  const head = parseLockfile(lockfilePath, null, includeTransitive);
   const changes = comparePackages(base, head);
   const markdown = formatMarkdown(changes);
 

src/action.ts

@@ -60,7 +60,7 @@ test('Functional test: detects devDependency change in package.json', () => {
     const env = {
       ...process.env,
       INPUT_PATH: 'package.json',
-      'INPUT_BASE-REF': 'main',
+      INPUT_BASE_REF: 'main',
       GITHUB_OUTPUT: outputPath,
       GITHUB_WORKSPACE: tempDir,
     };
@@ -148,7 +148,7 @@ test('Functional test: detects package upgrade in package-lock.json', () => {
     const env = {
       ...process.env,
       // INPUT_PATH defaults to package-lock.json
-      'INPUT_BASE-REF': 'main',
+      INPUT_BASE_REF: 'main',
       GITHUB_OUTPUT: outputPath,
       GITHUB_WORKSPACE: tempDir,
     };

test/functional/action.test.js

@@ -0,0 +1,123 @@
+/* eslint-disable @typescript-eslint/no-require-imports, no-undef */
+const { test } = require('node:test');
+const assert = require('node:assert');
+const { execSync } = require('child_process');
+const fs = require('fs');
+const path = require('path');
+
+test('Functional test: respects include-transitive input', () => {
+  const root = process.cwd();
+  const tempDir = path.join(root, 'temp-functional-test-transitive');
+  if (fs.existsSync(tempDir)) {
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  }
+  fs.mkdirSync(tempDir);
+
+  try {
+    // Setup git repo
+    execSync('git init', { cwd: tempDir });
+    execSync('git config user.email "test@example.com"', { cwd: tempDir });
+    execSync('git config user.name "Test User"', { cwd: tempDir });
+
+    // Create initial package-lock.json (v3) with a transitive dependency
+    const lockfile = {
+      name: 'test-pkg',
+      version: '1.0.0',
+      lockfileVersion: 3,
+      requires: true,
+      packages: {
+        '': {
+          name: 'test-pkg',
+          version: '1.0.0',
+          dependencies: {
+            axios: '^1.0.0',
+          },
+        },
+        'node_modules/axios': {
+          version: '1.0.0',
+          dependencies: {
+            follow: '1.0.0',
+          },
+        },
+        'node_modules/follow': {
+          version: '1.0.0',
+        },
+      },
+    };
+    fs.writeFileSync(path.join(tempDir, 'package-lock.json'), JSON.stringify(lockfile, null, 2));
+    execSync('git add package-lock.json', { cwd: tempDir });
+    execSync('git commit -m "Initial commit"', { cwd: tempDir });
+    execSync('git branch -m main', { cwd: tempDir });
+
+    // Create a new branch and upgrade BOTH direct and transitive dependency
+    execSync('git checkout -b feature', { cwd: tempDir });
+    lockfile.packages['node_modules/axios'].version = '1.1.0';
+    lockfile.packages[''].dependencies.axios = '^1.1.0';
+    lockfile.packages['node_modules/follow'].version = '1.1.0';
+
+    fs.writeFileSync(path.join(tempDir, 'package-lock.json'), JSON.stringify(lockfile, null, 2));
+    execSync('git add package-lock.json', { cwd: tempDir });
+    execSync('git commit -m "Upgrade axios and follow"', { cwd: tempDir });
+
+    // Add remote origin to self to satisfy git merge-base origin/main
+    execSync('git remote add origin .', { cwd: tempDir });
+    execSync('git fetch origin', { cwd: tempDir });
+
+    // Prepare GitHub Action environment
+    const outputPath = path.join(tempDir, 'github_output');
+
+    // Test Case 1: include-transitive = false (default)
+    fs.writeFileSync(outputPath, '');
+    const envDefault = {
+      ...process.env,
+      INPUT_BASE_REF: 'main',
+      INPUT_INCLUDE_TRANSITIVE: 'false',
+      GITHUB_OUTPUT: outputPath,
+      GITHUB_WORKSPACE: tempDir,
+    };
+
+    const actionPath = path.join(root, 'src', 'action.ts');
+    execSync(`node -r ts-node/register ${actionPath}`, {
+      cwd: tempDir,
+      env: envDefault,
+      stdio: 'inherit',
+    });
+
+    let outputContent = fs.readFileSync(outputPath, 'utf8');
+    console.log('Action Outputs (transitive=false):\n', outputContent);
+
+    assert.match(outputContent, /updated_count[\s\S]+1/);
+    assert.match(outputContent, /axios \| Upgraded/);
+    assert.ok(
+      !outputContent.includes('follow | Upgraded'),
+      'Should NOT include transitive dependency follow'
+    );
+
+    // Test Case 2: include-transitive = true
+    fs.writeFileSync(outputPath, '');
+    const envTransitive = {
+      ...process.env,
+      INPUT_BASE_REF: 'main',
+      INPUT_INCLUDE_TRANSITIVE: 'true',
+      GITHUB_OUTPUT: outputPath,
+      GITHUB_WORKSPACE: tempDir,
+    };
+
+    execSync(`node -r ts-node/register ${actionPath}`, {
+      cwd: tempDir,
+      env: envTransitive,
+      stdio: 'inherit',
+    });
+
+    outputContent = fs.readFileSync(outputPath, 'utf8');
+    console.log('Action Outputs (transitive=true):\n', outputContent);
+
+    assert.match(outputContent, /updated_count[\s\S]+2/);
+    assert.match(outputContent, /axios \| Upgraded/);
+    assert.match(outputContent, /follow \| Upgraded/);
+  } finally {
+    if (fs.existsSync(tempDir)) {
+      fs.rmSync(tempDir, { recursive: true, force: true });
+    }
+  }
+});

test/functional/transitive.test.js

@@ -74,7 +74,13 @@ test('parseLockfile handles package.json and devDependencies', () => {
 test('parseLockfile handles lockfile v3 packages', () => {
   const content = JSON.stringify({
     packages: {
-      '': { name: 'root', version: '1.0.0' },
+      '': {
+        name: 'root',
+        version: '1.0.0',
+        dependencies: {
+          pkg: '^1.1.0',
+        },
+      },
       'node_modules/pkg': { version: '1.1.0' },
     },
   });