Why does this check for IsPromise?

[evilpie]

I am going over the patch from @anba for the Firefox implementation. He made this comment:

// Step 2.
// TODO: Why does the proposal use a brand check, the steps
// are general enough to work on any object. Cf. Promise.prototype.catch.

I don't follow the Promise implementation, but this observation makes sense to me.

[littledan]

A change was made at the end of this thread: #37 . But if you're asking, why try to get to a native promise at all? That might go further back, when there was an attempt to make the resolve/reject callbacks not observable.

[ljharb]

In general yes, it's to reduce observability.

Specifically about step 2; .catch may be generic, but it calls into .then, which has the same step 2: https://tc39.github.io/ecma262/#sec-promise.prototype.then

In other words, catch not having it is a mistake imo (and in the majority case, it effectively has the brand check anyways), and I didn't want that mistake to be repeated on finally.

[anba]

I think the spec normally only applies brand checks were necessary, see for example Promise.prototype.catch and the various RegExp.prototype methods. So having the brand check in Promise.prototype.finally feels off and kind of creates an inconsistency in the language. Are there any other built-ins with a brand check even though the check is not necessary?

[ljharb]

(The RegExp.prototype methods use IsRegExp, which either look for Symbol.match or a brand check)

To me, it is necessary here (because I think all APIs should be as strict as possible); what's the use case for having finally be used on a generic object? Note that even if I removed the brand check here, in the 99% case, it'd delegate to .then which has the same brand check - and extends Promise instances would pass the brand check.

[allenwb]

I agree with @anba WRT this issue.

think all APIs should be as strict as possible

this is not the design philosophy that ECMAScript has traditionally followed. We generally specify built-in methods to be a generic as possibly so they can be used with alternative object implementations that support a common API. Generally, brand checks are used in situations where where an algorithm has dependencies on the existence of an internal slot.

IsRegExp is used when an built-in has different behavior depending upon whether a string or a "regular expression" as passed to it as an argument. A "regular expression" is defined to be any object that has a Symbol.match method.

Note that the delegation to "then" in finally is a generic invocation, not a call to s specific method implementation or abstract operation. It may not may not end up invoking the built-in method that does a brand check. Basically, the promise finally method should be usable with any object that implements "then".

[ljharb]

I understand the traditionally followed philosophy; however, I'm asking what the use case is.

If we keep the brand check, it can always be removed later - if we remove it now, it can't be added back later. Is there a concrete reason to loosen it up besides "consistency"?

[zloirock]

I'm asking what the use case is

Promise.prototype.finally.call(instanceOfSomeNonNativePromiseWithoutFinally, onFinally);

Available too many different Promise implementations, a serious part of them haven't Promise#finally.

[ljharb]

@zloirock why wouldn't you prefer Promise.resolve(instanceOfSomeNonNativePromiseWithoutFinally).finally(onFinally)? That way you'd get consistent timing semantics, instead of merely passing two spec-created functions to instanceOfSomeNonNativePromiseWithoutFinally's .then method.

[zloirock]

@ljharb OK, another example:

YourNonNativePromise.prototype.finally = Promise.prototype.finally;

instead of the previous example it's real case for me with core-js.

[ljharb]

In an engine where Promise.prototype.finally exists, why would YourNonNativePromise not extends Promise? Why would core-js need to even create YourNonNativePromise in the first place, in that engine?

[zloirock]

In an engine where Promise.prototype.finally exists, why would YourNonNativePromise not extends Promise?

It's not always possible. For example, Promise.prototype.finally can be available from polyfills, but babel can't extend built-ins.

Why would core-js need to even create YourNonNativePromise in the first place, in that engine?

For example, for engines without support unhandled promise rejection tracking or for the version without global namespace pollution.

[ljharb]

I’m still not sure I understand where you would have an engine that a) natively implements finally, b) doesn’t natively implement builtin extension and class, and even c) that wouldn’t already have unhandled promise rejection tracking built in.

In other words, by the time finally is available natively, what parts of Promise would even be left to polyfill? In the case of unhandled rejection handling, you’d have to reimplement finally anyways so you could track unhandled rejections in the promises it creates - there’s just not a polyfill case I’m seeing where you’d ever want the native finally implementation without the rest of the native Promise.

[allenwb]

@ljharb

None of your objections really matter. You are adding a single new built-in method, that is not time or place to be breaking internal consistency or trying to change a design philosophy that has been used to achieve internal consistency within the specification.

This sort of consistency is not just a tradition. It is a reflection of specific design choices made within TC39 in reaction to earlier experience where there was variability among implementations about such checks and observing the problems that caused. Problems, because people actually did things like try to build alternative implementations of build-ins or selectively reuse built-ins in context other then their original use.

