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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable TypeScript strict mode #374

Merged
merged 17 commits into from Nov 13, 2022
Merged
Show file tree
Hide file tree
Changes from 15 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
8 changes: 3 additions & 5 deletions bin/concurrently.spec.ts
Expand Up @@ -59,12 +59,10 @@ const run = (args: string, ctrlcWrapper?: boolean) => {

const stdout = readline.createInterface({
input: child.stdout,
output: null,
});

const stderr = readline.createInterface({
input: child.stderr,
output: null,
});

const log = new Rx.Observable<string>((observer) => {
Expand All @@ -82,8 +80,8 @@ const run = (args: string, ctrlcWrapper?: boolean) => {
});

const exit = Rx.firstValueFrom(
Rx.fromEvent(child, 'exit').pipe(
map((exit: [number | null, NodeJS.Signals | null]) => {
Rx.fromEvent<[number | null, NodeJS.Signals | null]>(child, 'exit').pipe(
paescuj marked this conversation as resolved.
Show resolved Hide resolved
map((exit) => {
return {
/** The exit code if the child exited on its own. */
code: exit[0],
Expand Down Expand Up @@ -180,7 +178,7 @@ describe('exiting conditions', () => {
// Instruct the wrapper to send CTRL+C to its child
sendCtrlC(child.process);
} else {
process.kill(child.pid, 'SIGINT');
process.kill(Number(child.pid), 'SIGINT');
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/command-parser/expand-npm-shortcut.spec.ts
Expand Up @@ -3,7 +3,7 @@ import { ExpandNpmShortcut } from './expand-npm-shortcut';

const parser = new ExpandNpmShortcut();

const createCommandInfo = (command: string, name?: string): CommandInfo => ({
const createCommandInfo = (command: string, name = ''): CommandInfo => ({
name,
command,
});
Expand Down
2 changes: 1 addition & 1 deletion src/command-parser/expand-npm-wildcard.spec.ts
Expand Up @@ -30,7 +30,7 @@ describe('ExpandNpmWildcard#readPackage', () => {
if (path === 'package.json') {
return JSON.stringify(expectedPackage);
}
return null;
return '';
});

const actualReadPackage = ExpandNpmWildcard.readPackage();
Expand Down
19 changes: 16 additions & 3 deletions src/command.spec.ts
Expand Up @@ -44,7 +44,7 @@ beforeEach(() => {
killProcess = jest.fn();
});

const createCommand = (overrides?: Partial<CommandInfo>, spawnOpts?: SpawnOptions) => {
const createCommand = (overrides?: Partial<CommandInfo>, spawnOpts: SpawnOptions = {}) => {
const command = new Command(
{ index: 0, name: '', command: 'echo foo', ...overrides },
spawnOpts,
Expand Down Expand Up @@ -198,7 +198,7 @@ describe('#start()', () => {
const { command } = createCommand();
const stdout = Rx.firstValueFrom(command.stdout);
command.start();
process.stdout.emit('data', Buffer.from('hello'));
process.stdout?.emit('data', Buffer.from('hello'));

expect((await stdout).toString()).toBe('hello');
});
Expand All @@ -207,7 +207,7 @@ describe('#start()', () => {
const { command } = createCommand();
const stderr = Rx.firstValueFrom(command.stderr);
command.start();
process.stderr.emit('data', Buffer.from('dang'));
process.stderr?.emit('data', Buffer.from('dang'));

expect((await stderr).toString()).toBe('dang');
});
Expand Down Expand Up @@ -252,3 +252,16 @@ describe('#kill()', () => {
expect(close).toMatchObject({ exitCode: 1, killed: true });
});
});

describe('.canKill()', () => {
it('returns whether command has both PID and process', () => {
const command = createCommand();
expect(Command.canKill(command)).toBe(false);

command.pid = 1;
expect(Command.canKill(command)).toBe(false);

command.process = process;
expect(Command.canKill(command)).toBe(true);
});
});
22 changes: 16 additions & 6 deletions src/command.ts
Expand Up @@ -92,7 +92,7 @@ export class Command implements CommandInfo {
readonly command: string;

/** @inheritdoc */
readonly prefixColor: string;
readonly prefixColor?: string;
paescuj marked this conversation as resolved.
Show resolved Hide resolved

/** @inheritdoc */
readonly env: Record<string, unknown>;
Expand All @@ -112,8 +112,9 @@ export class Command implements CommandInfo {
killed = false;
exited = false;

/** @deprecated */
paescuj marked this conversation as resolved.
Show resolved Hide resolved
get killable() {
return !!this.process;
return Command.canKill(this);
}

constructor(
Expand All @@ -126,7 +127,7 @@ export class Command implements CommandInfo {
this.name = name;
this.command = command;
this.prefixColor = prefixColor;
this.env = env;
this.env = env || {};
this.cwd = cwd;
this.killProcess = killProcess;
this.spawn = spawn;
Expand Down Expand Up @@ -161,7 +162,7 @@ export class Command implements CommandInfo {
this.close.next({
command: this,
index: this.index,
exitCode: exitCode === null ? signal : exitCode,
exitCode: exitCode ?? String(signal),
killed: this.killed,
timings: {
startDate,
Expand All @@ -173,18 +174,27 @@ export class Command implements CommandInfo {
);
child.stdout && pipeTo(Rx.fromEvent<Buffer>(child.stdout, 'data'), this.stdout);
child.stderr && pipeTo(Rx.fromEvent<Buffer>(child.stderr, 'data'), this.stderr);
this.stdin = child.stdin;
this.stdin = child.stdin || undefined;
}

/**
* Kills this command, optionally specifying a signal to send to it.
*/
kill(code?: string) {
if (this.killable) {
if (Command.canKill(this)) {
this.killed = true;
this.killProcess(this.pid, code);
}
}

/**
* Detects whether a command can be killed.
*
* Also works as a type guard on the input `command`.
*/
static canKill(command: Command): command is Command & { pid: number; process: ChildProcess } {
return !!command.pid && !!command.process;
}
}

/**
Expand Down
8 changes: 6 additions & 2 deletions src/completion-listener.ts
Expand Up @@ -92,11 +92,15 @@ export class CompletionListener {
bufferCount(closeStreams.length),
switchMap((exitInfos) =>
this.isSuccess(exitInfos)
? Rx.of(exitInfos, this.scheduler)
: Rx.throwError(exitInfos, this.scheduler)
? this.emitWithScheduler(Rx.of(exitInfos))
: this.emitWithScheduler(Rx.throwError(exitInfos))
),
take(1)
)
);
}

private emitWithScheduler<O>(input: Rx.Observable<O>): Rx.Observable<O> {
return this.scheduler ? input.pipe(Rx.observeOn(this.scheduler)) : input;
paescuj marked this conversation as resolved.
Show resolved Hide resolved
}
}
17 changes: 8 additions & 9 deletions src/concurrently.ts
Expand Up @@ -30,7 +30,7 @@ const defaults: ConcurrentlyOptions = {
* If value is a string, then that's the command's command line.
* Fine grained options can be defined by using the object format.
*/
export type ConcurrentlyCommandInput = string | Partial<CommandInfo>;
export type ConcurrentlyCommandInput = string | ({ command: string } & Partial<CommandInfo>);
paescuj marked this conversation as resolved.
Show resolved Hide resolved

export type ConcurrentlyResult = {
/**
Expand Down Expand Up @@ -173,14 +173,17 @@ export function concurrently(
onFinishCallbacks: _.concat(onFinishCallbacks, onFinish ? [onFinish] : []),
};
},
{ commands, onFinishCallbacks: [] }
{ commands, onFinishCallbacks: [] } as {
commands: Command[];
onFinishCallbacks: (() => void)[];
}
);
commands = handleResult.commands;

if (options.logger && options.outputStream) {
const outputWriter = new OutputWriter({
outputStream: options.outputStream,
group: options.group,
group: !!options.group,
commands,
});
options.logger.output.subscribe(({ command, text }) => outputWriter.write(command, text));
Expand All @@ -206,14 +209,10 @@ export function concurrently(

function mapToCommandInfo(command: ConcurrentlyCommandInput): CommandInfo {
if (typeof command === 'string') {
return {
command,
name: '',
env: {},
cwd: '',
};
return mapToCommandInfo({ command });
}

assert.ok(command.command, '[concurrently] command cannot be empty');
return {
command: command.command,
name: command.name || '',
Expand Down
42 changes: 22 additions & 20 deletions src/flow-control/input-handler.spec.ts
Expand Up @@ -37,27 +37,27 @@ it('does nothing if called without input stream', () => {
}).handle(commands);
inputStream.write('something');

expect(commands[0].stdin.write).not.toHaveBeenCalled();
expect(commands[0].stdin?.write).not.toHaveBeenCalled();
});

it('forwards input stream to default target ID', () => {
controller.handle(commands);

inputStream.write('something');

expect(commands[0].stdin.write).toHaveBeenCalledTimes(1);
expect(commands[0].stdin.write).toHaveBeenCalledWith('something');
expect(commands[1].stdin.write).not.toHaveBeenCalled();
expect(commands[0].stdin?.write).toHaveBeenCalledTimes(1);
expect(commands[0].stdin?.write).toHaveBeenCalledWith('something');
expect(commands[1].stdin?.write).not.toHaveBeenCalled();
});

it('forwards input stream to target index specified in input', () => {
controller.handle(commands);

inputStream.write('1:something');

expect(commands[0].stdin.write).not.toHaveBeenCalled();
expect(commands[1].stdin.write).toHaveBeenCalledTimes(1);
expect(commands[1].stdin.write).toHaveBeenCalledWith('something');
expect(commands[0].stdin?.write).not.toHaveBeenCalled();
expect(commands[1].stdin?.write).toHaveBeenCalledTimes(1);
expect(commands[1].stdin?.write).toHaveBeenCalledWith('something');
});

it('forwards input stream to target index specified in input when input contains colon', () => {
Expand All @@ -66,29 +66,29 @@ it('forwards input stream to target index specified in input when input contains
inputStream.emit('data', Buffer.from('1::something'));
inputStream.emit('data', Buffer.from('1:some:thing'));

expect(commands[0].stdin.write).not.toHaveBeenCalled();
expect(commands[1].stdin.write).toHaveBeenCalledTimes(2);
expect(commands[1].stdin.write).toHaveBeenCalledWith(':something');
expect(commands[1].stdin.write).toHaveBeenCalledWith('some:thing');
expect(commands[0].stdin?.write).not.toHaveBeenCalled();
expect(commands[1].stdin?.write).toHaveBeenCalledTimes(2);
expect(commands[1].stdin?.write).toHaveBeenCalledWith(':something');
expect(commands[1].stdin?.write).toHaveBeenCalledWith('some:thing');
});

it('forwards input stream to target name specified in input', () => {
controller.handle(commands);

inputStream.write('bar:something');

expect(commands[0].stdin.write).not.toHaveBeenCalled();
expect(commands[1].stdin.write).toHaveBeenCalledTimes(1);
expect(commands[1].stdin.write).toHaveBeenCalledWith('something');
expect(commands[0].stdin?.write).not.toHaveBeenCalled();
expect(commands[1].stdin?.write).toHaveBeenCalledTimes(1);
expect(commands[1].stdin?.write).toHaveBeenCalledWith('something');
});

it('logs error if command has no stdin open', () => {
commands[0].stdin = null;
commands[0].stdin = undefined;
controller.handle(commands);

inputStream.write('something');

expect(commands[1].stdin.write).not.toHaveBeenCalled();
expect(commands[1].stdin?.write).not.toHaveBeenCalled();
expect(logger.logGlobalEvent).toHaveBeenCalledWith(
'Unable to find command 0, or it has no stdin open\n'
);
Expand All @@ -99,8 +99,8 @@ it('logs error if command is not found', () => {

inputStream.write('foobar:something');

expect(commands[0].stdin.write).not.toHaveBeenCalled();
expect(commands[1].stdin.write).not.toHaveBeenCalled();
expect(commands[0].stdin?.write).not.toHaveBeenCalled();
expect(commands[1].stdin?.write).not.toHaveBeenCalled();
expect(logger.logGlobalEvent).toHaveBeenCalledWith(
'Unable to find command foobar, or it has no stdin open\n'
);
Expand All @@ -112,7 +112,8 @@ it('pauses input stream when finished', () => {
const { onFinish } = controller.handle(commands);
expect(inputStream.readableFlowing).toBe(true);

onFinish();
expect(onFinish).toBeDefined();
onFinish?.();
expect(inputStream.readableFlowing).toBe(false);
});

Expand All @@ -124,6 +125,7 @@ it('does not pause input stream when pauseInputStreamOnFinish is set to false',
const { onFinish } = controller.handle(commands);
expect(inputStream.readableFlowing).toBe(true);

onFinish();
expect(onFinish).toBeDefined();
onFinish?.();
expect(inputStream.readableFlowing).toBe(true);
});
13 changes: 7 additions & 6 deletions src/flow-control/input-handler.ts
Expand Up @@ -19,7 +19,7 @@ import { FlowController } from './flow-controller';
export class InputHandler implements FlowController {
private readonly logger: Logger;
private readonly defaultInputTarget: CommandIdentifier;
private readonly inputStream: Readable;
private readonly inputStream?: Readable;
private readonly pauseInputStreamOnFinish: boolean;

constructor({
Expand All @@ -28,7 +28,7 @@ export class InputHandler implements FlowController {
pauseInputStreamOnFinish,
logger,
}: {
inputStream: Readable;
inputStream?: Readable;
logger: Logger;
defaultInputTarget?: CommandIdentifier;
pauseInputStreamOnFinish?: boolean;
Expand All @@ -43,12 +43,13 @@ export class InputHandler implements FlowController {
commands: Command[];
onFinish?: () => void | undefined;
} {
if (!this.inputStream) {
const { inputStream } = this;
if (!inputStream) {
return { commands };
}

Rx.fromEvent(this.inputStream, 'data')
.pipe(map((data) => data.toString()))
Rx.fromEvent(inputStream, 'data')
.pipe(map((data) => String(data)))
.subscribe((data) => {
const dataParts = data.split(/:(.+)/);
const targetId = dataParts.length > 1 ? dataParts[0] : this.defaultInputTarget;
Expand All @@ -74,7 +75,7 @@ export class InputHandler implements FlowController {
onFinish: () => {
if (this.pauseInputStreamOnFinish) {
// https://github.com/kimmobrunfeldt/concurrently/issues/252
this.inputStream.pause();
inputStream.pause();
}
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/flow-control/kill-others.ts
Expand Up @@ -8,7 +8,7 @@ import { FlowController } from './flow-controller';
export type ProcessCloseCondition = 'failure' | 'success';

/**
* Sends a SIGTERM signal to all commands when one of the exits with a matching condition.
* Sends a SIGTERM signal to all commands when one of the commands exits with a matching condition.
*/
export class KillOthers implements FlowController {
private readonly logger: Logger;
Expand Down