fix: remove bluebird #436
fix: remove bluebird #436
Conversation
test/subproc-specs.js
Outdated
| let subproc = new SubProcess('tail', ['-f', path.resolve(__filename)]); | ||
| await subproc.start(); | ||
| subproc.start(); |
There was a problem hiding this comment.
I am not sure it is a good idea to remove await from here
There was a problem hiding this comment.
The executor function can also be an async function . However, this is usually a mistake, for a few reasons: If an async executor function throws an error, the error will be lost and won't cause the newly-constructed Promise to reject. This could make it difficult to debug and handle some errors.
from eslint docs
There was a problem hiding this comment.
I refactored those two tests to use simple async / await. Event listeners are sync functions anyway and don't support async ones (it kind of defeats the point of event listening, which is already async)
Co-authored-by: Kazuaki Matsuo <[email protected]>
|
Just signed it |
| reject(); | ||
| } else { | ||
| resolve(); | ||
| throw new Error(); |
There was a problem hiding this comment.
I believe this change would create an uncaught exception failure
There was a problem hiding this comment.
I tried flipping the if statement and it stops the test, so the exception doesn't hang
| signal.should.equal(stopSignal); | ||
| resolve(); |
There was a problem hiding this comment.
I am not sure this change works as expected in case of a failure
There was a problem hiding this comment.
in the old code it would reject a promise which would return an exception and fail the test. now it just throws directly and also stops the test, so there's virtually no difference:
1) SubProcess
#stop
should send the right signal to stop a proc:
Uncaught
Error
at SubProcess.<anonymous> (test/subproc-specs.js:225:19)
at SubProcess.emit (node:events:520:28)
at SubProcess.emit (node:domain:488:12)
at ChildProcess.<anonymous> (lib/subprocess.js:193:14)
at ChildProcess.emit (node:events:520:28)
at ChildProcess.emit (node:domain:488:12)
at Process.ChildProcess._handle.onexit (node:internal/child_process:294:12)
|
I guess it's not gonna get merged so I'll just close it |
This PR removes Bluebird as it is no longer needed in 2024. All runtimes and browsers (except IE11, which a package consumer should load polyfill for) support Promise API.
All tests pass, no fancy features of Promise are used, just the Promise constructor.