Finally, note that JS is not a nominally typed language and JS subclassing is not subtyping. In fact JS does not even have the concept of distinct object types. All object are the same ES primitive type. Dynamic "type checking" in JS (and most other dynamic languages) isn't really type checking at all, it is really safety checking for low level primitive operations. Such checks prevents unsafe primitive operations such as accessing an internal slots that does not exist or trying to use a numeric object as an object reference. Your use of branding in finally does not follow that pattern. It is not being used as a primitive safety check. Instead, it is trying to enforce some higher level concept of object type. It is unnecessarily for safety and in the "normal" case redundant because the same check will be performed by built-in then.

Finally the argument, "if we add the error check now, we can always remove it in the future" doesn't work for actual developers. Individual developers who might run into such a unnecessary check can't wait years for the specification to be changed and for implementations updated. The "we can remove it latter" argument really only applies to internal TC39 considerations, not to actual usage.

[ljharb]

Indeed, I'm trying to apply it to internal TC39 considerations: that I think this consistency has been overly applied, and that I'd rather see us keep the check for now, given that it's a reversible decision, rather than make yet another irreversible decision.

It's worth noting that if it were web-compatible, the committee would have preferred to change catch to do a brand check, and not to observably call .then - the consensus was that this choice for catch was a mistake, and we regret it, but it's too late to fix it. For consistency with catch, finally was changed to observably call .then, but I don't see that as an argument to make this method expand what receivers it can work with from "Promises or subclassed Promises" to "any thenable" - and thus expand how much user code can observe engine-created closures, something folks have already expressed discomfort with.

[ljharb]

At any rate, if members feel strongly that this brand check should be removed, and implementors aren't concerned about the optimizability of wider exposure of ideally unobservable spec-created closures, then I'm sure I'll end up removing this - it's not a hill I want to die on. I do think, however, that more things should be doing brand checks, and I think it's unfortunate if "consistency" is going to forever constrain us to having everything duck type.

[littledan]

cc @gsathya

[ljharb]

(I’ll also add; anything with internal slots is indeed nominally typed; that’s why brand checks continue to exist for all builtin types besides Error; the fact that many methods on a builtin don’t depend on the internal slots doesn’t change that)

[allenwb]

(I’ll also add; anything with internal slots is indeed nominally typed; that’s why brand checks continue to exist for all builtin types besides Error; the fact that many methods on a builtin don’t depend on the internal slots doesn’t change that)

It depends upon your definition of "nominal type". Wikipedia's says

In computer science, a nominal or nominative type system (or name-based type system) is a major class of type system, in which compatibility and equivalence of data types is determined by explicit declarations and/or the name of the types. Nominal systems are used to determine if types are equivalent, as well as if a type is a subtype of another. It contrasts with structural systems, where comparisons are based on the structure of the types in question and do not require explicit declarations.

In ES you can define a class that in response to new:

1. creates an instance of some built-in constructor whose instances have internal slots
2. change the [[Prototype]] of the instance so it doesn't inherit from that built-in but instead inherits from the prototype provided by the class
3. Add additional methods that might or might not reference the internal slots (internal slot referencing methods would have to have been copied from the original built-in).
4. Return that instance as the result of the new operator.

I don't see how, using the Wikipedia definition, you could say that the resulting instance has the same nominal type as the related built-in or is a nominal subtype of the built-in. It probably could be described as having some sort of structural typing relationship with the built-in.

[ljharb]

In summary:

• I'm not seeing anyone besides myself defending the presence of this brand check
• The valid reasons not to have it seem to be:
  1. general philosophy of "only add brand checks where needed"
  2. in the common use case, the brand check will apply anyways
  3. the common API for Promises is deemed to be .then, so that's the only place that should brand check, to allow subclassing/extensibility

Nobody has yet demonstrated a use case where they'd need a native .finally, but need it to work with an alternative Promise implementation, or a non-native/non-subclassed Promise; and subclassing Promises is effectively dead because everybody in the ecosystem uses await and Promise.resolve already.

However, in the interest of moving the proposal along, and because in the 99.99999% case, the .then brand check will apply anyways, I'm going to merge the PR and remove the brand check.

This will be a brief mention in update for the November meeting; if anyone objects to removing the brand check, or thinks it needs further discussion in the committee, please let me know.

[littledan]

Were we discussing whether the PromiseResolve call in the then and catch functions is necessary? I don't see that changed by the patch.

[ljharb]

No, but that’s still necessary since the user’s callback could return any thenable - this is also the first time that’s been questioned; can you file a separate issue about it?