| 02:17 | <Jack Works> | Unlucky |
| 02:17 | <Jack Works> | What's the reason it being rejected? |
| 03:28 | <devsnek> | it was considered to be adding too much complexity |
| 03:28 | <devsnek> | ultimately i think that was a mistake (i still see people run into the dynamic import problem in the wild) but what can ya do 🤷 |
| 03:30 | <devsnek> | hmm here are the notes https://github.com/tc39/notes/blob/main/meetings/2018-05/may-24.md#symbolthenable-for-stage-1-or-2 |
| 04:04 | <Mathieu Hofman> | Is it possible to add a Symbol.unthenable and attach it to the module namespace object? |
| 07:26 | <Rob Palmer> | Just some history... the hazard of dynamic import resolving thenables was known ahead of time. I tried to point it out but it didn't end up going anywhere as it was framed as yet-another-instance of an existing hazard. https://github.com/tc39/proposal-dynamic-import/issues/48 |
| 18:59 | <ljharb> | I think unfortunately that ship has definitely sailed. At this point adding such a symbol to the language wouldn't accomplish anything. You can't add such a symbol automatically to all module namespace objects as it'd be Web incompatible. And developers can't add such a symbol themselves (only string identifiers can be exports) before they'd get tripped by a dynamic import. |
| 19:00 | <ljharb> | or do you just mean, making it unthenable would be incompatible |
| 19:01 | <Mathieu Hofman> | I meant adding a symbol such as unthenable by default to all module namespace objects. There are modules in the wild that do expect this thenable behavior |
| 19:01 | <ljharb> | right, true enough. altho probably not a ton, since having a thenable module is broken in node (i forget if it's broken just in the repl, or just not in the repl) |
| 19:04 | <Mathieu Hofman> | It definitely works in the browser, I have personally (ab)used this mechanism |
| 19:41 | <Justin Ridgewell> | Ooo boy, did anyone realize we don't actually follow the Promises A+ spec? |
| 19:42 | <Justin Ridgewell> | We pass the spec's tests, only because it forgot to test a behavior. |
| 19:42 | <Justin Ridgewell> | Step 13-15 of https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promise-resolve-functions |
| 19:43 | <Justin Ridgewell> | We create a new job to handle a thenable passed to resovle in new Promise(res => res(thenable)) |
| 19:43 | <Justin Ridgewell> | That's not correct |
| 19:43 | <Justin Ridgewell> | https://promisesaplus.com/#point-56 |
| 19:44 | <Justin Ridgewell> |
|
| 19:44 | <Justin Ridgewell> | It makes no mention of a new job for thenables |
| 19:45 | <Justin Ridgewell> | This is the root cause of both https://github.com/tc39/ecma262/issues/2770 and https://github.com/tc39/ecma262/issues/1250 |
| 19:47 | <Justin Ridgewell> | thenable.then(resolve) and resolve(thenable) are supposed to take the same number of ticks. |
| 19:48 | <Justin Ridgewell> | But because resolve(thenable) creates a new job for doing the then.call(prom, onFul, onRej) and the then internally creates a new job before calling onFul/onRej, we get 2 ticks instead of 1. |
| 20:05 | <Mathieu Hofman> | It might be tricky to untangle this. Promise.resolve() should most likely not call .then in the same tick (It's already unfortunate it does a [[Get]] of the constructor and then). Maybe there is a way to not use a new callback job if we're already inside then callback job? I haven't thought this through yet. |
| 20:10 | <Justin Ridgewell> | Are you thinking about throws? |
| 20:11 | <Justin Ridgewell> | Note that Promise.resolve isn't part of A+, so we could diverge and do an tick before invoking the thenable's then |
| 20:11 | <Mathieu Hofman> | I'm wondering how you could test this though. The .then call would always happen when no user code is on the stack. Maybe the test could create a resolved promise and attach a fulfillment handler (as a "next tick" detection) before returning the test thenable, and ensure that the .then is called before the "next tick" ? |
| 20:14 | <Justin Ridgewell> | It's easy, I think: |
| 20:15 | <Mathieu Hofman> | ah right, the constructor calls the resolve step, silly me |
| 20:16 | <Justin Ridgewell> | Can you explain why Promise.resolve(thenable) shouldn't call thenable.then sync? |
| 20:18 | <Mathieu Hofman> | It's just a non-obvious reentrancy hazard. Basically the only way to protect yourself from an evil promise is to do Promise.resolve(() => thenable) |
| 20:18 | <Justin Ridgewell> | Promise A+ handles reentrancy |
| 20:19 | <Mathieu Hofman> | Which means you're always losing a tick when you don't know where a potential thenable is coming from. |
| 20:19 | <Mathieu Hofman> | I'm talking about user code trying to protect itself from reentrancy induced by evil promises |
| 20:19 | <Justin Ridgewell> | The onFul and onRej passed to the thenable's then are already job deferred callbacks |
| 20:20 | <Justin Ridgewell> | They can only be called once |
| 20:20 | <Justin Ridgewell> | And if then throws, then it'll be caught and onRej will be called (if neither onFul nor onRej have already been called) |
| 20:21 | <Mathieu Hofman> | Aye, we're not talking about the same thing. |
| 20:24 | <Mathieu Hofman> | calling fooResult.then() can cause synchronous reentrancy in your code if you can't trust where fooResult comes from. And there is simply no way to brand check for a native promise without causing similar hazard (Promise.resolve will trigger constructor and then get logic) |
| 20:25 | <Justin Ridgewell> | Sorry, I'm still not getting it |
| 20:26 | <Justin Ridgewell> | If you can't trust fooResult, then Promise.resolve(fooResult) would give you a trusted promise (and I assume you're already doing this because fooResult isn't trusted) |
| 20:27 | <Justin Ridgewell> | So we haven't introduced a new tick? |
| 20:27 | <bakkot> | I don't think code should be trying to protect itself from reentrancy induced by evil promises |
| 20:27 | <bakkot> | except in extremely unusual cases |
| 20:28 | <Mathieu Hofman> | Promise.resolve({ get then() { doSomethingEvil(); return () => {}; } }) will call doSomethingEvil synchronously. |
| 20:28 | <Mathieu Hofman> | I don't think code should be trying to protect itself from reentrancy induced by evil promises |
| 20:29 | <bakkot> |
|
| 20:30 | <Justin Ridgewell> | Given get then() { doSomethingEvil() }, is sync calling then() { doSomethingEvil() } something we need to protect against? |
| 20:32 | <Mathieu Hofman> | Anyway, we're off topic for Justin's topic. I just wish we had a way to brand check native promises so that something like brandCheck(p) && Reflect.getPrototypeOf(p) === Promise.prototype && !Reflect.getOwnPropertyDescriptor(p) would be sufficient to know I can safely call p.then |
| 20:33 | <Mathieu Hofman> | (given that Promise.prototype is frozen of course) |
| 21:04 | <ljharb> | yes, it is, because Promise.resolve({ then: class {} }) shouldn't throw, it should reject |
| 21:07 | <Justin Ridgewell> | It would still reject with the correct A+ behavior |
| 21:07 | <Justin Ridgewell> | Throws are caught |
| 22:56 | <ljharb> | i'd love a stamp on https://github.com/tc39/notes/pull/197 if someone has a sec |
| 23:07 | <rbuckton> | Rather than adding a symbol to the module, could we allow a user to add a built-in symbol to a function they export named then that would indicate it shouldn't be used for Promise fulfillment? Non-native promises might not understand it, but it would still work for imports. |
| 23:24 | <Mathieu Hofman> | Interesting idea. But that's a protocol that would have to work for every thenable, not just module namespaces. And that means one more lookup during promise resolve. |
| 23:59 | <Jack Works> | I think unfortunately that ship has definitely sailed. At this point adding such a symbol to the language wouldn't accomplish anything. You can't add such a symbol automatically to all module namespace objects as it'd be Web incompatible. And developers can't add such a symbol themselves (only string identifiers can be exports) before they'd get tripped by a dynamic import. js import * as self from './file.js' self[Symbol.thenable] = false |