Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(run-lifecycle): lifecycle events should run to completion in series #3262

Merged
merged 1 commit into from Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion commands/bootstrap/package.json
Expand Up @@ -45,7 +45,7 @@
"@lerna/symlink-binary": "file:../../utils/symlink-binary",
"@lerna/symlink-dependencies": "file:../../utils/symlink-dependencies",
"@lerna/validation-error": "file:../../core/validation-error",
"@npmcli/arborist": "5.2.0",
"@npmcli/arborist": "5.3.0",
"dedent": "^0.7.0",
"get-port": "^5.1.1",
"multimatch": "^5.0.0",
Expand Down
83 changes: 30 additions & 53 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -42,8 +42,8 @@
"license": "MIT",
"homepage": "https://lerna.js.org",
"dependencies": {
"@npmcli/arborist": "5.2.0",
"@npmcli/run-script": "^3.0.2",
"@npmcli/arborist": "5.3.0",
"@npmcli/run-script": "^4.1.7",
"@octokit/plugin-enterprise-rest": "^6.0.1",
"@octokit/rest": "^19.0.3",
"byte-size": "^7.0.0",
Expand Down
1 change: 1 addition & 0 deletions utils/pack-directory/lib/pack-directory.js
Expand Up @@ -42,6 +42,7 @@ function packDirectory(_pkg, dir, options) {
chain = chain.then(() => runLifecycle(pkg, "prepare", opts));

if (opts.lernaCommand === "publish") {
opts.stdio = "inherit";
chain = chain.then(() => pkg.refresh());
chain = chain.then(() => runLifecycle(pkg, "prepublishOnly", opts));
chain = chain.then(() => pkg.refresh());
Expand Down
5 changes: 3 additions & 2 deletions utils/run-lifecycle/package.json
Expand Up @@ -32,7 +32,8 @@
},
"dependencies": {
"@lerna/npm-conf": "file:../npm-conf",
"@npmcli/run-script": "^3.0.2",
"npmlog": "^6.0.2"
"@npmcli/run-script": "^4.1.7",
"npmlog": "^6.0.2",
"p-queue": "^6.6.2"
}
}
84 changes: 46 additions & 38 deletions utils/run-lifecycle/run-lifecycle.js
Expand Up @@ -5,6 +5,9 @@
const log = require("npmlog");
const runScript = require("@npmcli/run-script");
const npmConf = require("@lerna/npm-conf");
const PQueue = require("p-queue").default;

const queue = new PQueue({ concurrency: 1 });

module.exports.runLifecycle = runLifecycle;
module.exports.createRunner = createRunner;
Expand All @@ -18,6 +21,7 @@ module.exports.createRunner = createRunner;
* @property {string} [scriptShell]
* @property {boolean} [scriptsPrependNodePath]
* @property {boolean} [unsafePerm=true]
* @property {string} [stdio]
*/

/**
Expand Down Expand Up @@ -115,49 +119,53 @@ function runLifecycle(pkg, stage, options) {

/**
* In order to match the previous behavior of "npm-lifecycle", we have to disable the writing
* to the parent process and print the command banner ourself.
* to the parent process and print the command banner ourselves, unless overridden by the options.
*/
const stdio = "pipe";
const stdio = opts.stdio || "pipe";
if (log.level !== "silent") {
printCommandBanner(id, stage, pkg.scripts[stage], dir);
}

return runScript({
event: stage,
path: dir,
pkg,
args: [],
stdio,
banner: false,
scriptShell: config.scriptShell,
}).then(
({ stdout }) => {
/**
* This adjustment is based on trying to match the existing integration test outputs when migrating
* from "npm-lifecycle" to "@npmcli/run-script".
*/
// eslint-disable-next-line no-console
console.log(stdout.toString().trimEnd());

opts.log.silly("lifecycle", "%j finished in %j", stage, pkg.name);
},
(err) => {
// propagate the exit code
const exitCode = err.code || 1;

// error logging has already occurred on stderr, but we need to stop the chain
log.error("lifecycle", "%j errored in %j, exiting %d", stage, pkg.name, exitCode);

// ensure clean logging, avoiding spurious log dump
err.name = "ValidationError";

// our yargs.fail() handler expects a numeric .exitCode, not .errno
err.exitCode = exitCode;
process.exitCode = exitCode;

// stop the chain
throw err;
}
return queue.add(async () =>
runScript({
event: stage,
path: dir,
pkg,
args: [],
stdio,
banner: false,
scriptShell: config.scriptShell,
}).then(
({ stdout }) => {
if (stdout) {
/**
* This adjustment is based on trying to match the existing integration test outputs when migrating
* from "npm-lifecycle" to "@npmcli/run-script".
*/
// eslint-disable-next-line no-console
console.log(stdout.toString().trimEnd());
}

opts.log.silly("lifecycle", "%j finished in %j", stage, pkg.name);
},
(err) => {
// propagate the exit code
const exitCode = err.code || 1;

// error logging has already occurred on stderr, but we need to stop the chain
log.error("lifecycle", "%j errored in %j, exiting %d", stage, pkg.name, exitCode);

// ensure clean logging, avoiding spurious log dump
err.name = "ValidationError";

// our yargs.fail() handler expects a numeric .exitCode, not .errno
err.exitCode = exitCode;
process.exitCode = exitCode;

// stop the chain
throw err;
}
)
);
}

Expand Down