From 5b257348887ffb0cea68905fdd444ad3803054c4 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 29 Dec 2023 21:31:35 +0100 Subject: [PATCH] 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 https://github.com/ipfs/helia-ipns/pull/159 --- src/cmds/dependency-check.js | 12 +++++ src/config/user.js | 11 +++- src/dependency-check.js | 97 +++++++++++++++++++++++++----------- src/types.ts | 14 +++++- test/dependency-check.js | 2 +- 5 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/cmds/dependency-check.js b/src/cmds/dependency-check.js index 3feff2db5..ef3c9d3d4 100644 --- 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 index 5be649eb9..5595cfa23 100644 --- 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 index 2a8ee4c26..06953d4d5 100644 --- 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 index 02007fa6e..7a1053b06 100644 --- 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 index 0f8c2a7d5..25df57485 100644 --- 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')