| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Review requested:
|
Sorry, something went wrong.
There was a problem hiding this comment.
if it's always making a new promise, any reason not to do this?
| function ensureIsPromise(fn, thisArg, ...args) { | |
| try { | |
| const value = FunctionPrototypeCall(fn, thisArg, ...args); | |
| return isPromise(value) ? value : PromiseResolve(value); | |
| return new Promise((r) => r(value)); | |
| } catch (error) { | |
| return PromiseReject(error); | |
| } | |
| } | |
| async function ensureIsPromise(fn, thisArg, ...args) { | |
| return await FunctionPrototypeCall(fn, thisArg, ...args); | |
| } |
Sorry, something went wrong.
There was a problem hiding this comment.
I like that a lot! An async function always returns a fresh Promise, so this should satisfy Web IDL. 🙂
Sorry, something went wrong.
There was a problem hiding this comment.
Unfortunately, this change causes some tests to fail again... 😞
However, if you remove the await, it does work:
async function ensureIsPromise(fn, thisArg, ...args) {
return FunctionPrototypeCall(fn, thisArg, ...args);
}You still get a fresh Promise (from async function), errors are still correctly turned into rejections, and the timings match Web IDL.
Still, I don't like how sensitive these tests are to the number of microtasks. I'll try to fix this in upstream WPT.
Sorry, something went wrong.
|
cc @nodejs/whatwg-stream |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
PR-URL: #51168 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
this doesn't land cleanly on v20 requires a manual backport |
Sorry, something went wrong.
|
@marco-ippolito This PR builds upon #50107, specifically: 0d45e6f needs a85e418. However, that other PR is marked "dont-land-on-v20.x". 😕 I'm not sure how to proceed with a manual backport. Should I try to rework my changes without that PR, or should we backport #50107 to v20.x anyway? |
Sorry, something went wrong.
Sorry, something went wrong.
|
On second thought, I think I can rework my changes. Let me try something... 👨🔬 |
Sorry, something went wrong.
|
@marco-ippolito I managed to make it work without #50107, see #52773. However, If we also want to backport #50107, then I'll need to change it up again. 😛 Let me know how you want to proceed. |
Sorry, something went wrong.
|
I marked it dont-land because it depended on #47956, which was dont-land. Feel free to remove the label and backport. |
Sorry, something went wrong.
PR-URL: nodejs#51168 Backport-PR-URL: nodejs#52773 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
| Back | FazBrowse Home | New Git URL |
Follow-up on #50126.