Skip to content

Commit

Permalink
fix!: suppress warning when using npm v9
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: npm v9 changes `--global-style` to `--install-strategy=shallow`.  so now we need to retain the version that we found when running `which npm`, and change the shell command accordingly.  Note that even passing a custom path to `npm` will cause it to be executed so that we can verify the version.  This also modifies the `FIND_NPM_OK` event to return an `NpmInfo` object containing props `path` and `version`, instead of just the `string` path.
  • Loading branch information
@boneskull
boneskull committed Jul 24, 2023
1 parent 106bcd6 commit a325ad4
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 75 deletions.
86 changes: 48 additions & 38 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ class Smoker extends createStrictEventEmitterClass() {
*/
opts;

/** @type {string|undefined} */
#npmPath;
/** @type {NpmInfo|undefined} */
#npmInfo;

/** @type {boolean} */
#force = false;
Expand Down Expand Up @@ -159,7 +159,7 @@ class Smoker extends createStrictEventEmitterClass() {
this.#workspaces = normalizeArray(opts.workspace);
if (this.#allWorkspaces && this.#workspaces.length) {
throw new Error(
'Option "workspace" is mutually exclusive with "all" and/or "includeRoot"'
'Option "workspace" is mutually exclusive with "all" and/or "includeRoot"',
);
}
this.#extraNpmInstallArgs = normalizeArray(opts.installArgs);
Expand All @@ -184,25 +184,30 @@ class Smoker extends createStrictEventEmitterClass() {

/**
*
* @returns {Promise<string>}
* @returns {Promise<NpmInfo>}
*/
async findNpm() {
if (this.#npmPath) {
return this.#npmPath;
}
if (this.opts.npm) {
this.#npmPath = this.opts.npm.trim();
return this.#npmPath;
/**
* @param {string} npmPath
* @returns {Promise<NpmInfo>}
*/
const getVersion = async (npmPath) => {
const {stdout: version} = await execa(npmPath, ['--version']);
debug('(findNpm) Found npm %s at %s', version, npmPath);
return {path: npmPath, version};
};

if (this.#npmInfo) {
return this.#npmInfo;
}

this.emit(FIND_NPM_BEGIN);

try {
const npmPath = await which('npm');
// using #runNpm here would be recursive
const {stdout: version} = await execa(npmPath, ['--version']);
debug('(findNpm) Found npm %s at %s', version, npmPath);
this.#npmPath = npmPath;
this.emit(FIND_NPM_OK, npmPath);
return npmPath;
const npmPath = this.opts.npm ? this.opts.npm.trim() : await which('npm');
this.#npmInfo = await getVersion(npmPath);
this.emit(FIND_NPM_OK, this.#npmInfo);
return this.#npmInfo;
} catch (err) {
this.emit(FIND_NPM_FAILED, /** @type {Error} */ (err));
throw err;
Expand Down Expand Up @@ -240,7 +245,7 @@ class Smoker extends createStrictEventEmitterClass() {
return;
}
throw new Error(
`Working directory ${wd} already exists. Use "force" option to proceed anyhow.`
`Working directory ${wd} already exists. Use "force" option to proceed anyhow.`,
);
}

Expand Down Expand Up @@ -329,7 +334,7 @@ class Smoker extends createStrictEventEmitterClass() {
if (value.exitCode) {
debug('(pack) Failed: %O', value);
const error = new Error(
`"npm pack" failed with exit code ${value.exitCode}`
`"npm pack" failed with exit code ${value.exitCode}`,
);
this.emit(PACK_FAILED, error);
throw error;
Expand All @@ -343,7 +348,7 @@ class Smoker extends createStrictEventEmitterClass() {
} catch {
debug('(pack) Failed to parse JSON: %s', packOutput);
const error = new SyntaxError(
`Failed to parse JSON output from "npm pack": ${packOutput}`
`Failed to parse JSON output from "npm pack": ${packOutput}`,
);
this.emit(PACK_FAILED, error);
throw error;
Expand All @@ -369,8 +374,8 @@ class Smoker extends createStrictEventEmitterClass() {
* @param {execa.Options} [options]
*/
async #runNpm(args, options = {}) {
const npmPath = await this.findNpm();
const command = `${npmPath} ${args.join(' ')}`;
const npmInfo = await this.findNpm();
const command = `${npmInfo.path} ${args.join(' ')}`;
this.emit(RUN_NPM_BEGIN, {
command,
options,
Expand All @@ -381,7 +386,7 @@ class Smoker extends createStrictEventEmitterClass() {
let proc;

try {
proc = execa(npmPath, args, opts);
proc = execa(npmInfo.path, args, opts);
} catch (err) {
this.emit(RUN_NPM_FAILED, /** @type {execa.ExecaError} */ (err));
throw err;
Expand All @@ -396,18 +401,16 @@ class Smoker extends createStrictEventEmitterClass() {
* @type {execa.ExecaReturnValue|undefined}
*/
let value;
/** @type {execa.ExecaError & NodeJS.ErrnoException|undefined} */
let error;
try {
value = await proc;
this.emit(RUN_NPM_OK, {command, options, value});
return value;
} catch (e) {
this.emit(
RUN_NPM_FAILED,
/** @type {execa.ExecaError & NodeJS.ErrnoException} */ (e)
/** @type {execa.ExecaError & NodeJS.ErrnoException} */ (e),
);
throw error;
throw e;
}
}

Expand All @@ -426,9 +429,15 @@ class Smoker extends createStrictEventEmitterClass() {
this.emit(INSTALL_BEGIN, packItems);
const extraArgs = this.#extraNpmInstallArgs;
const cwd = await this.createWorkingDirectory();
const npmInfo = await this.findNpm();
// otherwise we get a deprecation warning
const globalStyleFlag =
npmInfo.version.startsWith('7') || npmInfo.version.startsWith('8')
? '--global-style'
: '--install-strategy=shallow';
const installArgs = [
'install',
'--global-style',
globalStyleFlag,
...extraArgs,
...packItems.map(({tarballFilepath}) => tarballFilepath),
];
Expand All @@ -447,7 +456,7 @@ class Smoker extends createStrictEventEmitterClass() {
if (value.exitCode) {
debug('(install) Failed: %O', value);
const error = new Error(
`"npm install" failed with exit code ${value.exitCode}: ${value.stdout}`
`"npm install" failed with exit code ${value.exitCode}: ${value.stdout}`,
);
this.emit(INSTALL_FAILED, error);
throw error;
Expand Down Expand Up @@ -491,7 +500,7 @@ class Smoker extends createStrictEventEmitterClass() {
script,
value,
current,
total
total,
) => {
const result = {
pkgName,
Expand All @@ -503,19 +512,19 @@ class Smoker extends createStrictEventEmitterClass() {
if (/missing script:/i.test(value.stderr)) {
this.emit(RUN_SCRIPT_FAILED, {error: value, current, total, pkgName});
return new Error(
`Script "${script}" in package "${pkgName}" failed; npm was unable to find this script`
`Script "${script}" in package "${pkgName}" failed; npm was unable to find this script`,
);
}

return new Error(
`Script "${script}" in package "${pkgName}" failed with exit code ${value.exitCode}: ${value.all}`
`Script "${script}" in package "${pkgName}" failed with exit code ${value.exitCode}: ${value.all}`,
);
} else if (value.failed) {
this.emit(RUN_SCRIPT_FAILED, {error: value, current, total, pkgName});
debug(
`(runScripts) Script "%s" in package "%s" failed; continuing...`,
script,
pkgName
pkgName,
);
} else {
this.emit(RUN_SCRIPT_OK, {
Expand All @@ -526,7 +535,7 @@ class Smoker extends createStrictEventEmitterClass() {
debug(
'(runScripts) Successfully executed script %s in package %s',
script,
pkgName
pkgName,
);
}
};
Expand All @@ -535,7 +544,7 @@ class Smoker extends createStrictEventEmitterClass() {
const npmArgs = ['run-script', script];
try {
for await (const [pkgIdx, {installPath: cwd}] of Object.entries(
packItems
packItems,
)) {
const pkgName = pathToPackageName(cwd);
const current = Number(pkgIdx) + Number(currentScriptIdx);
Expand All @@ -560,7 +569,7 @@ class Smoker extends createStrictEventEmitterClass() {
script,
/** @type {execa.ExecaError} */ (err),
current,
total
total,
);
}

Expand All @@ -569,7 +578,7 @@ class Smoker extends createStrictEventEmitterClass() {
script,
value,
current,
total
total,
);
if (err) {
throw err;
Expand All @@ -578,7 +587,7 @@ class Smoker extends createStrictEventEmitterClass() {
} finally {
const failures = results.reduce(
(acc, {failed = false}) => acc + Number(failed),
0
0,
);
if (failures) {
this.emit(RUN_SCRIPTS_FAILED, {
Expand Down Expand Up @@ -634,4 +643,5 @@ exports.smoke = async function smoke(scripts, opts = {}) {
* @typedef {import('./static').RunScriptResult} RunScriptResult
* @typedef {import('./static').Events} Events
* @typedef {import('./static').TSmokerEmitter} TSmokerEmitter
* @typedef {import('./static').NpmInfo} NpmInfo
*/
9 changes: 7 additions & 2 deletions src/static.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export interface Events {
SmokeOk: void;
SmokeFailed: (err: Error) => void;
FindNpmBegin: void;
FindNpmOk: string;
FindNpmOk: NpmInfo;
FindNpmFailed: (err: Error) => void;

PackBegin: void;
Expand All @@ -131,7 +131,7 @@ export interface Events {
total: number;
executed: number;
failures: number;
results: Array<ExecaReturnValue<string>|ExecaError<string>>;
results: Array<ExecaReturnValue<string> | ExecaError<string>>;
scripts: string[];
};
RunScriptsOk: {
Expand Down Expand Up @@ -163,3 +163,8 @@ export interface Events {
}

export type TSmokerEmitter = StrictEventEmitter<EventEmitter, Events>;

export interface NpmInfo {
path: string;
version: string;
}
Loading

0 comments on commit a325ad4

Please sign in to comment.