Recoil: Async selector set function

[Khachatour]

Hello dear Recoil team! Using Recoil for the production application and loving it so far. Just one specific clarification.

In turns out that I can't have a async selectorFamily set function, example below

const filtersSelector = selectorFamily({
  key: 'filtersSelector',
  get: (filterField: string) => ({ get }) => get(filtersState)[filterField],
  set: (filterField: string) => async ({ set, get }, newVal) => {
    const filterTerms = {
      ...get(filtersState),
      [filterField]: newVal,
    } as FilterTypes;

    const setter = (prevValue: FilterTypes) =>
      ({ ...prevValue, [filterField]: newVal } as FilterTypes);

    set(filtersState, setter);

    const data = await getPhoneNumbers(filterTerms);

    set(phoneNumbersState, data);
  },
});

This code snippet works this way, the filtersState set function works and sets new filter terms, after that the getPhoneNumbers request works (I can see it in the network tab) but the last phoneNumbersState set function does not change the state even though if I write

  set(phoneNumbersState, updateFunc);

the updateFunc gets invoked but I can't see state updates after that

Thanks in advance!

[drarmstr]

Selectors and selector families do not currently allow async set() methods. Flow or TypeScript should give an error that you are trying to return a Promise from a void function..

The current mechanism to support async sets is to just use the setter from useRecoilState() or useSetRecoilState() asynchronously. Or, for transactions with multiple atoms or abstractions you can use useRecoilCallback() to asynchronously set atoms. E.g.:

function useFilters(filterField: string) {
  const [filters, setFilters] = useRecoilState(filtersState);
  const setPhoneNumbers = useSetRecoilState(phoneNumbersState);
  const setFilter = useCallback(newValue => {
    setFilters(prevFilters => {
      const filterTerms = {...prevFilters, [filterField]: newValue});
      setFilters(filterTerms);
      const data = await getPhoneNumbers(filterTerms);
      setPhoneNumbers(data);
    });
  }, [filters]);

  return [
    filters[filterField],
    setFilter,
  ];
}

However, note that with this approach you may have some inconsistent state because you are setting the filterState atom first, then later setting the phoneNumbersState, so there is a gap they will not match. Also, if someone else sets the filter to a new value while getPhoneNumbers() is still pending, then it could be set to stale values of the filters depending on the relative ordering they complete. To help ensure consistency, you may want to just use derived state for phone numbers:

const filtersSelector = selectorFamily({
  key: 'Filter',
  get: field => ({get}) => get(filtersState)[field],
  set: field => ({set}, newFilter) =>
    set(filtersState, prevFilters => ({...prevFilters, [field]: newFilter instanceof DefaultValue ? undefined, newFilter})),
});

const phoneNumbers = selector({
  key: 'PhoneNumbers',
  get: ({get}) => getPhoneNumbers(get(filtersState)),
});

Anyway, one motivation for not allowing async selector sets was to help discourage these timing issues and help ensure that the set takes effect and can be observed by subsequent sets or actions using the updater form to get the "current" value. Though, this seems to be a common request... So, I'll leave this issue open as a feature request for further team discussion as we prepare the next API refresh.

[Khachatour]

This approach solved the issue thanks!

I had a misunderstanding on how selectors work, taking from this example, the get function always re-evaluates even if the given input is the same right? Because I had this issue also. Am I understanding this correclty?

[drarmstr]

@Khachatour - the get() should only re-evaluate if the family parameter changes, a dependency is updated, or some cache is invalidated, as discussed in #747. But, let's keep this issue focused on the async set() and follow-up discussion of get() evaluations in #747.

[drarmstr]

This issue keeps coming up with users attempting async selector sets and either not having or ignoring static type-checking errors. So, while we are considering if we should add support for this, add dynamic error reporting by throwing errors if async sets are attempted in #777

[drarmstr]

From @jjalonso:

Will async sets on selector will never be implemented?
its quite unfair that it is for gets only, some people like me need to derivate data using promise, one example is this.

Imagine I have two kind of events that im subscribed on externally, channelAdded, channelRemoved, on channelAdded and channelRemoved i get as argument a channel instance from an 3rd party lib, and need to call some methods that return promises, like getMembers() etc, in order to serialize,

Then we endup having a bidirectional selector get/set where the get return alls channels and the set have a switch case on the argument to evaluate value.type 'ADDED' or 'REMOVED' in order to do something with value.payload but just the added need to serialize. Ideally we should need to have the derivate data logic inside the setter and not doing out previously the set call,

currently with the current implementation that doesnt support async set, we need to serialise out and send to the set the channel serialize in case of added and the channel without serialize in case of remove.

It is still under consideration to support async set() or not. One motivation to not support it is to ensure a consistent abstraction of atoms/selectors and guarantee that the effect of calling a setter on any atom or selector can be immediately observed in any subsequent sets which use the updater form to get the "current" value. e.g.:

