Promise.resolve(thenable) executes thenable.then synchronously

[domenic]

From getify/native-promise-only#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).

[erights]

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.

[getify]

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?

[erights]

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 [email protected]:

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//issues/105#issuecomment-42755833
.

Text by me above is hereby placed in the public domain

Cheers,
--MarkM

[stefanpenner]

@getify the wording of immediately fulfilled promise is potentially confusing as a sequence of two operations occur.

1. promise sealing its fate (synchronous/sameTurn)
2. something observing/extracting its fate (asynchronously/nextTurn)

As the consumer is at best notified nextTurn the perspective of the system is promises are async.

[getify]

@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. :)

[stopsopa]

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