TypeScript simple localStorage cache

Question

I have a simple localStorage cache class written in TypeScript, would love to have some insights on any way/aspect you think it can be improved (making it more readable, maintainable, usable, etc.):

export interface CacheItem<T> { data: T; expiration: number; } class SimpleLocalStorageCache<T> { constructor(private key: string, private durationInSeconds: number) {} get(): CacheItem<T> | null { const cache = localStorage.getItem(this.key); if (!cache) { return null; } const parsedCache = JSON.parse(cache); if (Date.now() >= parsedCache.expiration) { return null; } return parsedCache; } update(data: T): void { const durationInMilliseconds = this.durationInSeconds * 1000; localStorage.setItem( this.key, JSON.stringify({ data, expiration: Date.now() + durationInMilliseconds, }) ); } } export default CacheSimpleLocalStorageCache; 

Really appreciate any feedback on how to improve.

Answer 1

I took a look at it and will elaborate on my few findings below.

First of all, your default export is wrong, but most likely you knew about it and you have already fixed it in your local version (might be also some pasting issue) - it's a no brainer. SimpleLocalStorageCache vs. CacheSimpleLocalStorageCache. Also I don't think it's necessary to export the interface CacheItem as it's most likely an internal interface to be used. Do not leak it, to prevent misuse :-)

Next, looking at the constructor constructor(private key: string, private durationInSeconds: number) {} it's quite nice that you've used constructor assignment, thumbs up for that. What I think is a bit weird though, is the fact, that you've decided that the consumer of the API has to pass in seconds. That's a bit odd if we look at for example the parameters of setTimeout.

Another thing that bugs me with the seconds/calculation is, that I would except it to be converted to milliseconds inside the constructor. That's a one time calculation and isn't required to be done each time a consumer of this API is calling the update function.

I noticed that I can pass in functions which get's lost:

import Storage from "yourStorageImplementation"; const TWO_SECONDS = 2; const instance = new Storage("someKey", TWO_SECONDS); instance.update(() => console.log("this will be lost!")); 

Fetching the exact same, results in this unexpected behavior:

instance.get() // -> { data: undefined, expiration: /*some value */ } 

If you don't support persisting functions, which is fine, prevent the consumer of the API to pass in such data. This could be achieved by e.g. adjusting the generic to only allow strings, booleans, and objects with key, value pairs maybe extend/reduce this to your needs:

class SimpleLocalStorageCache<T extends string | number | boolean | { [key: string]: string | number | boolean }> { ... } 

Alternatively you could do filter out just any function like:

type IsNotFuction<X> = X extends (...args: any[]) => any ? never : X; class SimpleLocalStorageCache<T> { ... update(data: IsNotFuction<T>): void { const durationInMilliseconds = this.durationInSeconds * 1000; localStorage.setItem( this.key, JSON.stringify({ data, expiration: Date.now() + durationInMilliseconds, }) ); } } // Usage, isn't quite handy: const storage1 = new SimpleLocalStorageCache<() => void>("someKey", 2); // It's required to provide the generic here, meeeh! console.log(storage1.update(() => alert("bla"))) // Errors here! console.log(storage1.get()); 

I do like the solution and most of my write up is opinionated. Below are some suggestions I came up with while fiddling around with your solution:

Suggestion: Do you intend to extend it for sessionStorage and let the user decide the caching strategy? Is there a plan to properly support functions?

Suggestion: What do you think about a fluent API for this extension. Wouldn't it be nice to support something like: new Storage("key", 2).update(/* some data */).get(); to immediately fetch the result again?