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

packages/core/src/models/interfaces.ts

@@ -53,13 +53,19 @@ export interface ExecOpts {
 }
 
 export interface LifecycleConfig {
-  log?: typeof log;
+  access?: 'public' | 'restricted';
+  defaultTag?: string;
   ignorePrepublish?: boolean;
   ignoreScripts?: boolean;
+  log: log.Logger;
+  lernaCommand?: string;
   nodeOptions?: string;
+  projectScope?: string | null;
   scriptShell?: string;
   scriptsPrependNodePath?: boolean;
   snapshot?: any;
+  stdio?: string;
+  tag?: string;
   unsafePerm?: boolean;
 }
 
@@ -82,13 +88,13 @@ export interface UpdateChangelogOption {
 
 export interface FetchConfig {
   fetchRetries: number;
-  log: typeof log;
+  log: log.Logger;
   registry: string;
   username: string;
 }
 
 export interface PackConfig {
-  log: typeof log;
+  log: log.Logger;
 
   /* If "publish", run "prepublishOnly" lifecycle */
   lernaCommand?: string;

packages/core/src/utils/__tests__/run-lifecycle.spec.ts

@@ -1,11 +1,12 @@
 jest.mock('@npmcli/run-script', () => jest.fn(() => Promise.resolve({ stdout: '' })));
 
-const log = require('npmlog');
-const { loggingOutput } = require('@lerna-test/helpers/logging-output');
-const runScript = require('@npmcli/run-script');
-const { npmConf } = require('../npm-conf');
-const { Package } = require('../../package');
-const { runLifecycle, createRunner } = require('../run-lifecycle');
+import log from 'npmlog';
+import { loggingOutput } from '@lerna-test/helpers/logging-output';
+import runScript from '@npmcli/run-script';
+import { npmConf } from '../npm-conf';
+import { Package } from '../../package';
+import { runLifecycle, createRunner } from '../run-lifecycle';
+import { LifecycleConfig } from '../../models';
 
 describe('runLifecycle()', () => {
   const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
@@ -19,7 +20,7 @@ describe('runLifecycle()', () => {
       name: 'no-scripts',
     };
 
-    await runLifecycle(pkg, 'foo', new Map());
+    await runLifecycle(pkg as Package, 'foo', new Map() as any);
 
     expect(runScript).not.toHaveBeenCalled();
   });
@@ -32,7 +33,7 @@ describe('runLifecycle()', () => {
       },
     };
 
-    await runLifecycle(pkg, 'bar', new Map());
+    await runLifecycle(pkg as Package, 'bar', new Map() as any);
 
     expect(runScript).not.toHaveBeenCalled();
   });
@@ -48,13 +49,13 @@ describe('runLifecycle()', () => {
         engines: {
           node: '>= 8.9.0',
         },
-      },
+      } as unknown as Package,
       '/test/location'
     );
     const stage = 'preversion';
-    const opts = npmConf({ 'custom-cli-flag': true });
+    const opts = npmConf({ 'custom-cli-flag': true }) as unknown as LifecycleConfig;
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts);
 
     expect(runScript).toHaveBeenLastCalledWith(
       expect.objectContaining({
@@ -85,13 +86,13 @@ describe('runLifecycle()', () => {
         engines: {
           node: '>= 8.9.0',
         },
-      },
+      } as unknown as Package,
       '/test/location'
     );
     const stage = 'preversion';
     const opts = npmConf({ 'custom-cli-flag': true });
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts as unknown as LifecycleConfig);
 
     expect(consoleSpy).toHaveBeenCalledWith(`\n> test-name@1.0.0-test preversion /test/location\n> test\n`);
   });
@@ -111,7 +112,7 @@ describe('runLifecycle()', () => {
       'script-shell': 'fish',
     };
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts as unknown as LifecycleConfig);
 
     expect(runScript).toHaveBeenLastCalledWith(
       expect.objectContaining({
@@ -135,7 +136,7 @@ describe('runLifecycle()', () => {
       'ignore-prepublish': true,
     };
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts as unknown as LifecycleConfig);
 
     expect(runScript).not.toHaveBeenCalled();
   });
@@ -152,7 +153,7 @@ describe('runLifecycle()', () => {
       'ignore-scripts': true,
     };
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts as unknown as LifecycleConfig);
 
     expect(runScript).not.toHaveBeenCalled();
   });
