dai-shi / react-suspense-fetch

[NOT MAINTAINED] A low-level library for React Suspense for Data Fetching
MIT License
296 stars 8 forks source link

Strong caching for object inputs #18

Closed axelboc closed 3 years ago

axelboc commented 3 years ago

If I understand correctly, the library keeps two caches: a "weak" cache (WeakMap) and a "strong" cache (Map). The weak cache is used for object inputs; the strong cache for primitive inputs.

I would really like to be able to pass an object to the store's methods while benefiting from the strong caching strategy.

Right now, I've written a little wrapper store that calls JSON.stringify (and JSON.parse on the way out) so that my input objects are transformed into strings in order to get the strong caching strategy. It works fine, but it'd be nice if createFetchStore could support it out of the box with some configuration options of sorts. The ideal would probably be to allow passing custom stringify and parse functions to pre- and post-process the inputs/results.

dai-shi commented 3 years ago

Interesting. I wanted everything "weak" and "strong" for primitives are kind of regret. Out of curiosity, why do you need "strong" for your objects? Because if you keep the object reference in your code, it will not be GC'd. And even if you keep it "strongly" in the lib, there's no way to get it. So, it seems to me that you wouldn't notice if it's "weak" or "strong".

axelboc commented 3 years ago

I guess I don't want to keep references to the objects in my code, as it would defeat the purpose of having a store with a built-in cache. Basically, I don't want to have to care about object references at all -- I want the cache to be against the keys and values of the objects I pass to prefetch, get and evict, since I call those from various places.

Perhaps I should clarify my use case, since the root limitation I'm facing runs deeper.

I have an API that abstracts fetching operations, say async getValue(path: string, selection: string). This basically results in a request to something like /my-endpoint?path={path}&selection={selection}.

What I'd like to be able to do (in pseudo-code) is:

const myStore = createObjectKeyStore(getValue);
myStore.get('/my-path', 'my-selection-1'); // request
myStore.get('/my-path', 'my-selection-1'); // cache hit
myStore.get('/my-path', 'my-selection-2'); // request
myStore.get('/my-other-path', 'my-selection-1'); // request

So the real problem I'm facing is that createFetchStore expects an async function with a single parameter, whereas my API function requires two parameters.

I worked around this by rewriting my API function to accept a single object parameter: async getValue(params: { path: string; selection: string; }), but then encountered the weak caching problem. I don't keep the references of the params objects I pass to myStore.get() and to the other methods, so because of WeakMap, they can end up being "evicted" from the cache by the garbage collector.

I worked around this second issue by writing a little wrapper that stringifies params before passing it to the inner fetch store (thus benefiting from the strong caching strategy), and by parsing it back to an object before passing it to the API function.

Here is the code, for reference ```ts interface ObjectKeyStore, R = unknown> { get: (obj: I) => R; prefetch: (obj: I) => void; evict: (obj: I) => void; } export function createObjectKeyStore, R = unknown>( fetchFunc: (params: I) => Promise ): ObjectKeyStore { const store = createFetchStore((key: string) => { return fetchFunc(JSON.parse(key) as I); }); function wrapMethod(method: (key: string) => T) { return (params: I) => { return method(JSON.stringify(params)); }; } const { get, prefetch, evict } = store; return { get: wrapMethod(get), prefetch: wrapMethod(prefetch), evict: wrapMethod(evict), }; } ```

Obviously this is far from ideal, since the order of the keys matters, and some values cannot be serialised to JSON. So ideally, createFetchStore would accept an async function with any number of parameters and of any types, and keep a strong cache of return values against the union of all these parameters. However, this is too complex and too specific for a generic library like react-suspense-fetch, which is why I suggested letting users pass their own stringify and parse functions.

So perhaps I should reword the title of my issue, as I think the current strong/weak caching behaviour for primitive/object inputs can stay. Here's what I have in mind:

async function getValue(path: string, selection: string) { ... }

const myStore = createObjectKeyStore(getValue, {
  stringifier: (path: string, selection: string) {
    return JSON.stringify({ path, selection }); // predictable order of keys + I can deal with non-parseable values any way I want
  },
  parser: (fetchFunc: FetchFunc, cacheKey: string) {
    const { path, selection } = JSON.parse(cacheKey);
    return fetchFunc(path, selection);
  }
});

myStore.get('/my-path', 'my-selection');
axelboc commented 3 years ago

Note that I'd still be perfectly happy with a single boolean option, like autoSerialization that automatically runs JSON.stringify and JSON.parse. I would have to be careful about key ordering and non-serialisable values, but it would do the trick:

async function getValue({ path: string, selection: string }) { ... }
const myStore = createObjectKeyStore(getValue, { autoSerialization: true });
myStore.get({ path: '/my-path', selection: 'my-selection' });
dai-shi commented 3 years ago

Thanks for the explanation. I totally understand the use case. Yeah, this use case isn't covered currently and that was partly intentional to make the library simple. But, your explanation is convincing me that this would be good for DX.

Let me see. I think I would add custom comparator. (serialization is not super nice. Although, JSON.stringify is very fast for small objects, it doesn't seem to be predictable, and I wouldn't prefer it.)

dai-shi commented 3 years ago

Hm, passing areEqual for all get() makes sense, but not very handy. I should change the impl completely to decide which map "weak" or "strong" to use at the store creation. I don't thinks it's rare to combine both and it'd be confusing anyway.

axelboc commented 3 years ago

Yeah, that would make sense.

axelboc commented 3 years ago

If I'm following, you're thinking of something like:

import shallow from 'zustand/shallow';

const myStore = createObjectKeyStore(getValue, {
  weakCache: false, // probably the default
  comparator: shallow
});
dai-shi commented 3 years ago

yeah, and I'm thinking if I can create a simple api without option object...

dai-shi commented 3 years ago

Hmmm, it's going to be really strange without option object... Gave up. Let's use option.