selectAtom not working any more

[joacub]

Summary

selectAtom is returning for async atoms always a promise which cause a hooks to return a throw (promise), we expect that the hook just return the state if the state is not ready then do not return the state and set the sate later.

but now it is imposible to use the hook as this always return an promise like error and never update the state.

[dai-shi]

Please provide a minimal reproduction, preferably with https://csb.jotai.org.
I will close it for now and reopen it once provided.

[dai-shi]

If this is an issue with async behavior, I assume it can be reproduced without selectAtom.
Or, do you think it's selectAtom specific issue?

[joacub]

Is specific for the select atom as this function get a derived atom which is a synchronous always

[dai-shi]

I will close it for now and reopen it once provided.

On second thought, I'm converting this to a discussion, because #1158 is a closed issue.

[dai-shi]

@joacub Can you summarize again what you have found in #1158?

[dai-shi]

I'm double checking the difference between v2.2.0 and v2.1.1.
v2.1.1...v2.2.0

Only the change made in core is #1966.
We assume it's just dev only methods. I hope that they have no bugs.
So, I'm very curious to hear that @joacub says it worked with v2.1.1.

[joacub]

yes let me explain, i already investigate and i saw the issue, there were a really breaking change in the latest version about async behavior. in my next post i will explain you whaat is happening.

[joacub]

When I use selectAtom, I always get an asynchronous atom, which means it returns a Promise-like error every time. To overcome this issue, I created a wrapper function called useAtomAsync to handle the error and internally update the state when it becomes ready. This ensures that the function won't be called again unnecessarily.

const useAtomAsync = (selector) => {
  const [state, setState] = useState({ data: undefined });

  try {
    const result = selector();
    return result;
  } catch (promiseError) {
    promiseError.orig.then((resultPromise) => {
      setState(resultPromise);
    });
  }
  
  return state;
};

Without this wrapper function, selectAtom becomes unusable for me because the hook always returns a Promise-like error. What I expect from selectAtom is to retrieve the current state available in the storage. However, when the selector directly accesses the asynchronous atom, it fetches the state every time the hook is mounted, which is not the desired behavior. Instead, the selector should only return the current state. When the asynchronous or any other function resolves, it will update the atom, triggering the reevaluation of all hooks.

[dai-shi]

Hm, so, it sounds like a selectAtom-specific issue. Does it work differently in v2.1.1?

[joacub]

It appears that there hasn't been any change since the update regarding asynchronous operations, and I suspect that the issue lies in that area. The code remains broken because it can break at any time when the selector returns a promise. However, it seemed to be working before, possibly due to luck.

The problem seems to be related to the changes made in the atomWithStorage function, although the issue was present even before these changes were implemented.

Let me explain the situation. We have a store specification where the store should always return a value, regardless of what that value is. However, when we use the selectAtom function and it returns an error instead of the current state, it clearly goes against the desired behavior of a storage. The selector should always resolve to the current state, irrespective of whether it is an asynchronous storage or not. We want to access the current state, not the asynchronous function that will eventually return the value. It's important to note that React hooks do not allow for asynchronous operations. To address this, we are using an internal state that has its own storage to access unmounted data stored within it. Essentially, the storage is not functioning correctly as the first action always returns nothing.

[dai-shi]

Sounds like you have pretty better understanding of the issue. I'm curious what's the difficulty of reproducing the issue in codesandbox? We want to create a test before tackling.

[joacub]

Sounds like you have pretty better understanding of the issue. I'm curious what's the difficulty of reproducing the issue in codesandbox? We want to create a test before tackling.

I will create a complete reproduction give me a moment

[joacub]

https://codesandbox.io/s/quizzical-haslett-tdqph2?file=/src/useCache.ts

you can see here, this example is not triggering anything, not even the update. so that's maybe the reason is not been a loop.

[dai-shi]

Can you make a minimal reproduction? Or a test case?
I think you can reproduce the issue with only a couple of lines of code.

[joacub]

It appears that the issue I'm encountering doesn't occur when running the code in a browser using Jot.

In the browser environment, React-Dom gracefully handles the error with suspense, ensuring smooth operation. However, in React Native, the situation is different. Consequently, the problem persists and keeps repeating at that specific point.

[dai-shi]

If you can't reproduce the issue with useAtom in the browser environment,
I wonder if you can reproduce the issue with the store API.

Hm, but reading your comment below, you can reproduce it in codesandbox?

[joacub]

You can see in the logs, that the promise error happens at the beginning, in react native when that happen all the hooks gets executed again without pass to the next code block.

every time that get unmount and mount the promise like error happens. That not the desired behavior. Anyways the promise like error shouldn’t happen never. As I said the hook should return the current state always.

