Skip to content

Commit

Permalink
Don't invoke npm prune, npm install is already enough
Browse files Browse the repository at this point in the history
  • Loading branch information
@mgol
mgol committed Nov 14, 2023
1 parent 65107d2 commit 1f514eb
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 52 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
70 changes: 22 additions & 48 deletions lib/check-dependencies.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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`,
);
Expand All @@ -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`,
);
Expand All @@ -285,52 +284,43 @@ 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.
let msg, spawnReturn;
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,
});
} 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',
});
Expand All @@ -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;
Expand All @@ -353,33 +343,21 @@ 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 {
if (installNeeded) {
installMissing();
}

if (pruneNeeded) {
pruneExcessive();
}

success = true;
} catch (error) {
success = false;
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions test/spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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('');

Expand Down

0 comments on commit 1f514eb

Please sign in to comment.