From 1f514ebc317d0ef0fee761c9c1dd90001f558b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82e=CC=A8biowski-Owczarek?= Date: Wed, 15 Nov 2023 00:17:12 +0100 Subject: [PATCH] Don't invoke `npm prune`, `npm install` is already enough --- README.md | 2 +- lib/check-dependencies.js | 70 ++++++++++++--------------------------- test/spec.js | 6 ++-- 3 files changed, 26 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index c9378ce..38e44bd 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ Default: `false` #### install -Installs packages if they don't match. With the `onlySpecified` option enabled prune excessive packages as well. +Installs packages if they don't match. With the `onlySpecified` option enabled it installs if excessive packages are present as well. Type: `boolean` diff --git a/lib/check-dependencies.js b/lib/check-dependencies.js index 39822d1..e1f840b 100644 --- a/lib/check-dependencies.js +++ b/lib/check-dependencies.js @@ -49,10 +49,9 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { let packageJson, pkgManagerPath; - let installPrunePromise = Promise.resolve(); + let installPromise = Promise.resolve(); let success = true; let installNeeded = false; - let pruneNeeded = false; const log = message => { output.log.push(message); @@ -259,7 +258,7 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { // isn't one, just skip it. if (depSubDirName && !fullDepsMappings[depName]) { success = false; - pruneNeeded = true; + installNeeded = true; error( `Package ${depName} installed, though it shouldn't be`, ); @@ -270,7 +269,7 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { // Regular packages if (!fullDepsMappings[depName]) { success = false; - pruneNeeded = true; + installNeeded = true; error( `Package ${depName} installed, though it shouldn't be`, ); @@ -285,26 +284,18 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { output.depsWereOk = false; if (!options.install) { - if (options.onlySpecified) { - error( - `Invoke ${pico.green( - `${options.packageManager} prune`, - )} and ${pico.green( - `${options.packageManager} install`, - )} to install missing packages and remove excessive ones`, - ); - } else { - error( - `Invoke ${pico.green( - `${options.packageManager} install`, - )} to install missing packages`, - ); - } + error( + `Invoke ${pico.green( + `${options.packageManager} install`, + )} to install missing packages${ + options.onlySpecified ? ' and remove excessive ones' : '' + }`, + ); return finish(); } - const installOrPrune = mode => { - log(`Invoking ${pico.green(`${options.packageManager} ${mode}`)}...`); + const install = () => { + log(`Invoking ${pico.green(`${options.packageManager} install`)}...`); // If we're using a direct path, on Windows we need to invoke it via `node path`, not // `cmd /c path`. In UNIX systems we can execute the command directly so no need to wrap. @@ -312,7 +303,7 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { const method = syncOrAsync === 'sync' ? spawnSync : spawn; if (spawnShellSupported) { - spawnReturn = method(`${options.packageManager} ${mode}`, { + spawnReturn = method(`${options.packageManager} install`, { cwd: options.packageDir, stdio: 'inherit', shell: true, @@ -320,17 +311,16 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { } else if (win32) { spawnReturn = method( pkgManagerPath ? 'node' : 'cmd', - (pkgManagerPath - ? [pkgManagerPath] - : ['/c', options.packageManager] - ).concat(mode), + pkgManagerPath + ? [pkgManagerPath, 'install'] + : ['/c', options.packageManager, 'install'], { cwd: options.packageDir, stdio: 'inherit', }, ); } else { - spawnReturn = method(options.packageManager, [mode], { + spawnReturn = method(options.packageManager, ['install'], { cwd: options.packageDir, stdio: 'inherit', }); @@ -340,7 +330,7 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { if (spawnReturn.status !== 0) { msg = `${ options.packageManager - } ${mode} failed with code: ${pico.red(spawnReturn.status)}`; + } install failed with code: ${pico.red(spawnReturn.status)}`; throw new Error(msg); } return null; @@ -353,22 +343,14 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { } msg = `${ options.packageManager - } ${mode} failed with code: ${pico.red(code)}`; + } install failed with code: ${pico.red(code)}`; error(msg); reject(msg); }); }); }; - const installMissing = () => installOrPrune('install'); - const pruneExcessive = () => installOrPrune('prune'); - - // In some circumstances, e.g. if npm discovers a package is broken, npm may decide to remove - // a package when pruning even if versions in package.json match. It's safer to always install - // before prunning then so that the end state is always correct. - if (pruneNeeded) { - installNeeded = true; - } + const installMissing = () => install(); if (syncOrAsync === 'sync') { try { @@ -376,10 +358,6 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { installMissing(); } - if (pruneNeeded) { - pruneExcessive(); - } - success = true; } catch (error) { success = false; @@ -389,14 +367,10 @@ const checkDependenciesHelper = (syncOrAsync, config, callback) => { // Async scenario if (installNeeded) { - installPrunePromise = installPrunePromise.then(installMissing); - } - - if (pruneNeeded) { - installPrunePromise = installPrunePromise.then(pruneExcessive); + installPromise = installPromise.then(installMissing); } - return installPrunePromise + return installPromise .then(() => { success = true; return finish(); diff --git a/test/spec.js b/test/spec.js index f4fd718..af2fbfe 100644 --- a/test/spec.js +++ b/test/spec.js @@ -53,7 +53,7 @@ describe('checkDependencies', () => { const checkDeps = getCheckDependencies(); const installMessage = `Invoke ${packageManager} install to install missing packages`; - const pruneAndInstallMessage = `Invoke ${packageManager} prune and ${packageManager} install to install missing packages and remove excessive ones`; + const installMessageWithOnlySpecified = `Invoke ${packageManager} install to install missing packages and remove excessive ones`; const errorsForNotOk = [ 'a: installed: 1.2.4, expected: 1.2.3', @@ -247,7 +247,7 @@ describe('checkDependencies', () => { output => { assert.deepEqual(output.error, [ "Package c installed, though it shouldn't be", - pruneAndInstallMessage, + installMessageWithOnlySpecified, ]); assert.strictEqual(output.status, 1); done(); @@ -1017,7 +1017,7 @@ describe('checkDependencies', () => { describe('the --scope-list and --optional-scope-list options', () => { const errorMessage = [ "Package c installed, though it shouldn't be\n", - 'Invoke npm prune and npm install to install missing packages ', + 'Invoke npm install to install missing packages ', 'and remove excessive ones\n', ].join('');