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

"killOthers" missing from "ConcurrentlyOptions" type #424

Closed
electrovir opened this issue May 16, 2023 · 4 comments
Closed

"killOthers" missing from "ConcurrentlyOptions" type #424

electrovir opened this issue May 16, 2023 · 4 comments

Comments

@electrovir
Copy link

killOthers is listed in the README:

concurrently/README.md

Lines 330 to 331 in a0dcbd9

- `killOthers`: an array of exitting conditions that will cause a process to kill others.
Can contain any of `success` or `failure`.

But it is not listed in the types:

export type ConcurrentlyOptions = {
logger?: Logger;
/**
* Which stream should the commands output be written to.
*/
outputStream?: Writable;
/**
* Whether the output should be ordered as if the commands were run sequentially.
*/
group?: boolean;
/**
* Comma-separated list of chalk colors to use on prefixes.
*/
prefixColors?: string[];
/**
* Maximum number of commands to run at once.
* Exact number or a percent of CPUs available (for example "50%").
*
* If undefined, then all processes will start in parallel.
* Setting this value to 1 will achieve sequential running.
*/
maxProcesses?: number | string;
/**
* Whether commands should be spawned in raw mode.
* Defaults to false.
*/
raw?: boolean;
/**
* The current working directory of commands which didn't specify one.
* Defaults to `process.cwd()`.
*/
cwd?: string;
/**
* @see CompletionListener
*/
successCondition?: SuccessCondition;
/**
* Which flow controllers should be applied on commands spawned by concurrently.
* Defaults to an empty array.
*/
controllers: FlowController[];
/**
* A function that will spawn commands.
* Defaults to the `spawn-command` module.
*/
spawn: SpawnCommand;
/**
* A function that will kill processes.
* Defaults to the `tree-kill` module.
*/
kill: KillProcess;
/**
* Signal to send to killed processes.
*/
killSignal?: string;
/**
* List of additional arguments passed that will get replaced in each command.
* If not defined, no argument replacing will happen.
*
* @see ExpandArguments
*/
additionalArguments?: string[];
};

@electrovir electrovir changed the title "killOthers" missing from "ConcurrentlyOptions:" type "killOthers" missing from "ConcurrentlyOptions" type May 16, 2023
@electrovir
Copy link
Author

electrovir commented May 16, 2023

It appears that my confusion stems from multiple duplicate variable names within the code.

This copy of ConcurrentlyOptions does indeed contain killOthers:

export type ConcurrentlyOptions = BaseConcurrentlyOptions & {
// Logger options
/**
* Which command(s) should have their output hidden.
*/
hide?: CommandIdentifier | CommandIdentifier[];
/**
* The prefix format to use when logging a command's output.
* Defaults to the command's index.
*/
prefix?: string;
/**
* How many characters should a prefix have at most, used when the prefix format is `command`.
*/
prefixLength?: number;
/**
* Whether output should be formatted to include prefixes and whether "event" logs will be logged.
*/
raw?: boolean;
/**
* Date format used when logging date/time.
* @see https://date-fns.org/v2.0.1/docs/format
*/
timestampFormat?: string;
// Input handling options
defaultInputTarget?: CommandIdentifier;
inputStream?: Readable;
handleInput?: boolean;
pauseInputStreamOnFinish?: boolean;
// Restarting options
/**
* How much time in milliseconds to wait before restarting a command.
*
* @see RestartProcess
*/
restartDelay?: number;
/**
* How many times commands should be restarted when they exit with a failure.
*
* @see RestartProcess
*/
restartTries?: number;
// Process killing options
/**
* Under which condition(s) should other commands be killed when the first one exits.
*
* @see KillOthers
*/
killOthers?: ProcessCloseCondition | ProcessCloseCondition[];
// Timing options
/**
* Whether to output timing information for processes.
*
* @see LogTimings
*/
timings?: boolean;
/**
* List of additional arguments passed that will get replaced in each command.
* If not defined, no argument replacing will happen.
*/
additionalArguments?: string[];
};

In order to use that copy of ConcurrentlyOptions, the default export from concurrently must be used, rather than the destructed export. Note that my concurrently api command was also functioning entirely incorrectly with a destructured import; not seeing the killOthers type was merely a correlated issue.

tl;dr

This works:

import concurrently from 'concurrently';

This does not work:
(it runs, but it bypasses a lot of code that is necessary for expected operation)

import {concurrently} from 'concurrently';

@GabenGar
Copy link

GabenGar commented Aug 8, 2023

Why did you close it? It's clearly a typing issue which can only be fixed upstream.

@electrovir
Copy link
Author

electrovir commented Aug 8, 2023

Why did you close it? It's clearly a typing issue which can only be fixed upstream.

It's more of a code organization problem than a type issue. The type and export names conflict with each other so it's a bit confusing. (While that organization problem definitely requires an upstream fix, as you said, it's not really the scope of this ticket.)

As I discovered above, if you use the default export then you'll have the export that does include the killOthers option:

This works:

import concurrently from 'concurrently';

@paescuj
Copy link
Collaborator

paescuj commented Aug 8, 2023

I think that's what we track in #399.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants