[Feature request] Mocking Recoil state for testing

[BenjaBobs]

Proposal

I would like to be able to set the cached value of a selector so that I can setup unit testing scenarios for selectors that depend upon other selectors.

Crude example of selector setup:

const numbers = selector<number[]>({
    key: 'numbers',
    get: async () => {
        // some external api
        return await fetchNumbers();
    },
})

const average = selector<number>({
    key: 'average',
    get: ({ get }) => {
        const allNumbers = get(numbers);
        const avg = get(numbers).reduce((acc, next) => acc + next, 0) / allNumbers.length;

        return avg;
    },
})

When testing:

test('Test of average', () => {
    const snapshot = snapshot_UNSTABLE(state => {
        // compile error, because numbers is readonly, which makes sense
        state.set(numbers, [10,20]);

        // this is what I would like so I don't have to mock fetchNumbers()
        state.setCache(numbers, [10, 20]); // sets the cached value of numbers without triggering the getter
    });

    expect(snapshot.getLoadable(average).getValue()).toEqual(15);
});

Current workaround

If there are no other dependencies in the first selector, it can be converted to an atom, which we can set:

const numbers = atom({
    key: 'numbers',
    default: selector<number[]>({
        key: 'numbers_impl',
        get: async () => {
            // some external api
            return await fetchNumbers();
        },
    })
})

However if the first selector also has dependencies, we cannot convert it to an atom.

[drarmstr]

Hmm, well selectors are derived state and we've had concerns about anything trying to explicitly "set" a selector itself as that could lead to consistency problem between the selector and other downstream and upstream selectors/atoms in the data flow graph. This is one of the reasons why we don't have "selector effects".

I empathize with the testing use-case here, but really worry about consistency issues for other workflows. It does seem like a mock is what you're looking for..

[BenjaBobs]

Yeah, I can see how you could wind up with state that's out of sync, so the action would be dangerous I suppose, not unlike mutable state. I actually found a pattern that is not too much boilerplate, and works. Note that I would (if possible) prefer a way to do this with the recoil api where it doesn't involve re-writing my selectors to achieve isolated testability.

// a recoil state setup
const fetch_number = selector<number>({
  key: "fetch_number",
  get: () => {
    return 5; // imagine this was fetched via some http request
  }
});

const magic_number = selector<number>({
  key: "magic_number",
  get: ({ get }) => {
    const number = get(fetch_number);
    // imagine here some data processing that justifies the existense of this selector
    return number;
  }
});

Now what we want to do is test magic_number in isolation.

// first we define a way to get a falsy value given DefaultValue
// this is important, because sometimes falsy values can still be valid
// and we need a way to distinguish between a valid falsy value, and no value at all.
function ifNotDefault<T>(data: T | typeof DefaultValue) {
  return data === DefaultValue ? null : (data as T);
}

// this is a utility type that given a recoil value produces a type that is the wrapped value or DefaultValue
type RecoilValueMock<Type> = Type extends RecoilValue<infer X> ? X | typeof DefaultValue : never;

// And this is our mocked value, which defaults to DefaultValue, such that ifNotDefault will
// return a falsy value if it has not been changed
const fetch_number_mock = atom<RecoilValueMock<typeof fetch_number>>({
  key: "fetch_number_mock",
  default: DefaultValue
});

This allows us use the following setup:

const magic_number = selector<number>({
  key: "magic_number",
  get: ({ get }) => {
-    const number = get(fetch_number);
+    const number = ifNotDefault(get(fetch_number_mock)) ?? get(fetch_number);
    // imagine here some data processing that justifies the existense of this selector
    return number;
  }
});

Which will result in the mock being used instead of the real implementation. Working example: https://codesandbox.io/s/stupefied-lamarr-veudv?file=/src/App.tsx It's a bit of a hassle though.

What are your thoughts on the subject? Do you have anything in the workings that would allow easier isolated tests?

[drarmstr]

My first thought would be if you could just mock the async fetch call and leave all of the selectors as-is.. Not sure on your infra, though.

What do you think about this brainstorm:

Instead of using a selector for the async request use an atom with the default value set to a promise for the async request. This should work the same as the selector case in normal use (except it is RW instead of RO). Then, add a mockEffect() to effects_UNSTABLE in the atom options. This effect could detect your testing infra and initialize the atom to a mock value instead, as appropriate.

const numberQuery = atom({
  key: 'NumberQuery',
  default: fetchNumber(),
  effects_UNSTABLE: [mockEffect],
});

