I think that this is a bad approach

[zloirock]

The design of promises is based on the principle of duck typing and interaction between promises from different authors on the thenable protocol. Native promises are not the only popular promises on the web. For example, promises from babel-runtime -> core-js-pure are wrapped and, in some cases, are not native. Bluebird and other userland Promise implementations are still very popular. Even jQuery could return a promise that will work by the thenable protocol, but will not be a native promise.

Your code could get an instance of alternative promise implementations from third-party code and something like let foo = Promise.isPromise(bar) ? await bar : bar will be broken.

Error.isError? It works with userland errors since they are native error subclasses. That's false for promises.

Promise.isThenable that will check the existence of a callable .then will work much better for the case of optimization.

In my vision, adding Promise.isPromise could be good for native class identification in low-level utilities, together and at the same time with Map.isMap and all the rest similar constructors / cases, but not for optimization.

[ljharb]

isThenable is just x && typeof x.then === 'function'.

You certainly wouldn’t only await things that were isPromise - that’s not the use case for the brand check.

[michaelficarra]

@zloirock I think you're getting caught up in the name instead of the listed motivation. The motivation is only to detect what kinds of values will be handled specially by await, as native promises (and only native promises) are today. I think a rename as I suggested in #3 might make that clearer.

[mhofman]

In my vision, adding Promise.isPromise could be good for native class identification in low-level utilities

That is exactly the goal of this proposal.

for the case of optimization.

I don't see how this proposal or any thenable check relates to optimization. I understand that some people attempt to switch to synchronous behavior and conditional await, but that is not something we would ever encourage, and I don't believe this proposal makes that any easier, since @ljharb says, these misguided optimizations can already check if a value is thenable.

I will see how we could make the nature of the check more clear, but I believe a component of this will have to be documentation like MDN.

[zloirock]

isThenable is just x && typeof x.then === 'function'.

The first part of this is definitely incorrect. It should be checked on object or null / undefined. Both parts of this should be protected from [[IsHTMLDDA]], that's extra code in userland. We don't want any abstraction leakage, do we? Even if it's simple, it's not a reason not to add it - for example, we have Number.isNaN that's could be implemented in 4 chars (x!=x).

I think you're getting caught up in the name instead of the listed motivation.

Nope, I think that the naming is correct. The problem is different.

Brand check of promises could be solved with .then. Yes, it's heavy. But... How often brand checking of native promises is required? It's very rare case, since handling of promises in most cases based on the thenable protocol. The above mentioned case of isMap is more popular and also is not lightweight (for example, bicycle based on try-catch and methods that could have logarithmic complexity) - and it's similar for almost the rest other built-ins. The most of users even are forced to write bicycles even for isObject. Bugs, incompleteness, incorrectness are often parts of such bicycles. These bicycles consume a significant amount of network traffic. This causes problems for everyone.

If this proposal is about optimization promises handling - it should be isThenable, if it's about brand checking - should be added a complex of utilites for brand checking / identification of all built-in instances.

[mhofman]

While a thenable check is possibly "complex", it is possible to implement in userland.

A "native promise brand check" however is downright impossible to implement in userland without side-effects, as explained in the readme. Looking up .then is NOT a brand check.

brand checking native promises is needed for any defensive code that wants to know whether the async/await or native promise machinery will be triggering non-native behavior (.then of objects, which merely checking for existence could also trigger user code, because of proxies). I do agree it's likely not a feature that will be widely used by end authors, but it would be by lower level libraries. In general brand checking is not something needed by most authors.

Afaik, there is already a way to brand check most other built-ins, sometimes in a convoluted way. I suspect @ljharb has a cheat sheet lying around for them. Again, in most cases brand checking is not something regular JS authors need to do, and I'm not convinced we need these checks to be ergonomic.

I would be happy to have a non-ergonomic brand check for native promises, I'm just not sure what that would look like.

[zloirock]

A "native promise brand check" however is downright impossible to implement in userland without side-effects, as explained in the readme. Looking up .then is NOT a brand check.

? O_o

[mhofman]

I'm not sure what I'm supposed to understand from these? Yes Promise.prototype.then internally performs a brand check, but it has side-effects in the positive case (species lookup & the rest of the then machinery).

[zloirock]

Your README explains only the case with promise adoption that's missed here, so I'm confused why you're against the species lookup, since in the userland implementation of isThenable case that you recommend presents the lookup of .then.

So, what's the real use case of isPromise (not isThenable)? For me, for all time, it was required only inside promise polyfills - and the brand check was available inside polyfills internals. I don't remember any cases of this in millions of lines of utilities and userland code that I saw.

[mhofman]

Right, Promise.resolve() has side-effects for the negative case (non-native promises), and Promise.prototype.then has side-effects in the positive case (native promises). There is no way to build a brand check without side effects.

Knowing whether a promise is a native promise is required to know whether some operations can cause unexpected code to execute.

await foo will not cause any user-code to execute synchronously if foo is a native promise. If tc39/ecma262#3689 is adopted, so would Promise.withResolvers().resolve(foo).

Basically it's exposing whether the promise implementation will be adopting the internal state of one of its own objects, or whether it will be assimilating the object (possibly through a .then, if present).

We also have a case that effectively classifies the type of values in nested structures in order to expose them to remote systems. One type of acceptable values are promises, but only native ones as we don't consider thenables safe to interact with (there is no way to guarantee their behavior). We do not want to cause side-effects in our classifier.

[zloirock]

So, what about real use cases? It's highly doubtful that this example with synchronous promises will be of any use to anyone in real life - there is no big difference is something called syncroniously or in microtask if anyway we are awaiting the result. Moreover, it would still require variable identification as a thenable that's a side effect.

[mhofman]

I'm not sure why you consider these use cases aren't real. They are very much a problem in our environment. I never claimed this brand check would have wide developer adoption. Except for Array.isArray, almost no brand check is needed by regular developers in JavaScript.

Furthermore vulnerabilities arising from unexpected synchronous reentrancy are very real (mostly in "host" code, but not exclusively), and an increasingly amount of host-like code is written as JavaScript, and cannot assume to have privileged access to the engine's internal. While reentrancy is not as much of a problem for await, it very much is for the resolver functions.

[jridgewell]

A problem with isThenable: A+ implementations can't use it. A+ impls must only access .then a single time, and if it's a function, invoke it. isFoo() methods return booleans.

Because using Promise.isThenable(foo) accesses .then and doesn't return it, we can't get the method value to invoke it without accessing it a second time.