open-cli-tools / concurrently

Run commands concurrently. Like `npm run watch-js & npm run watch-less` but better.
https://www.npmjs.com/package/concurrently
MIT License
7k stars 228 forks source link

"killOthers" missing from "ConcurrentlyOptions" type #424

Closed electrovir closed 1 year ago

electrovir commented 1 year ago

killOthers is listed in the README: https://github.com/open-cli-tools/concurrently/blob/a0dcbd9bf2917401d777c567227a4aeabe3d17ba/README.md?plain=1#L330-L331

But it is not listed in the types: https://github.com/open-cli-tools/concurrently/blob/a0dcbd9bf2917401d777c567227a4aeabe3d17ba/src/concurrently.ts#L51-L125

electrovir commented 1 year ago

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

This copy of ConcurrentlyOptions does indeed contain killOthers: https://github.com/open-cli-tools/concurrently/blob/a0dcbd9bf2917401d777c567227a4aeabe3d17ba/src/index.ts#L21-L92

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 commented 1 year ago

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

electrovir commented 1 year ago

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 commented 1 year ago

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