sindresorhus/quick-lru

Explicit maxAge in set() is stored as relative

mwojnars opened this issue · 4 comments

It seems that maxAge argument is used incorrectly in the code of set(): the addition of Date.now() is present only for a default this.maxAge, but missing when an explicit maxAge was provided, which results in assigning a relative [ms] value to expiry instead of an absolute timestamp value.

set(key, value, {maxAge = this.maxAge === Number.POSITIVE_INFINITY ? undefined : Date.now() + this.maxAge} = {}) {

Let's add a test for this too haha, spent 30min going, what is going on 🙃 ?? As it took me a while to start questioning the assumption that simply setting a maxAge correctly did not work 😅 .

Downgraded to 6.0.0 for now. Might have some time later to fix this, need to run now!

Thanks for a great library!

ooooh, well "correctly" is debatable here. Seems from the test the option is intended to be used as a timestamp, not a duration in milliseconds.

I'd suggest we either:

  1. not use the same name for an option that on the one hand takes a number of milliseconds to keep the item in cache before expiring, and on the other hand takes a timestamp that indicates when to expire the item.
  2. accept a number of milliseconds to keep the item in cache in both cases.

fwiw 2. has my preference. and I'm not sure but, this may be breaking backward compatibility?

You are right @alextes bad concept there, I Will release a fix using the second option as soon I have some free time ;)