Why the overly specific test?

[moll]

Hey,

I see a commit message for the stricter test, but it's not clear why's that extra processing good. Perhaps you can share some light. Thanks!

[ForbesLindesay]

Because it would be possible to add something like:

String.prototype.then = function () {
  throw new Error('oops');
};

This would be totally valid, and would make all strings have a .then method, but it would not make strings into promises. A value should only be treated as a promise if it is an object or a function and has a .then method. Hence the more specific test.

[moll]

Wouldn't you say that's an inherent problem of duck typing? Is there any advantage trying to protect the string special case when the same person could just as well think adding then to Object.prototype, Function.prototype or best of yet, to Date.prototype, is a good idea?

It's not that I oppose strictness, it's just that protecting against prototype modification seems premature and YAGNI. Unless it's actually faster to do the typeof checks before property access.

[ForbesLindesay]

The check in this library matches the check in the Promises/A+ Resolution Procedure which first checks that it is an object or function (and not null), then checks that it has a .then method.

It's always possible to screw up duck typing, but the goal is to be as specific as possible while not excluding anything that should be valid.

[moll]

Okay, fair enough. Thank you for the explanation!

[mikew]

Curious how this helps when it only guards:

• String
• Boolean
• Symbol
• Number
• null / undefined (which are already guarded against with !!)

but still allows:

• Date
• RegExp
• Array
• Int8Array
• Uint8Array
• Uint8ClampedArray
• Int16Array
• Uint16Array
• Int32Array
• Uint32Array
• Float32Array
• Float64Array
• BigInt64Array
• BigUint64Array
• Map
• Set
• WeakMap
• WeakSet
• ArrayBuffer
• SharedArrayBuffer
• DataTransfer
• Proxy

and probably more.

[ForbesLindesay]

According to the spec, all of those are potentially valid "thenables". For example, the following example prints 42:

const x = new Map();
x.then = (fn) => fn(42);

Promise.resolve().then(v => x).then(console.log)

but:

const x = 'hello world';
String.prototype.then = (fn) => fn(42);

Promise.resolve().then(v => x).then(console.log)

prints "hello world" because x is not considered to be "thenable"