usmanyunusov / nanospinner

🌀 The simplest and tiniest terminal spinner for Node.js
https://npm.im/nanospinner
ISC License
183 stars 15 forks source link

Suggestion: move error handling to method #10

Closed henryruhs closed 2 years ago

henryruhs commented 2 years ago

What do you think about moving this to a method:

function bindExit() {
  process.on('SIGINT', () => {
      spinner.stop();
      process.exit();
  });
}

You could also handle more exit types like:

function bindExit() {
  [
      'SIGHUP',
      'SIGINT',
      'SIGQUIT',
      'SIGTERM',
      'uncaughtException'
  ]
  .forEach(eventType => process.on(eventType, () => {
      spinner.stop();
      process.exit();
  }));
}
usmanyunusov commented 2 years ago

I am going to clean process.on('SIGINT' Developers can handle the exited themselves

usmanyunusov commented 2 years ago

When calling several spinners I am catch an error Chrome(node:56934) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 3 SIGINT listeners added to [process]. Use emitter.setMaxListeners() to increase limit

henryruhs commented 2 years ago

Should be handled via uncaughtException then, but you are right - it would be fine to let developers handle it.

I would love to give a 0.3.0-beta.1 a try.

usmanyunusov commented 2 years ago

Release 0.3.0 (Removed process.on('SIGINT'))