medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.73k stars 61 forks source link

`max: 0` doesn't disable cache. #145

Open monkekode opened 1 week ago

monkekode commented 1 week ago

Hi, I'm trying to memoize a function to the point where there is no cache, but concurrent execution also doesn't happen. To do this max: 0 should work to disable cache, but in practice it doesn't. Example:

import memoizee from 'memoizee';
import {sleep} from "../lib/util.js";

const x = async (s: string) => {
    await sleep(500);
    console.log(`Running ${s}`);
}

const m = memoizee(x, {max: 0, primitive: true});

await m('TEST');
await m('TEST');
await m('TEST 2');
await m('TEST');

In this case output is

Running TEST
Running TEST 2

Meaning the values are memoized. Setting max to 1 works properly, as do higher values.

medikoo commented 1 week ago

@monkekode, what exactly is your use case? Why set up memoization when you don't want caching?

Behind the scenes, max: 0 is treated the same as when no max option is set -> https://github.com/medikoo/memoizee/blob/6aa37460d9c996e737cad3f127a7ed1b7af9d168/ext/max.js#L13

monkekode commented 1 week ago

@medikoo

I have an endpoint on the site that calls a slow external API. If 2+ requests come to my site at the same time, I don't want to have 2+ concurrently running requests going to the external API, I want the requests that came while the first one was underway to wait for the first one to finish and receive the same result. I also don't want to cache the response since it changes over time.

This works with p-memoize and cache: false, but I'm trying to find a package that has both the ability to disable cache and maxAge, which p-memoize for some reason doesn't.

medikoo commented 1 week ago

@monkekode In this case I believe you need to first decide when you consider the response you eventually cached is stale (likely not up to date anymore.

Having that you should achieve that with following options:

promise:true, maxAge: millisecondsAfterWhichShouldBeDiscarded

If you want the responses to be freed from cache near immediately after they're obtained, just set maxAge: 1

monkekode commented 1 week ago

Ok, thanks. Max age of 1 ms is fine in this case, although it would be nice to have the ability to just disable the cache altogether, maybe a max: 'no-cache' or something like that.

medikoo commented 1 week ago

@monkekode I'm not sure if disabling cache is right solution here. It's clear that you want to cache value, but just for a repeated requests which may happen when first one still is not fulfilled

One improvement coud be to support maxAge: 0 to clearly mark this intent. I've updated v1.0 API spec to take that into account