function mockEffect({node, setSelf}) {
  if (MOCKED_VALUES.has(node.key) {
    setSelf(MOCKED_VALUES.get(node.key);
  }
}

[BenjaBobs]

Well it solves the problem for selector chains of length <1, however if we have a selector chain that's longer than that, we run into the issue again. In my case, these selector chains are common, because we do heavy calculations, but some of the steps are cacheable, and we leverage that using multiple selectors. Example of problem:

const initialNumber = atom({
    key: 'initialNumber ',
    default: 5,
});

const multipliedBy2 = selector({
    key: 'multipliedBy2',
    get: ({ get }) => get(initialNumber) * 2,
});

const add10 = selector({
    key: 'add10',
    get: ({ get }) => get(multipliedBy2) + 10,
});

const divideBy4 = selector({
    key: 'divideBy4',
    get: ({ get }) => get(add10) / 4,
});

In this case, if I want to test divideBy4 in isolation, I'd have to (in my head) reverse all the selectors up to initialNumber, such that I can set a proper value for initialNumber in order to get the right input for divideBy4. Since each selector can depend on arbitrary many other recoil states, more complex cases get harder to test.

We can do

const add10_mock = atom({
    key: 'add10_mock',
    default: DefaultValue,
});

const add10 = selector({
    key: 'add10',
    get: ({ get }) => {
        const mockedValue = get(add10_mock);
        if (mockedValue !== DefaultValue) return mockedValue;

        return get(multipliedBy2) + 10;
    },
});

But we'd have to do this for every selector we want to mock. Having a snapshot.dangerously_SetCache(add10, <mock value>) or snapshot.testingly_Mock(add10, <mock value>) would solve this. 🙂

[drarmstr]

Well, you could reverse your stubs. So:

const add10 = atom({
  key: 'add10',
  default: selector({
    key: 'add10/internal`,
    get: ({get}) => get(multipliedBy2) + 10,
  }),
  effects_UNSTABLE: [mockEffect],
});

And that pattern could be provided in a helper something like:

function mockedSelector<T>(opts): RecoilValueReadOnly<T> {
  return readOnlySelector(atom({
    key: opts.key,
    default: selector({...opts, key: opts.key+'/internal'}),
    effects_UNSTABLE: [mockEffect(opts.key)],
  }));
}

const add10 = mockedSelector({
  key: 'add10',
  get: ({get}) => get(multipliedBy2) + 10,
});

The abstraction doesn't work with settable selectors, of course, and would add some run-time cost, though..

[BenjaBobs]

Thanks, that looks like an interesting solution. I'll try it out soon, see if it solves my use case.

Do you have anything in the pipeline to solve this from inside the api so we can avoid the overhead?

[drarmstr]

No other proposals so far that wouldn't be dangerous. As you point out, you want to use this for intermediate selectors. If you set their cached value then besides the concerns about state consistency there would also be the problem that upstream state changes would invalidate the cache anyway, making it both dangerous and fragile.

Another simple option to support settable selectors and avoid the runtime overhead of wrappers completely when not testing:

function mockedSelector<T>(opts): RecoilState<T> {
  const baseSelector = selector<T>(...opts);

  if (!TESTING_ENVIRONMENT) {
    return baseSelector;
  } else {
    return selector({
      ...opts
      key: opts.key+'/mocked',
      get: ({get}) =>
        MOCKED_VALUES.has(opts.key)
          ? MOCKED_VALUES.get(opts.key)
          : get(baseSelector),
    });
  } 
}

(You'd want to type this for both RecoilState and RecoilValueReadOnly variants)

[drarmstr]

@BenjaBobs - if you confirm this pattern is useful, we should update the documentation

[BenjaBobs]

Sorry I've been so slow to answer, was moved to another more urgent project so I haven't been able to implement and test it. How would this specific scenario work for family-variants? If I remember correctly, the serialization process for the parameters was not simply JSON serialization, right?

[drarmstr]

Right, it's a different serialization to support stable key ordering, Maps, Sets, &c. Though, not sure that is relevant here? A family variant would be needed as well. Would something like the following work:

function mockedSelectorFamily<T, P>(opts): P => RecoilState<T> {
  const baseSelectorFamily = selectorFamily<T>(...opts);

  if (!TESTING_ENVIRONMENT) {
    return baseSelectorFamily;
  } else {
    return selectorFamily({
      ...opts
      key: opts.key+'/mocked',
      get: param => ({get}) =>
        MOCKED_VALUES.has(opts.key)?.has(param)
          ? MOCKED_VALUES.get(opts.key)?.get(param)
          : get(baseSelectorFamily(param)),
    });
  } 
}

If you want to dynamically change the mocked value during a test, maybe using an atomFamily to store the mocked values would be safer:

const mockedValuesState = atomFamily({
  key: 'MockedValues',
  default: new DefaultValue(),
});

function mockedSelector<T>(opts): RecoilState<T> {
  const baseSelector = selector<T>(...opts);

  if (!TESTING_ENVIRONMENT) {
    return baseSelector;
  } else {
    return selector({
      ...opts
      key: opts.key+'/mocked',
      get: ({get}) => {
        const mockedValue = get(mockedValuesState(opts.key));
        return mockedValue instanceof DefaultValue ? get(baseSelector) : mockedValue;
      },
    });
  } 
}

function mockedSelectorFamily<T, P>(opts): P => RecoilState<T> {
  const baseSelectorFamily = selectorFamily<T>(...opts);

  if (!TESTING_ENVIRONMENT) {
    return baseSelectorFamily;
  } else {
    return selectorFamlily({
      ...opts
      key: opts.key+'/mocked',
      get: param => ({get}) => {
        const mockedValue = get(mockedValuesState(opts.key + '__' + stableStringify(param)));
        return mockedValue instanceof DefaultValue ? get(baseSelectorFamily(param)) : mockedValue;
      },
    });
  } 
}

[BenjaBobs]

This has once again become an issue for my team, as we have many many selectors around in our code, and recently started an effort to get it all tested. We can do the mock atom everywhere we define our selectors, I think that having to modify our code to be test friendly like this is both counter intuitive, and also hurts readability. It decreases the signal-to-noise ratio of the code and increases the cognitive load required to understand the defined selectors. Is there any way I can convince you to once again take a look at creating some sort of way for us to mock cached values in selectors?

[drarmstr]

Yeah, absolutely agree that the solution shouldn't pollute the code base. Well, the idea was that you could just swap out selector() for mockedSelector() and selectorFamily() for mockedSelectorFamily() and the mockedValuesState implementation would live with the tests. Happy to iterate on this approach for documentation or seeing if it makes sense to include something with the library.

An advantage of the wrapping approach versus messing with the cache is that it avoids complications with cache invalidation, etc. e.g. would the set cache entry be cleared if an upstream dependency is updated. A downside for your original proposal and the above wrapper approach is that it doesn't provide mocked values specific to the dependency input values, not sure if that's a requirement of yours. However, the wrapper approach could be extended to support that if the wrapper provided an alternative selector implementation vs just a value.

[soruban]

@drarmstr I'd be keen on seeing more documentation on the matter or know if anything will be added to the library. I have been enjoying using Recoil as it has severely reduced boilerplate in the application code but unfortunately increased it in the tests. Here is an example of our use case:

const numberFilterState = atom<number[]>({
  key: 'numberFilterState',
  default: selector<number[]>({
    key: 'numberFilterStateStateDefault',
    get: ({ get }) => {
      const metadata = get(metaDataState);

      return metadata.forEach(/* Do some transform and return number[] */);
    },
  })
})

const textFilterState = atom<string>({
  key: 'textFilterState',
  default: '',
})

const filteredDataState = selector<SomeData[]>({
  key: 'filteredDataState',
  get: ({get}) => {
    const allData = get(allDataState);
    const numberFilter = get(numberFilterState);
    const textFilter = get(textFilterState);

    return allData.filter(/* Apply numberFilter & textFilter */)
  }
})

function DataViewer() {
  const allData = useRecoilValue(filteredDataState);

  return (
    <div>
      {allData.map(/* Render each data item. */)}
    </div>
  )
}

Ideally, we should be able to test DataViewer by mocking filteredDataState only, with a "simple-ish" API like:

it('shows the data', () => {
  render(
    <RecoilTestRoot
      mockState={(mutableTestState) => {
        mutableTestState.mockSelector(filteredDataState).and.returnValue([1, 2, 3]);
      }}
    >
      <DataViewer />
    </RecoilTestRoot>
  );

  expect(...).toEqual(...)
});

[willschnicke-db]

I agree with @soruban. It is very challenging to mock Recoil state currently, and this is one of the biggest challenges. Mocking the result of a selector should be much simpler than it currently is.

Current solutions involve either editing the production code (making your readonly selectors writeable, yuck) or in cases where a selector derives state from other readonly selectors, you must often mock an atom far down the stack of selectors, which adds fragility.