eslint-plugin-unicorn
eslint-plugin-unicorn copied to clipboard
Use `Promise.withResolvers` instead of `new Promise()`?
I think in most cases need a promise, using Promise.withResolvers() should be more readable than new Promise(), because new Promise needs a new function scope.
I opened this issue instead of a rule proposal, since I'm not sure about it and want to discuss it first.
Welcome to post cases where new Promise() should be preferred.
The only case in my mind is the arrow function body.
const foo = () => new Promise(resolve => {
resolve()
});
The benefit of new Promise() is that it makes it harder to make mistakes. For example, it's easier to cause an unhandled rejection with Promise.withResolvers() or accidentally synchronously throw in a function that is supposed to be async.
new Promise(resolve => {
functionThatPotentiallyThrows(); // If this throws, it will cause a proper rejection.
});
I think it depends, new Promise() helps encapsulate the logic and I think it makes sense when you can do so. Promise.withResolvers only becomes useful when the logic is too complex.
A lot of p-defer usage I see is rather smelly and lacks encapsulation.
@sindresorhus you never use p-defer? 🤔
https://github.com/search?q=org%3Asindresorhus+p-defer&type=code
@fregante I'm not saying Promise.withResolvers is not useful, but this issue is asking to prefer Promise.withResolvers, and I'm arguing that new Promise is a better default.
We're saying the same thing: prefer new Promise(), and I notice you do to (judging by the lack of use of p-defer in your own modules)
I misunderstood your statement. I used to use p-defer more before, but transitioned to new Promise for the reasons I mentioned.
I think a rule could prefer withResolvers when it finds code taking resolve/reject functions out of new Promise(), like
let resolve;
const promise = new Promise(res => {
resolve = res
})
return [promise, resolve]
👍
Accepted, with the mentioned limited scope.
Preferred rule name? prefer-promise-with-resolvers?
I've been using the following pattern to await a dialog to close/confirm
const {promise, resolve} = Promise.withResolvers();
Object.assign(fooDialog, {show: true, resolve});
await promise;
But turns out
await new Promise((resolve) ==> Object.assign(fooDialog, {show: true, resolve}))
is simpler.
Though this matches https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2403#issuecomment-2294624254
taking
resolve/rejectfunctions out ofnew Promise()
I guess this should not be reported.
Maybe we should also consider how the promise is used.
@fregante