@@ -169,7 +170,7 @@ describe('runLifecycle()', () => {
     const stage = 'prepack';
     const opts = {};
 
-    await runLifecycle(pkg, stage, opts);
+    await runLifecycle(pkg as Package, stage, opts as unknown as LifecycleConfig);
 
     const callOpts = runScript.mock.calls.pop().pop();
 
@@ -179,6 +180,7 @@ describe('runLifecycle()', () => {
 });
 
 describe('createRunner', () => {
+  const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
   const runPackageLifecycle = createRunner({ 'other-cli-flag': 0 });
 
   it('skips missing scripts block', async () => {
@@ -188,7 +190,7 @@ describe('createRunner', () => {
       location: 'test',
     };
 
-    await runPackageLifecycle(pkg, 'prepare');
+    await runPackageLifecycle(pkg as Package, 'prepare');
     expect(runScript).not.toHaveBeenCalled();
   });
 
@@ -200,10 +202,27 @@ describe('createRunner', () => {
       scripts: { test: 'echo foo' },
     };
 
-    await runPackageLifecycle(pkg, 'prepare');
+    await runPackageLifecycle(pkg as Package, 'prepare');
     expect(runScript).not.toHaveBeenCalled();
   });
 
+  it('logs stdout from runScript() response', async () => {
+    runScript.mockImplementationOnce(({ pkg, event }) => {
+      return Promise.resolve({ stdout: 'runScript output' });
+    });
+
+    const pkg = {
+      name: 'has-script-error',
+      version: '1.0.0',
+      location: 'test',
+      scripts: { prepublishOnly: 'exit 123' },
+    };
+
+    await runPackageLifecycle(pkg as Package, 'prepublishOnly');
+
+    expect(consoleSpy).toHaveBeenCalledWith('runScript output');
+  });
+
   it('logs script error and re-throws', async () => {
     runScript.mockImplementationOnce(({ pkg, event }) => {
       const err: any = new Error('boom');
@@ -221,7 +240,7 @@ describe('createRunner', () => {
       scripts: { prepublishOnly: 'exit 123' },
     };
 
-    await expect(runPackageLifecycle(pkg, 'prepublishOnly')).rejects.toThrow(
+    await expect(runPackageLifecycle(pkg as Package, 'prepublishOnly')).rejects.toThrow(
       expect.objectContaining({
         exitCode: 123,
         script: 'exit 123',
@@ -250,7 +269,7 @@ describe('createRunner', () => {
       scripts: { prepack: 'a-thing-that-ends-poorly' },
     };
 
-    await expect(runPackageLifecycle(pkg, 'prepack')).rejects.toThrow(
+    await expect(runPackageLifecycle(pkg as Package, 'prepack')).rejects.toThrow(
       expect.objectContaining({
         exitCode: 1,
         script: 'a-thing-that-ends-poorly',

packages/core/src/utils/run-lifecycle.ts

@@ -4,6 +4,9 @@ import runScript from '@npmcli/run-script';
 import { npmConf } from '../utils/npm-conf';
 import { LifecycleConfig } from '../models';
 import { Package } from '../package';
+import PQueue from 'p-queue';
+
+const queue = new PQueue({ concurrency: 1 });
 
 /**
  * Alias dash-cased npmConf to camelCase
@@ -46,13 +49,14 @@ export function runLifecycle(pkg: Package, stage: string, options: LifecycleConf
   }
 
   const opts = {
+    // @ts-ignore
     log,
     unsafePerm: true,
     ...flattenOptions(options),
   };
   const dir = pkg.location;
   const id = `${pkg.name}@${pkg.version}`;
-  const config: LifecycleConfig = {};
+  const config = {} as LifecycleConfig;
 
   if (opts.ignoreScripts) {
     opts.log.verbose('lifecycle', '%j ignored in %j', stage, pkg.name);
@@ -100,45 +104,53 @@ export function runLifecycle(pkg: Package, stage: string, options: LifecycleConf
 
   /**
    * 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;
+      }
+    )
   );
 }
 

packages/publish/src/lib/npm-publish.ts

@@ -6,7 +6,7 @@ import pify from 'pify';
 import { publish } from 'libnpmpublish';
 import readJSON from 'read-package-json';
 
-import { OneTimePasswordCache, otplease, Package, RawManifest, runLifecycle } from '@lerna-lite/core';
+import { LifecycleConfig, OneTimePasswordCache, otplease, Package, RawManifest, runLifecycle } from '@lerna-lite/core';
 import { LibNpmPublishOptions, PackagePublishConfig } from '../models';
 
 const readJSONAsync = pify(readJSON);
@@ -100,8 +100,8 @@ export function npmPublish(
     });
   }
 
-  chain = chain.then(() => runLifecycle(pkg, 'publish', opts));
-  chain = chain.then(() => runLifecycle(pkg, 'postpublish', opts));
+  chain = chain.then(() => runLifecycle(pkg, 'publish', opts as LifecycleConfig));
+  chain = chain.then(() => runLifecycle(pkg, 'postpublish', opts as LifecycleConfig));
 
   return chain;
 }

packages/publish/src/lib/pack-directory.ts

@@ -3,7 +3,7 @@ import packlist from 'npm-packlist';
 import log from 'npmlog';
 import tar from 'tar';
 
-import { Package, PackConfig, runLifecycle, tempWrite } from '@lerna-lite/core';
+import { LifecycleConfig, Package, PackConfig, runLifecycle, tempWrite } from '@lerna-lite/core';
 import { getPacked } from './get-packed';
 import { Readable } from 'stream';
 import { Tarball } from '../models';
@@ -16,7 +16,7 @@ import { Tarball } from '../models';
  */
 export function packDirectory(_pkg: Package, dir: string, options: PackConfig) {
   const pkg = Package.lazy(_pkg, dir);
-  const opts = {
+  const opts: LifecycleConfig = {
     // @ts-ignore
     log,
     ...options,
@@ -33,6 +33,7 @@ export function packDirectory(_pkg: Package, dir: string, options: PackConfig) {
   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());