instanceof Promise in loadable utility not reliable in all environments

[joseph-hanvy-sophos]

Recently faced an issue using jotai in a project that used some kind of es6.promise shim/polyfill to provide backwards compatibility for old browser.

It turns out the instanceof Promise here:

const isPromise = <Value>(
  x: unknown,
): x is Promise<Awaited<Value>> & PromiseMeta<Value> => x instanceof Promise

https://github.com/pmndrs/jotai/blob/v2.8.0/src/vanilla/utils/loadable.ts#L13

It seems it isn't a super reliable way to check - I replaced it with typeof x.then === "function" instead, but I'm not sure that's the best way to check for a promise either.

There's an interesting discussion here:
https://stackoverflow.com/a/28133130/1305699

I notice that isPromiseLike is used here, which works fine:

jotai/src/react/useAtomValue.ts

Line 10 in 9ceaef6

const isPromiseLike = (x: unknown): x is PromiseLike<unknown> =>

It might be best to use that everywhere instead of instanceof Promise, e.g.

jotai/src/vanilla/utils/atomWithObservable.ts

Line 166 in 9ceaef6

if (result instanceof Promise) {

jotai/src/vanilla/utils/unwrap.ts

Line 18 in 9ceaef6

x instanceof Promise

jotai/src/vanilla/store.ts

Line 94 in 9ceaef6

!!a && 'v' in a && a.v instanceof Promise

jotai/src/vanilla/utils/atomWithObservable.ts

Line 166 in 9ceaef6

if (result instanceof Promise) {

jotai/src/vanilla/utils/atomWithStorage.ts

Line 236 in 9ceaef6

if (nextValue instanceof Promise) {

I doubt this is a problem faced by many but it should be an easy fix and it was a real pain to work out what was wrong!

[dai-shi]

Recently faced an issue using jotai in a project that used some kind of es6.promise shim/polyfill to provide backwards compatibility for old browser.

So, in your environment, new Promise() instanceof Promise is false? That's unexpected. I use PromiseLike for api facing cases, but not everywhere. Did you experience the unexpected behavior with loadable, right? Maybe, loadable and unwrap should use PromiseLike.

[joseph-hanvy-sophos]

Interestingly new Promise() isntanceof Promise returns true, so it seems explicit promises 'pass', however in most use-cases it's the result of an async function so I checked (async () => "hi")() instanceof Promise, and that returns false in the environment (expect true and see that in just a new chrome window console)

We experienced the loadable atom never entering the loadable state, though it was version 2.5 so it was falling into this if check and immediately going to hasData:

        if (!(value instanceof Promise)) {
          return { state: 'hasData', data: value } as Loadable<Value>
        }

I'm upgrading to see if the same happens in later versions though, the code has changed but I assume it'll fail in the same way here:
https://github.com/pmndrs/jotai/blob/v2.8.0/src/vanilla/utils/loadable.ts#L45

[dai-shi]

Do you know any way to experiment the behavior in node or latest chrome?

(async () => "hi")() instanceof Promise being false is understandable.

[joseph-hanvy-sophos]

Latest chrome just using the console window works as I'd expect, with it returning true:

[joseph-hanvy-sophos]

Hmm, actually I'm not sure this is an issue, I'm having trouble recreating it stepping into that if condition

[joseph-hanvy-sophos]

Ah, it was an issue we found in our custom version of loadable that we created for setters that had a very similar structure, but used the result of set instead of get.

I'd found the bug there, and noticed our loading wasn't working and assumed the root cause was the same. It seems get returns something that does fulfil instanceof Promise, unlike set.