Atom effect onSet() handler provides DefaultValue instead of new value when triggered by async default promise resolving.

[FraserHamilton]

We use atom effects to sync up our recoil state with the local storage in our react native application. As of recoil 0.3.0 there seems to have been an undocumented change to behavior of the atom effects.

When our application first loads we retrieve our state from recoil using the following selector.

export const localRecipeVersion = selectorFamily({
  key: "localrecipeversion",
  get: (recipeId) => ({ get }) => {

    const titleAtom = get(titleAtomFamily(recipeId));
    const descriptionAtom = get(descriptionAtomFamily(recipeId));
    const notesAtom = get(notesAtomFamily(recipeId));
    const tagsAtom = get(tagsAtomFamily(recipeId));
    const toolsAtom = get(toolsAtomFamily(recipeId));
    const difficultyAtom = get(difficultyAtomFamily(recipeId));
    const dishtypeAtom = get(dishAtomFamily(recipeId));
    const durationAtom = get(durationAtomFamily(recipeId));
    const servingsAtom = get(servingsAtomFamily(recipeId));
    const ingredientsAtom = get(ingredientsAtomFamily(recipeId));
    const imagesAtom = get(imagesAtomFamily(recipeId));
    const instructionsAtom = get(instructionsAtomFamily(recipeId));

    return {
      __typename: "recipe",
      title: titleAtom,
      description: descriptionAtom,
      difficulty: difficultyAtom,
      dishtype: dishtypeAtom,
      duration: durationAtom,
      images: imagesAtom,
      instructions: instructionsAtom,
      notes: notesAtom,
      recipe_ingredients: ingredientsAtom,
      recipe_tags: tagsAtom,
      recipe_tools: toolsAtom,
      servings: servingsAtom,
    };
  },
});

In recoil 0.3.0 using this selector also triggers the onSet part of our effects for these atomFamilies. See the code for the imagesAtomFamily and it's associated effect below.

export const imagesAtomFamily = atomFamily({
  key: "imagesatomfamily",
  default: async (param) =>
    defaultAsyncValue(param, "images", recipeData.images),
  effects_UNSTABLE: (recipeId) => [
    localImagesStorageEffect(`recipe_${recipeId}_images`),
  ],
});

export const localImagesStorageEffect = (key) => async ({ setSelf, onSet }) => {
  const savedValue = await AsyncStorage.getItem(key);

  if (savedValue != null) {
    setSelf(JSON.parse(savedValue));
  }

  onSet(async (newValue) => {
    if (newValue.length == 0) {
      await AsyncStorage.removeItem(key);
    } else {
      console.log("newValue", newValue);
      await AsyncStorage.setItem(key, JSON.stringify(newValue));
    }
  });
};

When the onSet part of the effect is triggered it passes an empty DefaultValue object, what's even more odd is that this DefaultValue is not the same as the default we pass to the atomFamily, recipeData.images above is an empty array.

[drarmstr]

There were a couple fixes mentioned in the 0.3.0 release notes to onSet() so it better matches the documented behaviour. onSet() should not be called based on sets that were initiated from the same atom effect. It sounds like you're reporting a regression where onSet() is being called incorrectly. Is this based on the setSelf() call or something else? Do you have a cut-down reproducer?

[FraserHamilton]

Upon further investigation it appears to be something to do with the way we were handling our default values. Here is a cut down reproducer of how we handle it currently.

import React from "react";
import { useRecoilValue } from "recoil";
import { atomFamily, selector } from "recoil";

const wait = (ms) => {
  return new Promise((resolve, reject) => setTimeout(resolve, ms));
};

const defaultAsyncValue = async (param, type, defaultValue) => {
  // this function usually checks if we have a value in local storage which is an async operations
  // if we have no value it provides a default
  await wait(2000);
  return defaultValue;
};

const testEffect = () => async ({ setSelf, onSet }) => {
  onSet(async (newValue) => {
    console.log("newValue", newValue);
  });
};

const testAtomFamily = atomFamily({
  key: "testatomfamily",
  default: (param) =>
    selector({
      key: "testatomfamily/Default",
      get: () => defaultAsyncValue(param, "test", []),
    }),
  effects_UNSTABLE: (recipeId) => [testEffect()],
});

export function TestComponent() {
  const testAtom = useRecoilValue(testAtomFamily(0));

  return <></>;
}

It seems that in 0.3.0 recoil doesn't wait for the DefaultAsyncValue to resolve and instead provides it's own default value an empty object. I'm unsure if this intended behavior, I did however find an example in the docs that uses a selector to provide a default value and this seems to resolve the issue, see below.

const testAtomFamily = atomFamily({
  key: "testatomfamily",
  default: (param) =>
    selector({
      key: "testatomfamily/Default",
      get: () => defaultAsyncValue(param, "test", []),
    }),
  effects_UNSTABLE: (recipeId) => [testEffect()],
});

Would you like me to update the title to better reflect the issue?

[drarmstr]

Note that using a selector() in the default for an atomFamily() can lead to duplicate selector/keys if you don't memoize. Easy fix is to use selectorFamily() instead:

const testAtomFamily = atomFamily({
  key: "testatomfamily",
  default: selectorFamily({
    key: "testatomfamily/Default",
    get: (param) => () => defaultAsyncValue(param, "test", "TEST")
  }),
  effects_UNSTABLE: (recipeId) => [testEffect()],
});

For this case the onSet() handler isn't actually called. That's because technically the atom value isn't changing or being set, just the fallback default selector value resolves. The selector can arbitrarily change values if it has any dependencies change. I'm thinking that's the appropriate behavior for this so far, but it's pretty debatable.. The atom wiring isn't quite setup to easily catch those state changes for effects.

I put in your test case reproducer here https://codesandbox.io/s/cool-forest-v2yyl?file=/src/App.js
with the default Promise:

const testAtomFamily = atomFamily({
  key: "testatomfamily",
  default: (param) => defaultAsyncValue(param, "test", "TEST"),
  effects_UNSTABLE: (recipeId) => [testEffect()],
});

For this case we expect the onSet() handler to be called because the value was set external to the atom effect (i.e. it was not set via setSelf() in the same hook). It currently gives the value DefaultValue as you point out. It does this in some situations with async values or error states since the handler parameter types currently can't handle those states. But, seems like in this case it should be fine to provide the proper new value. Fixed with #1059

[drarmstr]

Duplicate of #738

[drarmstr]

Addressed with #1059