Closed domenic closed 10 years ago
Confirmed, and more. Promise.resolve(whatever) should not be invoking the whatever at all, either synchronously or asynchronously. Remember, this is only a renaming of Promise.cast. It should only do a brand check to determine if it is a real promise or not. If it is, it must return it. If it isn't, it must wrap it. For this operation, if whatever is not a real promise, it does not even matter whether it is a thenable or not, as it is being wrapped in the same way regardless.
it does not even matter whether it is a thenable or not, as it is being wrapped in the same way regardless.
I think "being wrapped in the same way" might have a subtle nuance that's being lost, perhaps. Wrapped, yes. In the same way? Does it actually have to be? Here's what I mean:
If x
is a thenable, I can obviously see why that needs to be wrapped normally (that is, make sure NOT to call its then()
until the next cycle).
But, from a performance optimization perspective, it would seem that Promise.resolve(2)
could return an immediately fulfilled promise, right? That is, for the case of non-promise, non-thenables, it returns a wrapping promise, but that promise doesn't have to wait until the next cycle to be fulfilled.
In other words:
x = 10;
y = Promise.resolve(2);
z = 20;
In that snippet, all 3 actions are taken synchronously, and there's nothing else to complete on the next cycle, like fulfilling the y
promise, because y
can be synchronously fulfilled on line 2 of the snippet. Right?
Or am I missing something that makes that unacceptable?
Spec's only traffic in observables. Any change which is unobservable is only an optimization, not a spec change. And in neither case should Promise.resolve schedule anything asynchronously anyway. So what observable difference are we talking about?
On Sat, May 10, 2014 at 3:08 PM, Kyle Simpson notifications@github.comwrote:
it does not even matter whether it is a thenable or not, as it is being wrapped in the same way regardless.
I think "being wrapped in the same way" might have a subtle nuance that's being lost, perhaps. Wrapped, yes. In the same way? Does it actually have to be? Here's what I mean:
If x is a thenable, I can obviously see why that needs to be wrapped normally (that is, make sure NOT to call its then() until the next cycle).
But, from a performance optimization perspective, it would seem that Promise.resolve(2) could return an immediately fulfilled promise, right? That is, for the case of non-promise, non-thenables, it returns a wrapping promise, but that promise doesn't have to wait until the next cycle to be fulfilled.
In other words:
x = 10;y = Promise.resolve(2);z = 20;
In that snippet, all 3 actions are taken synchronously, and there's nothing else to complete on the next cycle, like fulfilling the y promise.
Or am I missing something that makes that unacceptable?
— Reply to this email directly or view it on GitHubhttps://github.com/domenic/promises-unwrapping/issues/105#issuecomment-42755833 .
Text by me above is hereby placed in the public domain
Cheers, --MarkM
@getify the wording of immediately fulfilled promise is potentially confusing as a sequence of two operations occur.
As the consumer is at best notified nextTurn the perspective of the system is promises are async.
@erights: So what observable difference are we talking about?
Your "in the same way" seemed to me to over-constrain what should have just been an implementation detail, but the spirit of my question was to make sure I wasn't missing some way that Promise.resolve(x)
(where x
is a non-promise, non-thenable) could be observable if implemented either sync or async. Sounds like the answer is no. :)
I know that is not place for that but i need help. I'm trying to implement for my own http://promisesaplus.com and i'm stock almost at the end. I dont know what i'm doing wrong. Test looks like:
...
`y` is an object
`then` calls `resolvePromise` synchronously
✓ via return from a fulfilled promise
✓ via return from a rejected promise
`then` calls `resolvePromise` asynchronously
✓ via return from a fulfilled promise
✓ via return from a rejected promise
`y` is an array
`then` calls `resolvePromise` synchronously
✓ via return from a fulfilled promise
✓ via return from a rejected promise
`then` calls `resolvePromise` asynchronously
✓ via return from a fulfilled promise
✓ via return from a rejected promise
`y` is a thenable
`y` is a synchronously-fulfilled custom thenable
`then` calls `resolvePromise` synchronously
1) via return from a fulfilled promise
2) via return from a rejected promise
`then` calls `resolvePromise` asynchronously
3) via return from a fulfilled promise
4) via return from a rejected promise
`y` is an asynchronously-fulfilled custom thenable
`then` calls `resolvePromise` synchronously
5) via return from a fulfilled promise
6) via return from a rejected promise
`then` calls `resolvePromise` asynchronously
7) via return from a fulfilled promise
8) via return from a rejected promise
`y` is a synchronously-fulfilled one-time thenable
`then` calls `resolvePromise` synchronously
9) via return from a fulfilled promise
...
Code is under: https://github.com/stopsopa/ipromise The error is somewhere after "isFunction(then)" in line 92
Or tell me please where to go to get help :D
From https://github.com/getify/native-promise-only/issues/5
As I understand it, this is really bad, as then the thenable can break the surrounding code's invariants. @erights @kriskowal can confirm.
I think the fix for this is pretty simple, and will be proposing it in a pull request shortly. (Just wrap the
then
call in an EnqueueTask, essentially).