[joacub]

This si the issue in the last update you are always doing the atom asynchronous:

jotai/src/vanilla/utils/atomWithStorage.ts

Line 130 in aa9859f

baseAtom.onMount = (setAtom) => {

jotai/src/vanilla/utils/atomWithStorage.ts

Line 131 in aa9859f

const value = storage.getItem(key, initialValue)

this is the old approach and the new one assign the asynchronous get item to the atom

And here you can see that you are telling the atom it is asynchronous when is not or does not have to be.

jotai/src/vanilla/utils/atomWithStorage.ts

Line 157 in f4adf6c

setAtom(storage.getItem(key, initialValue))

[dai-shi]

If I understand it correctly from your description, it's possible to reproduce the behavior with async atom and selectAtom. I don't have a concrete mental model yet, so if you can reproduce it, it'd be very helpful.

[joacub]

you can utilize the atomWithStorage feature available in version 2.2.0. By implementing it, you will consistently encounter a "promise-like" when using with the selectAtom error during each mounting process.

I have made significant cleanup to the codesandbox to present you with a simplified version that showcases my use case. Unfortunately, I am unable to reproduce it any further at the moment due to time constraints. I apologize for not being able to provide more assistance and for not being as helpful as I would have liked.

However, I want to mention that I have managed to resolve my use case on my own without requiring any modifications from your end. Nonetheless, if you would still like to take a look at the lines of code I previously shared, feel free to do so. It should be relatively straightforward to identify the issue and understand its root cause.

take a look in the lines i shared...

[dai-shi]

Sorry, I didn't know you updated the codesandbox. Will have a look.

[joacub]

I didn't, I just telling that that was the less I can get rid of the fat of my application, if a removed more then you can just use your atomWithStorage with the atomSelect and you will get the exactly same result.

I though you want to know my use case, that's why I added some of my context, but just by using the atomWithStorage you will get the promise like error

[dai-shi]

Hm, one misconception was that I thought it's extremely hard to reproduce the issue (because your csb was big). Okay, I will try something by myself. Thanks.

[dai-shi]

selectAtom is returning for async atoms always a promise

I'm trying to reproduce the behavior with atomWithStorage and selectAtom. (If successful, I will eliminate atomWithStorage to create a test.)

So far, here's what I did.
https://codesandbox.io/s/recursing-bose-9smf4k?file=/src/App.tsx
It works nicely. Now, what should be done next?

[joacub]

no, please read carefully my responses, may I been explaining my self wrongly ?? I think the issue is very simple to see.

You are converting with no reason an sync atom to an async atom when this atom never return any promise, is the storage which is asynchronous which have nothing to be with the atom it self, the storage just need to get the values stored and initializing the atom only in the mount and in background you should not set the atom asynchronous, this way it is unusable the atom as always is a async atom, it does not make any sense resolving over and over a promise that just need to resolve once in the mount.

[dai-shi]

So, isn't what you expect this behavior https://codesandbox.io/s/compassionate-bardeen-gddmxc ?
Otherwise, I might be missing something completely, and need someone to help explaining. I don't think it's your fault. I've been struggling with the minimal reproduction.

[joacub]

No that does not the expected behavior.
See the console, the suspense is calling the fallback, because the atom is async.
Let me try to explain it better,:
we have an atom which is not async is an synchronous atom, and we are using a storage to persist the values between reloads.

then we need to use the atomWithSotrage to init the atom with the saved values in the persistent storage when the app or website loads the first time.

we don't want the atom to be async, our atoms are always synchronous but you with your new function are forcing my atoms to be asynchronous, le me share my change to have this fixed in my end:

// import { atom } from '../../vanilla.ts'
// import type { WritableAtom } from '../../vanilla.ts'
// import { RESET } from './constants.ts'

import { type WritableAtom, atom } from 'jotai';
import { RESET } from 'jotai/utils';
import { __DEVELOPMENT__ } from '../shared/global-env';

const isPromiseLike = (x: unknown): x is PromiseLike<unknown> =>
  typeof (x as any)?.then === 'function';

type Unsubscribe = () => void;

type SetStateActionWithReset<Value> =
  | Value
  | typeof RESET
  | ((prev: Value) => Value | typeof RESET);

export interface AsyncStorage<Value> {
  getItem: (key: string, initialValue: Value) => PromiseLike<Value>;
  setItem: (key: string, newValue: Value) => PromiseLike<void>;
  removeItem: (key: string) => PromiseLike<void>;
  subscribe?: (key: string, callback: (value: Value) => void, initialValue: Value) => Unsubscribe;
}

export interface SyncStorage<Value> {
  getItem: (key: string, initialValue: Value) => Value;
  setItem: (key: string, newValue: Value) => void;
  removeItem: (key: string) => void;
  subscribe?: (key: string, callback: (value: Value) => void, initialValue: Value) => Unsubscribe;
}

export interface AsyncStringStorage {
  getItem: (key: string) => PromiseLike<string | null>;
  setItem: (key: string, newValue: string) => PromiseLike<void>;
  removeItem: (key: string) => PromiseLike<void>;
}

export interface SyncStringStorage {
  getItem: (key: string) => string | null;
  setItem: (key: string, newValue: string) => void;
  removeItem: (key: string) => void;
}

export function createJSONStorage<Value>(
  getStringStorage: () => AsyncStringStorage,
): AsyncStorage<Value>;

export function createJSONStorage<Value>(
  getStringStorage: () => SyncStringStorage,
): SyncStorage<Value>;

export function createJSONStorage<Value>(
  getStringStorage: () => AsyncStringStorage | SyncStringStorage | undefined,
): AsyncStorage<Value> | SyncStorage<Value> {
  let lastStr: string | undefined;
  let lastValue: any;
  const storage: AsyncStorage<Value> | SyncStorage<Value> = {
    getItem: (key, initialValue) => {
      const parse = (str: string | null) => {
        str = str || '';
        if (lastStr !== str) {
          try {
            lastValue = JSON.parse(str);
          } catch {
            return initialValue;
          }
          lastStr = str;
        }
        return lastValue;
      };
      const str = getStringStorage()?.getItem(key) ?? null;
      if (isPromiseLike(str)) {
        return str.then(parse);
      }
      return parse(str);
    },
    setItem: (key, newValue) => getStringStorage()?.setItem(key, JSON.stringify(newValue)),
    removeItem: (key) => getStringStorage()?.removeItem(key),
  };
  if (typeof window !== 'undefined' && typeof window.addEventListener === 'function') {
    storage.subscribe = (key, callback, initialValue) => {
      if (!(getStringStorage() instanceof window.Storage)) {
        return () => {};
      }
      const storageEventCallback = (e: StorageEvent) => {
        if (e.storageArea === getStringStorage() && e.key === key) {
          let newValue: Value;
          try {
            newValue = JSON.parse(e.newValue || '');
          } catch {
            newValue = initialValue;
          }
          callback(newValue);
        }
      };
      window.addEventListener('storage', storageEventCallback);
      return () => {
        window.removeEventListener('storage', storageEventCallback);
      };
    };
  }
  return storage;
}

const defaultStorage = createJSONStorage(() =>
  typeof window !== 'undefined' ? window.localStorage : (undefined as unknown as Storage),
);

export function atomWithStorage<Value>(
  key: string,
  initialValue: Value,
  storage: AsyncStorage<Value>,
  unstable_options?: { unstable_getOnInit?: boolean },
): WritableAtom<
  PromiseLike<Value> | Value,
  [SetStateActionWithReset<PromiseLike<Value> | Value>],
  PromiseLike<void>
>;

export function atomWithStorage<Value>(
  key: string,
  initialValue: Value,
  storage?: SyncStorage<Value>,
  unstable_options?: { unstable_getOnInit?: boolean },
): WritableAtom<Value, [SetStateActionWithReset<Value>], void>;

export function atomWithStorage<Value>(
  key: string,
  initialValue: Value,
  storage: SyncStorage<Value> | AsyncStorage<Value> = defaultStorage as SyncStorage<Value>,
  unstable_options?: { unstable_getOnInit?: boolean },
): any {
  const getOnInit = unstable_options?.unstable_getOnInit;
  const baseAtom = atom(getOnInit ? storage.getItem(key, initialValue) : initialValue);

  if (__DEVELOPMENT__) {
    baseAtom.debugPrivate = true;
  }

  baseAtom.onMount = (setAtom) => {
    if (!getOnInit) {
    //   setAtom(storage.getItem(key, initialValue));
      const value = storage.getItem(key, initialValue);
      if (value instanceof Promise) {
        value.then((v) => setAtom(v));
      } else {
        setAtom(value);
      }
    }
    let unsub: Unsubscribe | undefined;
    if (storage.subscribe) {
      unsub = storage.subscribe(key, setAtom, initialValue);
    }
    return unsub;
  };

  const anAtom = atom(
    (get) => get(baseAtom),
    (get, set, update: SetStateActionWithReset<PromiseLike<Value> | Value>) => {
      const nextValue =
        typeof update === 'function'
          ? (
              update as (
                prev: PromiseLike<Value> | Value,
              ) => PromiseLike<Value> | Value | typeof RESET
            )(get(baseAtom))
          : update;
      if (nextValue === RESET) {
        set(baseAtom, initialValue);
        return storage.removeItem(key);
      }
      if (isPromiseLike(nextValue)) {
        return nextValue.then((resolvedValue) => {
          set(baseAtom, resolvedValue);
          return storage.setItem(key, resolvedValue);
        });
      }
      set(baseAtom, nextValue);
      return storage.setItem(key, nextValue);
    },
  );

  return anAtom;
}

[joacub]

check only this:

baseAtom.onMount = (setAtom) => {
    if (!getOnInit) {
    //   setAtom(storage.getItem(key, initialValue));
      const value = storage.getItem(key, initialValue);
      if (value instanceof Promise) {
        value.then((v) => setAtom(v));
      } else {
        setAtom(value);
      }
    }
    let unsub: Unsubscribe | undefined;
    if (storage.subscribe) {
      unsub = storage.subscribe(key, setAtom, initialValue);
    }
    return unsub;
  };

[dai-shi]

So, you want to avoid suspending initially?
The intention of #1958 is make it async atom, so the design choice simply conflicts with your expectation. (It should have been done when we released v2.0.0.)
I feel very sorry about failing to explain the design change clearly in the release note.
I knew this limitation and the solution in my mind is unstable_unwrap.
I think it's a good timing to finalize unwrap and document it.

Hopefully last confirmation: Does this behave as you expect? https://codesandbox.io/s/frosty-dust-7cy7yk?file=/src/App.tsx

(The code is messy, and I think it should be improved. There are some findings, which I will work separately.)

[dai-shi]

There are some findings, which I will work separately

Opened 4 PRs: #1994 #1995 #1996 #1997
I will merge them later.

[dai-shi]

Right. There has been a feature request to suspend before the initial read from the async storage. So, the requirement simply conflicts.

There's room for improvement in persistence guide in docs. I think it should show various use cases and mention atomWithStorage covers some of them, not all of them. In the case atomWithStorage doesn't cover, it's time to create a util in userland.

[joacub]

Yeah make sense, we select jotai for performance issues in recoil. We would love that jotai keep that in mind 🙌

[dai-shi]

off topic: just out of curiosity, did you migrate from recoil when jotai was v1 or v2?

[joacub]

Nope, I migrate just one or two weeks ago, recoil has issues in memory really bad, and in react native is just really nasty the issue. So we decide to use jotai. Is simpler, does just what we want, and light weight. We can also control more jotai library. So we love that.

[joacub]

First version of jotai was 2.1 when the storage was async and you were hydrating in background when promise resolves, that way is really good. Remember that if you set the getItem that always will return an async function which does nothing over and over.

[vhfmag]

Hi! Sorry if I missed anything (the discussion is quite long at this point and seems to have started to look into storage instead), but here's a very minimal reproduction: https://codesandbox.io/p/sandbox/dh56hx. The basic idea is that if selectAtom is passed an atom whose value is a Promise, the selector callback gets a Promise as its argument. Maybe it's by design, but I don't feel it's intentional because:

1. selectAtom used to receive the Promise's result instead, but that changed at some point between 2.4.2 and 2.8.0:
2. The types tell us we get the Promise's result, which you can verify in the reproduction link, as per this screenshot:
3. The change also affects equalityFn, which makes it useless — I can't make the function return a Promise, which means I can't access the underlying value and so the best I can do is Object.is:
   

[vhfmag]

Either way, the function's type is still incorrect, given it leads consumers to think they have access to the Promise value instead of the Promise itself

[vhfmag]

Oh, I see unwrap exposes a fallback value if the Promise isn't yet resolved, which defeats the purpose in my case because I'm trying to derive atoms that should suspend. Seems like I'll either need custom solutions (e.g. selectAtom(atom, async input => selector(await input)) — which still doesn't solve the equalityFn bit and hence makes selectAtom useless since that's its only difference with a regular derived atom), or I'll have to migrate to derived atoms instead

[dai-shi]

I'll have to migrate to derived atoms instead

That sounds good. As you said, we can't simply compare two promises with equalityFn.

[dai-shi]

need custom solutions

That's an option I meant in #2435 (review)

We may consider supporting a new util for promise-capable selectAtom alternative in jotaijs/*.

[vhfmag]

That would be a nice alternative, yeah.

All in all, I'm very grateful of the hard work you and other collaborators have put into jotai, it's an extremely useful and reliable library I (and my company) use daily, but I must say my confidence on the library's stability took a bit of a hit with such a breaking change (as clearly marked in the commit's message and PR title) being introduced at a minor release. I'll make sure to read release notes of all versions before upgrading moving forward.

Either way, again, thanks for this amazing library