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

Feature: Abort controller support #527

Closed
vitalics opened this issue Oct 10, 2022 · 5 comments · Fixed by #734
Closed

Feature: Abort controller support #527

vitalics opened this issue Oct 10, 2022 · 5 comments · Fixed by #734
Labels

Comments

@vitalics
Copy link

vitalics commented Oct 10, 2022

As a part of node.js 16. Abort signal stable feature. I would like to propose Abort signal and Abort Controller support.

API:

import { $ } from 'zx';
const controller = new AbortController();

$('some long command', {signal: controller.signal});

if(somethingHapens){ // e.g. ctrl+c
controller.abort();
}
// some long command will be halted.
@antonmedv
Copy link
Collaborator

What about kill() method? It already works.

@vitalics
Copy link
Author

yes, that's worked, but the reason is resources. When signal comes for promises, it's not stops actually. and resources are not flushed. I mean what's happens when do the snippet:

function delay(ms: number){
	return new Promise(res => setTimeout(res, ms)})
}

await Promise.race([
delay(1000),
asyncFnLessThan1000ms(),
])

set timeout will not stops actually, results will be ignored (it takes machine time)

But if we use the signal to stop any async code(setTimeout in this case) - nodejs will real stop setTimeout fn and flush resources.

The same behavior for the child process, This feature will reduce process memory/time management

@gajus
Copy link

gajus commented Feb 24, 2023

Native intertop with signal would make the API a lot nicer to use.

Now the API feels awkward:

handle: async ({ signal }) => {
  const promise = $`sleep 30`;

  const kill = () => {
    promise.kill();
  };

  signal.addEventListener('abort', kill, { once: true });

  await promise;

  signal.removeEventListener('abort', kill);
},

If zx added native support, it could be simplified down to:

handle: async ({ signal }) => {
  await $({signal})`sleep 30`;
},

This API is similar to how sql.type() is used in Slonik, example.

@antonmedv
Copy link
Collaborator

Please show an example with usage with signal vs kill()

@gajus
Copy link

gajus commented Feb 24, 2023

Updated

antongolub added a commit to antongolub/zx that referenced this issue Mar 17, 2024
antonmedv pushed a commit that referenced this issue Mar 17, 2024
* feat: introduce `abort()` method

closes #527

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

Successfully merging a pull request may close this issue.

3 participants