SBoudrias / Inquirer.js

A collection of common interactive command line user interfaces.
MIT License
20.09k stars 1.3k forks source link

feat(inquirer): add AbortSignal support #1524

Closed mshima closed 3 weeks ago

mshima commented 1 month ago

Fixes https://github.com/SBoudrias/Inquirer.js/issues/1521

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@80c6c57). Learn more about missing BASE report.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1524 +/- ## ======================================= Coverage ? 98.25% ======================================= Files ? 37 Lines ? 2403 Branches ? 653 ======================================= Hits ? 2361 Misses ? 36 Partials ? 6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SBoudrias commented 4 weeks ago

I took the liberty to:

  1. Add more documentation and update the timeout demo. AbortSignal is a much better interface, so I want to make sure we surface it :)
  2. Refactored the cleanup logic in @inquirer/core - it wasn't great, and I felt it caused some struggle. It'd have been simpler without promise.cancel() but oh well, hindsight is 20/20! I'm looking forward to Promise.withResolver to make it to all LTS to simplify that further.

I'm very happy with the state of that feature inside @inquirer/core. If you want to merge this sooner, we could extract while we sidebar the inquirer interface question.

mshima commented 3 weeks ago

AbortSignal.any() requires node 18.17.0 and 20.3.0. I don't know a easy way to replace it.

mshima commented 3 weeks ago

This PR now adds Signal support to the createPromptModule, to inquirer.prompt() and uses signal to handle promise.ui.close().

SBoudrias commented 3 weeks ago

Did you look into polyfilling AborySignal.any() yet?

mshima commented 3 weeks ago

Did you look into polyfilling AborySignal.any() yet?

There is AbortSignal polyfills but not any().

SBoudrias commented 3 weeks ago

Great! Thanks for all the back and forth on this one 🤞🏻