setMyAtom(42);
setMyAtom(currentValue => {
  expect(currentValue).toBe(42);
  return 101;
});

We'd also have to consider the consistency for getting values in an async set and which state versions that should use.

Most cases using an async set can be handled by making abstractions just using hooks or using useRecoilCallback() for advanced use-cases. For example:

function useStateWithAsyncSet() {
  const [value, setValue] = useRecoilState(myAsyncSelector);
  return [
    value,
    async (newValue) => {
      await for something...
      setValue(newValue);
    },
  ];
}

Another feature we are considering is the ability to set an atom to an async Promise value (#237). With that approach, however, the atom would immediately enter the pending state for Suspense until the promise resolves. If a Selector set has some deferred async call to set(), then the upstream atoms would maintain their previous value until set.

Anyway, this is still under consideration.

[drarmstr]

See #780 for another argument to support async set.

[drarmstr]

Further thoughts on the semantics for an async selector set. We need to consider when various upstream sets take effect and the consistency of various interleaved get and set actions. For example, one might attempt to read an atom, then await on something, then set another upstream atom. But, that first atom may have changed value in the meantime and it may be problematic to now set the upstream value based on a stale value of the previous get. It likely makes sense to consider async set operations as atomic transactions.

One possible set of semantics might be to use Suspense-style semantics similar to the get method:

• Upstream set() actions take effect atomically upon the resolution of the Promise returned by the async selector set.
  
  
  
  • Upstream set() actions issued by other callbacks after the resolution of the primary Promise may then be ignored or throw an error?
  
  
• If a get() action references a dependency in an error state, then set will fail and returned Promise will be rejected.
• If a get() action references a dependency in a pending state, then the selector set method will be restarted when the dependency is available. Previous set() actions based on potentially stale state are ignored.
• Async selector set method should return a Promise<void>.

Also worth consideration is handling async transactions via selector abstraction vs something like a Recoil transaction hook. Note that the current useRecoilCallback() hook is not appropriate for async transactions since it doesn't batch updates across awaits. Similarly, useGotoRecoilSnapshot() of a Snapshot#asyncMap() isn't always appropriate for async transactions, it will apply updates atomically but unrelated state may have mutated in the meantime which would be overwritten and the gets during the map may also be referencing stale state. So, there are compelling arguments for a Recoil hook to support transactions.

[otakustay]

As a state normalization zealot, we very much want an async selector to load a remote data and to pass them forward to atoms like:

const users = atomFamily({key: 'user'});
const list = selectorFamily({
    key: 'users',
    // Want a set function here
    get: ({set}) => async params => {
        const users = await fetchRemoteUsesrs(params);
        for (const user of users) {
            set(users(user.id), user);
        }
        // Normalize it to a list of keys
        return users.map(u => u.id);
    },
});

This requires features similar to this issue, currently we do it in a recoil callback way without suspense support:

const users = atomFamily({key: 'user'});
const list = atomFamily({key: users});
const useListUsers = () => useRecoilCallback(
    ({snapshot, set}) => async params => {
        const current = snapshot.valueMayBe(users(params));

        if (current) {
            return current;
        }

        const users = await fetchRemoteUsesrs(params);
        for (const user of users) {
            set(users(user.id), user);
        }
        // Normalize it to a list of keys
        return users.map(u => u.id);
    },
    []
);

This still raise 2 questions to us:

1. Is the use of snapshot safe in this way? Does it cause some race condition in edge cases?
2. How can we cooperate this pattern to suspense's promise throwing pattern?

Still, is selector and selectorFamily can support this natively, recoil could be a best practice in async state management.

[BenjaBobs]

I've not run into issues using a snapshot to set values like that. We store each snapshot and expose a getter and a setter globally, since we have only one recoil root, which allows us to get/set from anywhere.

import React from 'react';
import { Loadable, RecoilState, RecoilValue, useRecoilCallback } from 'recoil';

export let RecoilGetLoadable: <T>(
  recoilValue: RecoilValue<T>
) => Loadable<T> = null as any;

export let RecoilSet: <T>(
  recoilVal: RecoilState<T>,
  valOrUpdater: ((currVal: T) => T) | T
) => void = null as any;

export function RecoilUtilsComponent() {
  // We need to update the RecoilGetLoadable every time there's a new snapshot
  // Otherwise we will load old values from when the component was mounted
  useRecoilTransactionObserver_UNSTABLE(({ snapshot }) => {
    RecoilGetLoadable = snapshot.getLoadable;
  });

  // We only need to assign RecoilSet once because it's not temporaly dependent like get is
  useRecoilCallback(({ set }) => {
    RecoilSet = set;

    return async () => {};
  })();

  return <></>;
}