Update Cache Interface types

[dmmulroy]

Update Cache Interface types

Update 04/08/23

Per discussion with @promer94, the solution outlined in An Alternative Approach has been implemented rather than updating the keys interface.

Motivation & Background

As it's currently typed, the Cache interface that is required for users to create and provide their own cache implementations mandates that the keys method return an IterableIterator<string>. I believe this to be incorrectly typed, or at the very least, inconveniently typed and a burden for consumers. I think that the correct type should be IterableIterator<Key> given that the get, set, and delete methods all require a Key as input.

Consider the default implementation provided by swr:

// Default cache provider
const [cache, mutate] = initCache(new Map()) as [Cache<any>, ScopedMutator<any>]

This is effectively the same as:

const cacheMap: Cache<any> = new Map<Key, State<any, any>>()
const [cache, mutate] = initCache(cacheMap) as [Cache<any>, ScopedMutator<any>]

Which results in a type error:

Type 'Map<Key, State<any, any>>' is not assignable to type 'Cache<any>'.
     The types returned by 'keys()' are incompatible between these types.
       Type 'IterableIterator<Key>' is not assignable to type 'IterableIterator<string>'.
         Type 'Key' is not assignable to type 'string'.
           Type 'undefined' is not assignable to type 'string'.

This puts a burden on users/consumers of swr when implementing their own cache, especially if the underlying data structure is a Map.

My use case is creating a custom LRU Cache to back swr, see the following gist for its implementation: https://gist.github.com/dmmulroy/33c31bef81554b099726b3a680dad974

Because keys is typed as IterableIterator<string> I have to cast.

/**
   * Returns an iterable iterator over the keys in the cache.
   *
   * @returns {IterableIterator<string>} - An iterator of the cache keys.
   */
  public keys(): IterableIterator<string> {
    return this.cache.keys() as IterableIterator<string>
  }

An Alternative Approach

An alternative approach would be to update the Cache interface to the following:

export interface Cache<Data = any> {
  keys(): IterableIterator<string>
  get(key: string): State<Data> | undefined
  set(key: string, value: State<Data>): void
  delete(key: string): void
}

This actually may be more accurate and reflect what occurs at run time. As far as I can tell, any time the cache is accessed by swr internally the key is always serialized into a string via the serialize¹ function prior to access.

You can see that any time the cache is accessed², it's accessed via callbacks returned from createCacheHelper³ in which the SWRGlobalState is referenced/retrieved and used get to the setter callback from initCache⁴ which is stored in global state. In all instances (you can search by reference to confirm), the Getter and Setter callbacks are passed string keys which have been serialized.

In addition, I believe this may actually be worth considering because unstable_serialize⁵ is publicly exported. I assume this is to aid consumers/users if they choose to directly access/mutate the cache (despite explicitly being called out in the docs as anti pattern⁶) as seen in #2216.

Footnotes

1. https://github.com/vercel/swr/blob/main/_internal/utils/serialize.ts#L6 ↩

2. Getter Callback, Setter Callback, Previous value in Setter, Setter Callback from SWRGlobalState ↩

3. https://github.com/vercel/swr/blob/main/_internal/utils/helper.ts#L32 ↩

4. https://github.com/vercel/swr/blob/main/_internal/utils/cache.ts#L26 ↩

5. https://github.com/vercel/swr/blob/main/core/use-swr.ts#L679 ↩

6. https://swr.vercel.app/docs/advanced/cache#modify-the-cache-data ↩

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b58d4a0:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration
SWR-SSR	Configuration

[dmmulroy]

I also just noticed that the docs are incorrect as well and list the type of Cache as the following:

interface Cache<Data> {
  get(key: string): Data | undefined
  set(key: string, value: Data): void
  delete(key: string): void
  keys(): IterableIterator<string>
}

[promer94]

Thanks for the PR !

I think we should take the alternative approach here.
It is more simple and the fact is that we only use string key here.

[dmmulroy]

Awesome - I'll be happy to make those changes and get them pushed up tomorrow or sunday.

[dmmulroy]

Changes have been made, take a look when you have the chance @promer94 🚀

[promer94]

Thanks