fix: check for missing production dependencies

Runs the dep-check command twice - once omitting dev deps and only checking non-test code, then again but only testing test code. Should prevent future bugs similar to ipfs/helia-ipns#159
ipfs · Dec 29, 2023 · 5b25734 · 5b25734
1 parent 705c899
commit 5b25734
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 32 deletions.
diff --git a/src/cmds/dependency-check.js b/src/cmds/dependency-check.js
@@ -39,6 +39,18 @@ export default {
         default: true,
         type: 'boolean'
       })
+      .option('P', {
+        alias: 'productionIgnorePatterns',
+        describe: 'Patterns to ignore while checking production dependencies',
+        array: true,
+        default: userConfig.dependencyCheck.productionIgnorePatterns
+      })
+      .option('D', {
+        alias: 'developmentIgnorePatterns',
+        describe: 'Patterns to ignore while checking development dependencies',
+        array: true,
+        default: userConfig.dependencyCheck.developmentIgnorePatterns
+      })
   },
   /**
    * @param {any} argv

diff --git a/src/config/user.js b/src/config/user.js
@@ -114,7 +114,16 @@ const defaults = {
   // dependency check cmd options
   dependencyCheck: {
     unused: false,
-    ignore: []
+    ignore: [],
+    productionIgnorePatterns: [
+      'dist',
+      'test',
+      '.aegir.js'
+    ],
+    developmentIgnorePatterns: [
+      'dist',
+      'src'
+    ]
   },
   exec: {
     bail: true,

diff --git a/src/dependency-check.js b/src/dependency-check.js
@@ -1,6 +1,8 @@
 /* eslint-disable no-console */
 
-import { cwd } from 'process'
+import fs from 'node:fs'
+import path from 'node:path'
+import { cwd } from 'node:process'
 import depcheck from 'depcheck'
 import kleur from 'kleur'
 import Listr from 'listr'
@@ -31,48 +33,85 @@ const tasks = new Listr(
        * @param {Task} task
        */
       task: async (ctx, task) => {
-        const result = await depcheck(cwd(), {
+        let missingOrUnusedPresent = false
+
+        // check production dependencies
+        const manifest = JSON.parse(fs.readFileSync(path.join(cwd(), 'package.json'), 'utf-8'))
+
+        const productionOnlyResult = await depcheck(cwd(), {
           parsers: {
             '**/*.js': depcheck.parser.es6,
             '**/*.ts': depcheck.parser.typescript,
             '**/*.cjs': depcheck.parser.es6,
             '**/*.mjs': depcheck.parser.es6
           },
-          ignoreMatches: ignoredDevDependencies.concat(ctx.fileConfig.dependencyCheck.ignore).concat(ctx.ignore)
+          ignoreMatches: ignoredDevDependencies.concat(ctx.fileConfig.dependencyCheck.ignore).concat(ctx.ignore),
+          ignorePatterns: ctx.productionIgnorePatterns,
+          package: {
+            ...manifest,
+            devDependencies: {}
+          }
         })
 
-        if (Object.keys(result.missing).length > 0 || result.dependencies.length > 0 || result.devDependencies.length > 0) {
-          if (Object.keys(result.missing).length > 0) {
-            console.error('')
-            console.error('Missing dependencies:')
-            console.error('')
+        if (Object.keys(productionOnlyResult.missing).length > 0) {
+          missingOrUnusedPresent = true
+          console.error('')
+          console.error('Missing production dependencies:')
+          console.error('')
 
-            Object.entries(result.missing).forEach(([dep, path]) => {
-              console.error(kleur.red(dep))
-              console.error(' ', kleur.gray(path.join('\n  ')))
-            })
-          }
+          Object.entries(productionOnlyResult.missing).forEach(([dep, path]) => {
+            console.error(kleur.red(dep))
+            console.error(' ', kleur.gray(path.join('\n  ')))
+          })
+        }
 
-          if (result.dependencies.length > 0) {
-            console.error('')
-            console.error('Unused production dependencies:')
-            console.error('')
+        if (productionOnlyResult.dependencies.length > 0) {
+          missingOrUnusedPresent = true
+          console.error('')
+          console.error('Unused production dependencies:')
+          console.error('')
 
-            result.dependencies.forEach(dep => {
-              console.error(kleur.yellow(dep))
-            })
-          }
+          productionOnlyResult.dependencies.forEach(dep => {
+            console.error(kleur.yellow(dep))
+          })
+        }
+
+        // check dev dependencies
+        const devlopmentOnlyResult = await depcheck(cwd(), {
+          parsers: {
+            '**/*.js': depcheck.parser.es6,
+            '**/*.ts': depcheck.parser.typescript,
+            '**/*.cjs': depcheck.parser.es6,
+            '**/*.mjs': depcheck.parser.es6
+          },
+          ignoreMatches: ignoredDevDependencies.concat(ctx.fileConfig.dependencyCheck.ignore).concat(ctx.ignore),
+          ignorePatterns: ctx.developmentIgnorePatterns
+        })
 
-          if (result.devDependencies.length > 0) {
-            console.error('')
-            console.error('Unused dev dependencies:')
-            console.error('')
+        if (Object.keys(devlopmentOnlyResult.missing).length > 0) {
+          missingOrUnusedPresent = true
+          console.error('')
+          console.error('Missing development dependencies:')
+          console.error('')
 
-            result.devDependencies.forEach(dep => {
-              console.error(kleur.yellow(dep))
-            })
-          }
+          Object.entries(devlopmentOnlyResult.missing).forEach(([dep, path]) => {
+            console.error(kleur.red(dep))
+            console.error(' ', kleur.gray(path.join('\n  ')))
+          })
+        }
+
+        if (devlopmentOnlyResult.devDependencies.length > 0) {
+          missingOrUnusedPresent = true
+          console.error('')
+          console.error('Unused development dependencies:')
+          console.error('')
+
+          devlopmentOnlyResult.devDependencies.forEach(dep => {
+            console.error(kleur.yellow(dep))
+          })
+        }
 
+        if (missingOrUnusedPresent) {
           // necessary because otherwise listr removes the last line of output
           console.error(' ')
 

diff --git a/src/types.ts b/src/types.ts
@@ -350,14 +350,26 @@ interface ReleaseRcOptions {
 
 interface DependencyCheckOptions {
   /**
-   * throws error on unused dependencies, default is false
+   * throws error on unused dependencies
+   *
+   * @default true
    */
   unused: boolean
+
   /**
    * Ignore these dependencies when considering which are used and which are not
    */
   ignore: string[]
 
+  /**
+   * Files to ignore when checking production dependencies
+   */
+  productionIgnorePatterns: string[]
+
+  /**
+   * Files to ignore when checking dev dependencies
+   */
+  developmentIgnorePatterns: string[]
 }
 
 interface ExecOptions {

diff --git a/test/dependency-check.js b/test/dependency-check.js
@@ -76,7 +76,7 @@ describe('dependency check', () => {
   /**
    * depcheck removed this option as it caused too many false positives
    */
-  it.skip('should fail for missing production deps', async () => {
+  it('should fail for missing production deps', async () => {
     await expect(
       execa(bin, ['dependency-check', '-p'], {
         cwd: path.join(__dirname, 'fixtures/dependency-check/fail-prod')