| 19:05 | <bakkot> | Domenic: do you happen to remember why new Promise(res => res(thenable)) calls thenable.then() on the next tick rather than synchronously, in defiance of the A+ spec? cf https://github.com/tc39/ecma262/issues/2770#issuecomment-1121612732 |
| 19:05 | <bakkot> | or bterlson or anyone else who was involved in the details of specifying async/await |
| 19:06 | <bterlson> | I don't recall sorry :( |
| 19:13 | <shu> | thenable delenda est :( |
| 19:15 | <bakkot> | happens when thenable is a native promise too, to be clear |
| 19:22 | <bakkot> | I guess this actually dates back to promises in ES6, which I always forget predate async/await |
| 19:41 | <bakkot> | aha, finally dug up previous discussion: https://github.com/domenic/promises-unwrapping/issues/105 |
| 19:59 | <Mathieu Hofman> | So basically the problem is that new Promise(res => res(thenable)) is not a clean stack butPromise.resolve().then(() => thenable) is a clean stack. As I mentioned earlier, I do believe that there would be a lot less code that will break if only the second case is made synchronous, since it'd be after a return, and thus I believe unlikely that any code would be relying on a new tick |
| 20:01 | <bakkot> | the actual issue Justin Ridgewell was worried about arises from the first case, though |
| 20:01 | <Mathieu Hofman> | does it? My understanding was the adoption of a return promise |
| 20:02 | <Justin Ridgewell> | It falls out of the first case |
| 20:03 | <Justin Ridgewell> | The async functions implicit promise wrapper calls resolve with the return value |
| 20:05 | <bakkot> | when the function returns, res(retv) is called with the (raw, not boxed) return value of the function, and when retv is already a thenable, it waits a tick before invoking retv.then |
| 20:06 | <bakkot> | we could just add a fast-path to res when the then from thenable is the actual Promise.prototype.then, I suppose |
| 20:09 | <Mathieu Hofman> | Oh yeah right, if we had (async () => thenable)() technically that would call the .then synchronously at that point, and not on a clean stack. Ugh |
| 20:09 | <Mathieu Hofman> | we could just add a fast-path to |
| 20:10 | <bakkot> | we already have those with the fast-path in Promise.resolve |
| 20:10 | <Mathieu Hofman> | Should the spec be allowed to recognize Promise.prototype.then form other realms ? |
| 20:10 | <Justin Ridgewell> | We have a similar fast path for await |
| 20:10 | <bakkot> | specifically PromiseResolve does SameValue, which fails cross-realm |
| 20:11 | <bakkot> | and it's fine |
| 21:56 | <shu> | i can imagine that might be a problem for shadowrealms which have different Promise prototypes but the same microtask queue |
| 21:57 | <shu> | i say this, i want to be very clear here, having zero concerns with having it just do SameValue |
| 22:14 | <bakkot> | you can't pass a promise across the shadowrealm boundary anyway |
| 22:51 | <Domenic> | The more you can simplify this mess the better |
| 22:51 | <Domenic> | I'm not sure if adding a fast-path is really simplifying, but I guess it's making things faster, which is also good |
| 22:52 | <Domenic> | But if you can just get rid of all the extra microtasks and "untrusted" code guards and stuff... that was all really misguided. |
| 22:53 | <Domenic> | Thenable support in general, was probably still important, but less important than I thought it would be, and we could have gotten away with something simpler I'm sure. |
| 23:31 | <bakkot> | Yeah, fast-path would be the "we're adding a back-compat hack to make the common case less gross" option: not an actual reduction in complexity when you understand the full machinery in detail but probably a reduction in complexity in practice because the common case ends up cleaner |
| 23:32 | <bakkot> | Mathieu Hofman: this is off topic for the PR thread, but how would having a built-in IsPromise help your use case? IsPromise holds for instances of Promise subclasses, but those can have custom then which the spec will dutifully call (as long as the subclass instances don't reset their .constructor to the native Promise). |
| 23:37 | <Mathieu Hofman> | const isSafePromise = (p) => Promise.isPromise(p) && !Reflect.isExtensible(p) && Reflect.getPrototypeOf(p) === Promise.prototype && !Reflect.getOwnPropertyDescriptor(p, 'then') && !Reflect.getOwnPropertyDescriptor(p, 'constructor') (Given frozen instrinsics) |
| 23:39 | <Mathieu Hofman> | Or whatever Promise subclass the user chooses to trust |
| 23:41 | <Mathieu Hofman> | The problem is that right now the only way to guard against evil thenables is to take a tick hit on every objects values, including unaltered native promises. |
| 23:51 | <bakkot> | what do you mean by "guard against" here? |
| 23:55 | <Mathieu Hofman> | Making sure the untrusted value will not be able to trigger any synchronous execution when doing val.then() or Promise.resolve(val) |
| 23:56 | <Mathieu Hofman> | The only safe way right now is basically something like val !== null && typeof val === 'object' ? Promise.resolve().then(() => val) : Promise.resolve(val) |