Inquirer.js icon indicating copy to clipboard operation
Inquirer.js copied to clipboard

feat: add AbortSignal support

Open mshima opened this issue 1 year ago • 1 comments

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

mshima avatar Aug 28 '24 16:08 mshima

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
@@           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.

codecov[bot] avatar Aug 28 '24 16:08 codecov[bot]

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.

SBoudrias avatar Sep 01 '24 21:09 SBoudrias

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

mshima avatar Sep 02 '24 23:09 mshima

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

mshima avatar Sep 03 '24 00:09 mshima

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

SBoudrias avatar Sep 03 '24 01:09 SBoudrias

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

There is AbortSignal polyfills but not any().

mshima avatar Sep 03 '24 01:09 mshima

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

SBoudrias avatar Sep 07 '24 19:09 SBoudrias