Unpromise support for Thenable objects. aka: Jest makes some promises not a promise

[davidfiala]

When using Unpromise with jest I noticed that some node builtins like Passthrough and possibly other Readables which expose an async iterator do not return objects that pass instanceof Promise. I cannot at this time explain why, but it's a unique phenomena inside of jest. They do seem to be Thenable though and work with native Promise.race.

Unpromise has an explicit check for instanceof Promise which causes these Thenables to be passed to the new Promise constructor which then throws an error.

https://github.com/cefn/watchable/blob/ca7f84e5fbbddd6c79e3fe35d797ace24358f98b/packages/unpromise/src/unpromise.ts#L83

My weak understanding would be that it might be possible for Unpromise to support Thenables more generically than just taking a Promise or testing against instanceof Promise.

Unfortunately, whatever conditions are causing Jest to make a Passthrough async iterator in to "not a promise" does not affect the typescript typing. So the exception occurs at runtime, and can be difficult to track down.

I could and probably should report this with jest, but regardless of outcome it may be advantageous for Unpromise to support Thenables in general since the same memory leaks it aims to fix ought to be present with Thenables as well as promises.

I've attached a minimal PoC that you can use to see the behavior different between running npm start vs npm test: badjest.zip

And for keyword SEO, this is the error one might see at runtime if you hit this issue: TypeError: Promise resolver [object Promise] is not a function

[cefn]

I agree it makes sense to modify the runtime behaviour to try and fit around mainstream patterns.

If it's not a compile-time error and the inverted check is likely to be more robust (e.g. check if typeof arg === "function" to differentiate a Promise from an executor instead, so it falls back to being interpreted as a Promise) then we could go ahead with that.

What do you think?

[davidfiala]

This looks good, and I tested it works to solve the use case described above. Thank you for such a fast response :) I'd very much appreciate a version bump on NPM for this.

This exercise got me thinking about a couple other FR's we could consider separately:

1) Typescript: Typing to detect Thenables that aren't a Promise<T> type. 2) JS compatibility: Regular objects/scalars as acceptable input, too. For example, this is valid: await Promise.race(['hello', 123])

[cefn]

I released as a patch since it was basically a bug.

Items can confirm to Promise<T> structurally without being instanceof Promise (tied to the class constructor which was my error).

I'm not sure I can extend support for structural interfaces not defined in the Typescript core. Interfaces that ARE defined there, Promise<T> has then() AND catch(). PromiseLike<T> has just then().

• https://github.com/microsoft/TypeScript/blob/98f2c120d8739ae8120b4aa5fc72a0d35a65990d/src/lib/es5.d.ts#L1525
• https://github.com/microsoft/TypeScript/blob/98f2c120d8739ae8120b4aa5fc72a0d35a65990d/src/lib/es2018.promise.d.ts#L4
• https://github.com/microsoft/TypeScript/blob/98f2c120d8739ae8120b4aa5fc72a0d35a65990d/src/lib/es5.d.ts#L1512

For alignment with Promise ProxyPromise has all the methods - then(), catch(), finally().

Not sure how I should handle catch() or finally() when the backing PromiseLike doesn't have any support for them.

Could be (effectively) a no-op I guess, but I wonder if it's easier to just limit support to things which are duck-typed as a full Promise at least.

[cefn]

To be clear about the no-op behaviour, if we went ahead and supported PromiseLike<T> we would just need a guard at https://github.com/cefn/watchable/blob/b9b71c26fe27fc36fa6396d10ec0079a14f79911/packages/unpromise/src/unpromise.ts#L104 to avoid calling catch when it's not present on the underlying object.

The methods catch() and finally() would still be inevitably defined on Unpromises (since they are backed by full-blown promises) but catch() and the catch-behaviour-of-finally would never be directly triggered by the originating thenable, because there was never any catch() defined in the first place.

[davidfiala]

I'm learning too, so forgive me if I make an obvious mistake.

It appears Promise.race, etc check if the first argument typeof thing == "object" && then in thing && typeof thing.then === "function". At least from my testing.

As for catching, would it be to see if our original then got an onrejected, then we run their catch? And finally is equally just a run?

I also wonder about how we ensure that we run their callbacks at the right point in the event loop depending on the type of the thing we passed to race. (immediate callback, setimmediate, settimeout, or via a .then() or new async, something else)?

Another brainstorming thought: Can we have 2 types of proxies? PromiseProxy and PromiseLikeProxy ?

[cefn]

I just shipped 1.0.0 with static method signatures for resolve() race() and any() that align with the core Promise ones (including for synchronous values). By all means PR some tests (passing or failing) for scenarios you expect to compile at build time or to work at runtime for compatibility with core Promises.

[cefn]

Unpromise tests are at https://github.com/cefn/watchable/tree/8e2cf60bfa79321d2060c63905f9e8f7970b1ddd/packages/unpromise/test

[davidfiala]

Hi Cefn, thanks for looking in to this again.

I haven't dived in yet, but a quick FYI that 1.0.0 adds a simpler regression:

const a = Promise.resolve('foo');
const b = Promise.resolve(123);
const c = Unpromise.race([a, b]);

Argument of type '(Promise<string> | Promise<number>)[]' is not assignable to parameter of type 'Iterable<string | PromiseLike<string>>'.
  The types returned by '[Symbol.iterator]().next(...)' are incompatible between these types.
    Type 'IteratorResult<Promise<string> | Promise<number>, any>' is not assignable to type 'IteratorResult<string | PromiseLike<string>, any>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Type 'IteratorYieldResult<Promise<string> | Promise<number>>' is not assignable to type 'IteratorResult<string | PromiseLike<string>, any>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
        Type 'IteratorYieldResult<Promise<string> | Promise<number>>' is not assignable to type 'IteratorYieldResult<string | PromiseLike<string>>' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
          Type 'Promise<string> | Promise<number>' is not assignable to type 'string | PromiseLike<string>'.
            Type 'Promise<number>' is not assignable to type 'string | PromiseLike<string>'.
              Type 'Promise<number>' is not assignable to type 'PromiseLike<string>'.
                Types of property 'then' are incompatible.
                  Type '<TResult1 = number, TResult2 = never>(onfulfilled?: ((value: number) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<...>) | null | undefined) => Promise<...>' is not assignable to type '<TResult1 = string, TResult2 = never>(onfulfilled?: ((value: string) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<...>) | null | undefined) => PromiseLike<...>'.
                    Types of parameters 'onfulfilled' and 'onfulfilled' are incompatible.
                      Types of parameters 'value' and 'value' are incompatible.
                        Type 'number' is not assignable to type 'string'.ts(2345)

typescript: 5.4.4

[cefn]

I had feared that would be the case. The typescript signatures had both array and iterable function overloads, but I convinced myself that it was a matter of fact that Array<T> extended Iterable<T> so that the array signature may have been a holdover from pre-iterable days. Looks like both overload signatures are still needed.

[cefn]

Released @watchable/unpromise@1.0.1 to fix the issue? Is that working better?

[davidfiala]

Cefn, thank you for fixing this! Functionally it does all seem to work now. :)