Skip to content

Commit

Permalink
fix(run-lifecycle): lifecycle events should run to completion in seri…
Browse files Browse the repository at this point in the history 
…es (#275)

* fix(run-lifecycle): lifecycle events should run to completion in series
  • Loading branch information
@ghiscoding
ghiscoding authored Jul 27, 2022
1 parent 9c3625d commit 8e45a1e
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 64 deletions.
12 changes: 9 additions & 3 deletions packages/core/src/models/interfaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
61 changes: 40 additions & 21 deletions packages/core/src/utils/__tests__/run-lifecycle.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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(() => {});
Expand All @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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({
Expand Down Expand Up @@ -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> [email protected] preversion /test/location\n> test\n`);
});
Expand All @@ -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({
Expand All @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();

Expand All @@ -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 () => {
Expand All @@ -188,7 +190,7 @@ describe('createRunner', () => {
location: 'test',
};

await runPackageLifecycle(pkg, 'prepare');
await runPackageLifecycle(pkg as Package, 'prepare');
expect(runScript).not.toHaveBeenCalled();
});

Expand All @@ -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');
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
82 changes: 47 additions & 35 deletions packages/core/src/utils/run-lifecycle.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
)
);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/publish/src/lib/npm-publish.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/publish/src/lib/pack-directory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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());
Expand Down

0 comments on commit 8e45a1e

Please sign in to comment.